Commit 8c852c8d authored by Matt Giuca's avatar Matt Giuca Committed by Commit Bot

Added null checks for properties on ToolbarView that can be null.

BrowserView's toolbar() will have null app_menu_button() and
browser_actions() in hosted app windows, which could result in null
dereferences in certain corner cases. This adds null checks where it's
obviously safe to do so.

A better fix would be to use toolbar_button_provider() to access the
hosted app window's real app menu button and actions container, but that
requires more careful consideration.

Bug: 826596, 811982
Change-Id: Iea0a0ea3acffe5ed1016e61bd8ae2fe36971c79d
Reviewed-on: https://chromium-review.googlesource.com/1010002Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550381}
parent b909c213
...@@ -23,13 +23,13 @@ constexpr int kDefaultIncognitoWindowPromoShowTimeInHours = 2; ...@@ -23,13 +23,13 @@ constexpr int kDefaultIncognitoWindowPromoShowTimeInHours = 2;
constexpr char kIncognitoWindowObservedSessionTimeKey[] = constexpr char kIncognitoWindowObservedSessionTimeKey[] =
"incognito_window_in_product_help_observed_session_time_key"; "incognito_window_in_product_help_observed_session_time_key";
// Note: May return null.
BrowserAppMenuButton* GetAppMenuButton() { BrowserAppMenuButton* GetAppMenuButton() {
auto* browser = BrowserView::GetBrowserViewForBrowser( auto* browser = BrowserView::GetBrowserViewForBrowser(
BrowserList::GetInstance()->GetLastActive()); BrowserList::GetInstance()->GetLastActive());
DCHECK(browser); DCHECK(browser);
DCHECK(browser->IsActive()); DCHECK(browser->IsActive());
DCHECK(browser->toolbar()); DCHECK(browser->toolbar());
DCHECK(browser->toolbar()->app_menu_button());
return browser->toolbar()->app_menu_button(); return browser->toolbar()->app_menu_button();
} }
...@@ -53,7 +53,6 @@ void IncognitoWindowTracker::OnIncognitoWindowOpened() { ...@@ -53,7 +53,6 @@ void IncognitoWindowTracker::OnIncognitoWindowOpened() {
void IncognitoWindowTracker::OnBrowsingDataCleared() { void IncognitoWindowTracker::OnBrowsingDataCleared() {
auto* app_menu_button = GetAppMenuButton(); auto* app_menu_button = GetAppMenuButton();
// TODO(bdea): Remove early return once https://crbug.com/811982 is fixed.
if (!app_menu_button) if (!app_menu_button)
return; return;
...@@ -87,7 +86,9 @@ void IncognitoWindowTracker::OnWidgetDestroying(views::Widget* widget) { ...@@ -87,7 +86,9 @@ void IncognitoWindowTracker::OnWidgetDestroying(views::Widget* widget) {
if (incognito_promo_observer_.IsObserving(widget)) { if (incognito_promo_observer_.IsObserving(widget)) {
incognito_promo_observer_.Remove(widget); incognito_promo_observer_.Remove(widget);
GetAppMenuButton()->SetIsProminent(false); BrowserAppMenuButton* app_menu_button = GetAppMenuButton();
if (app_menu_button)
app_menu_button->SetIsProminent(false);
} }
} }
......
...@@ -90,6 +90,7 @@ views::View* AnchorViewForBrowser(ExtensionInstalledBubble* controller, ...@@ -90,6 +90,7 @@ views::View* AnchorViewForBrowser(ExtensionInstalledBubble* controller,
BrowserActionsContainer* container = BrowserActionsContainer* container =
browser_view->toolbar()->browser_actions(); browser_view->toolbar()->browser_actions();
// Hitting this DCHECK means |ShouldShow| failed. // Hitting this DCHECK means |ShouldShow| failed.
DCHECK(container);
DCHECK(!container->animating()); DCHECK(!container->animating());
reference_view = container->GetViewForId(controller->extension()->id()); reference_view = container->GetViewForId(controller->extension()->id());
...@@ -405,7 +406,7 @@ bool ExtensionInstalledBubble::ShouldShow() { ...@@ -405,7 +406,7 @@ bool ExtensionInstalledBubble::ShouldShow() {
BrowserView::GetBrowserViewForBrowser(browser()) BrowserView::GetBrowserViewForBrowser(browser())
->toolbar() ->toolbar()
->browser_actions(); ->browser_actions();
return !container->animating(); return container && !container->animating();
} }
return true; return true;
} }
......
...@@ -39,7 +39,7 @@ bool MediaRouterActionPlatformDelegateViews::CloseOverflowMenuIfOpen() { ...@@ -39,7 +39,7 @@ bool MediaRouterActionPlatformDelegateViews::CloseOverflowMenuIfOpen() {
BrowserView::GetBrowserViewForBrowser(browser_) BrowserView::GetBrowserViewForBrowser(browser_)
->toolbar() ->toolbar()
->app_menu_button(); ->app_menu_button();
if (!app_menu_button->IsMenuShowing()) if (!app_menu_button || !app_menu_button->IsMenuShowing())
return false; return false;
app_menu_button->CloseMenu(); app_menu_button->CloseMenu();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment