Desktop Page Info/Mac: Fix cookie updates crashing on transition to full screen.

Whenever the Page Info bubble needs to update the number of cookies being used
on a site, it will update the text in the Page Info bubble to reflect this,
which will then trigger a layout update and reanchor the bubble. However, if
this occurs while the browser window is moving into full screen mode, the Page
Info parent window will be null and this operation will crash.

Fix by checking whether the parent window exists before trying to reanchor.
Moving into full screen will also close the Page Info bubble, so the reanchor is
unnecessary anyway - it will be correctly positioned the next time it is opened.

Bug: 773283
Change-Id: I6f7418fcc96a95b8e3f0a3276ac009cef6eda92e
Reviewed-on: https://chromium-review.googlesource.com/720579
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510240}

chrome/browser/ui/cocoa/page_info/page_info_bubble_controller.mm

@@ -133,7 +133,7 @@ constexpr int kChosenObjectTag = CONTENT_SETTINGS_NUM_TYPES;
 // NOTE: This assumes that there will never be more than one page info
 // bubble shown, and that the one that is shown is associated with the current
 // window. This matches the behaviour in Views: see PageInfoBubbleView.
-bool g_is_bubble_showing = false;
+PageInfoBubbleController* g_page_info_bubble = nullptr;
 
 // Takes in the parent window, which should be a BrowserWindow, and gets the
 // proper anchor point for the bubble. The returned point is in screen
@@ -228,6 +228,10 @@ NSPoint AnchorPointForWindow(NSWindow* parent) {
 
 @implementation PageInfoBubbleController
 
++ (PageInfoBubbleController*)getPageInfoBubbleForTest {
+  return g_page_info_bubble;
+}
+
 - (CGFloat)defaultWindowWidth {
   return kDefaultWindowWidth;
 }
@@ -625,6 +629,12 @@ bool IsInternalURL(const GURL& url) {
 // Layout all of the controls in the window. This should be called whenever
 // the content has changed.
 - (void)performLayout {
+  // Skip layout if the bubble is closing.
+  InfoBubbleWindow* bubbleWindow =
+      base::mac::ObjCCastStrict<InfoBubbleWindow>([self window]);
+  if ([bubbleWindow isClosing])
+    return;
+
   // Make the content at least as wide as the permissions view.
   CGFloat contentWidth =
       std::max([self defaultWindowWidth], NSWidth([permissionsView_ frame]));
@@ -1427,18 +1437,18 @@ PageInfoUIBridge::PageInfoUIBridge(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       web_contents_(web_contents),
       bubble_controller_(nil) {
-  DCHECK(!g_is_bubble_showing);
-  g_is_bubble_showing = true;
+  DCHECK(!g_page_info_bubble);
 }
 
 PageInfoUIBridge::~PageInfoUIBridge() {
-  DCHECK(g_is_bubble_showing);
-  g_is_bubble_showing = false;
+  DCHECK(g_page_info_bubble);
+  g_page_info_bubble = nullptr;
 }
 
 void PageInfoUIBridge::set_bubble_controller(
     PageInfoBubbleController* controller) {
   bubble_controller_ = controller;
+  g_page_info_bubble = controller;
 }
 
 void PageInfoUIBridge::SetIdentityInfo(
@@ -1487,7 +1497,7 @@ void ShowPageInfoDialogImpl(Browser* browser,
   // Don't show the bubble if it's already being shown. Since this method is
   // called each time the location icon is clicked, each click toggles the
   // bubble in and out.
-  if (g_is_bubble_showing)
+  if (g_page_info_bubble)
     return;
 
   // Create the bridge. This will be owned by the bubble controller.
@@ -1495,7 +1505,7 @@ void ShowPageInfoDialogImpl(Browser* browser,
   NSWindow* parent = browser->window()->GetNativeWindow();
 
   // Create the bubble controller. It will dealloc itself when it closes,
-  // resetting |g_is_bubble_showing|.
+  // resetting |g_page_info_bubble|.
   PageInfoBubbleController* bubble_controller =
       [[PageInfoBubbleController alloc] initWithParentWindow:parent
                                             pageInfoUIBridge:bridge

chrome/browser/ui/cocoa/page_info/page_info_bubble_views_mac_browsertest.mm

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "chrome/browser/ui/cocoa/page_info/page_info_bubble_controller.h"
+
+#include "base/command_line.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h"
+#include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
+#include "chrome/browser/ui/page_info/page_info_dialog.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/views/page_info/page_info_bubble_view.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "ui/base/material_design/material_design_controller.h"
+#import "ui/base/test/scoped_fake_nswindow_fullscreen.h"
+#include "ui/base/ui_base_switches.h"
+#include "ui/views/bubble/bubble_dialog_delegate.h"
+#include "ui/views/widget/widget.h"
+#include "url/url_constants.h"
+
+@interface PageInfoBubbleController (ExposedForTesting)
++ (PageInfoBubbleController*)getPageInfoBubbleForTest;
+- (void)performLayout;
+@end
+
+namespace {
+
+using PageInfoBubbleViewsMacTest = InProcessBrowserTest;
+
+}  // namespace
+
+// Test the Page Info bubble doesn't crash upon entering full screen mode while
+// it is open. This may occur when the bubble is trying to reanchor itself.
+IN_PROC_BROWSER_TEST_F(PageInfoBubbleViewsMacTest, NoCrashOnFullScreenToggle) {
+  ui::test::ScopedFakeNSWindowFullscreen fake_fullscreen;
+  ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
+  ShowPageInfoDialog(browser()->tab_strip_model()->GetWebContentsAt(0));
+  ExclusiveAccessManager* access_manager =
+      browser()->exclusive_access_manager();
+  FullscreenController* fullscreen_controller =
+      access_manager->fullscreen_controller();
+
+  fullscreen_controller->ToggleBrowserFullscreenMode();
+  if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
+    views::BubbleDialogDelegateView* page_info =
+        PageInfoBubbleView::GetPageInfoBubble();
+    EXPECT_TRUE(page_info);
+    views::Widget* page_info_bubble = page_info->GetWidget();
+    EXPECT_TRUE(page_info_bubble);
+    EXPECT_EQ(PageInfoBubbleView::BUBBLE_PAGE_INFO,
+              PageInfoBubbleView::GetShownBubbleType());
+    EXPECT_TRUE(page_info_bubble->IsVisible());
+  } else {
+    EXPECT_TRUE([PageInfoBubbleController getPageInfoBubbleForTest]);
+    // In Cocoa, the crash occurs when the bubble tries to re-layout.
+    [[PageInfoBubbleController getPageInfoBubbleForTest] performLayout];
+  }
+
+  // There should be no crash here from re-anchoring the Page Info bubble while
+  // transitioning into full screen.
+  fake_fullscreen.FinishTransition();
+}

chrome/test/BUILD.gn

@@ -2533,6 +2533,7 @@ test("browser_tests") {
           "../browser/ui/cocoa/location_bar/zoom_decoration_browsertest.mm",
           "../browser/ui/cocoa/omnibox/omnibox_view_mac_browsertest.mm",
           "../browser/ui/cocoa/one_click_signin_dialog_controller_browsertest.mm",
+          "../browser/ui/cocoa/page_info/page_info_bubble_views_mac_browsertest.mm",
           "../browser/ui/cocoa/passwords/passwords_bubble_browsertest.mm",
           "../browser/ui/cocoa/permission_bubble/permission_bubble_cocoa_browser_test.mm",
           "../browser/ui/cocoa/profiles/profile_signin_confirmation_view_controller_browsertest.mm",