Bookmark prompt controller should check browser window available or not. (issue 11361091)

[yo...@chromium.org]

Reviewers: sky,

Message:
Could you review this patch?
Thanks in advance.

Description:
CanShowBookmarkPrompt() is called during construction of Browser object
before
initializing window_ member variable by calling CreateBrowserWindow().

CreateBrowserWindow() calls Windows API CreateWindowEx() and it dispatches
WM_NCACTIVATE.

It seems some tools, such as RocketDoc(http://rocketdock.com/) and Matrox
PowrDesk(http://www.matrox.com/graphics/en/products/multi_display_software/),
are involved this symptom.

BUG=159146


Please review this at http://codereview.chromium.org/11361091/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc


Index: chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc
diff --git a/chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc
b/chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc
index
b557bb1bd917e6607359087ff7ab318136d6291d..8e8407a7fafc62bbe25efe99b77071ecff1e39ba
100644
--- a/chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc
@@ -51,7 +51,7 @@ bool CanShowBookmarkPrompt(Browser* browser) {
if (!browser || browser->type() != Browser::TYPE_TABBED ||
browser->profile()->IsOffTheRecord() ||
!browser->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR) ||
- !browser->window()->GetLocationBar())
+ (browser->window() && !browser->window()->GetLocationBar()))
return false;
BookmarkPromptPrefs prefs(browser->profile()->GetPrefs());
if (!prefs.IsBookmarkPromptEnabled())

[s...@chromium.org]

https://codereview.chromium.org/11361091/diff/1/chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc
File chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc (right):

https://codereview.chromium.org/11361091/diff/1/chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc#newcode54
chrome/browser/ui/bookmarks/bookmark_prompt_controller.cc:54:

(browser->window() && !browser->window()->GetLocationBar()))

Won't this mean OnBrowserSetLastActive could ignore the browser?

https://codereview.chromium.org/11361091/

[yo...@chromium.org]

Could you review this patch?
Thanks in advance.

= Changes since the last review =
* Checks "const" browser properties in OnBrowserSetLastActive() and use
CanSupportWindowFeature() instead of SupportsWindowFeature(), which checks
full-screen mode on Mac, to avoid redundant checking of Browser properties.
* Changes CanShowBookmarkPrompt() to check only profile values.
* Checks SupportsWindowFeature(FEATURE_LOCATIONBAR) before showing prompt.

http://codereview.chromium.org/11361091/

[s...@chromium.org]

LGTM

https://codereview.chromium.org/11361091/

[commi...@chromium.org]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yo...@chromium.org/11361091/5001


https://chromiumcodereview.appspot.com/11361091/

[commi...@chromium.org]

Change committed as 166385

https://chromiumcodereview.appspot.com/11361091/