From a9bf3a5fe62bead51846f9165dcebe3529bac134 Mon Sep 17 00:00:00 2001 From: "jeremya@chromium.org" Date: Sat, 29 Oct 2011 00:55:02 +0000 Subject: Fix a crash in FullscreenExitBubbleController when the user clicks the "Exit full screen" button. The crash was due to the fact that Browser::OnDenyFullscreenPermission causes BrowserWindow::ExitFullscreen to be called, which causes the FullscreenExitBubbleController to be destroyed. [FullscreenExitBubbleController deny:] was sending a -hideSoon message to its zombie self following the call to Browser::OnDenyFullscreenPermission. BUG=101835 TEST=The browser doesn't crash when you click "Exit full screen" on a fullscreened page. Review URL: http://codereview.chromium.org/8399017 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107841 0039d316-1c4b-4281-b951-d872f2087c98 --- .../browser/ui/cocoa/browser_window_controller.h | 3 ++ .../browser/ui/cocoa/browser_window_controller.mm | 5 ++ .../ui/cocoa/fullscreen_exit_bubble_controller.mm | 1 - .../fullscreen_exit_bubble_controller_unittest.mm | 59 ++++++++++++++++++++-- 4 files changed, 62 insertions(+), 6 deletions(-) (limited to 'chrome/browser/ui/cocoa') diff --git a/chrome/browser/ui/cocoa/browser_window_controller.h b/chrome/browser/ui/cocoa/browser_window_controller.h index cf77b7d..58e3856 100644 --- a/chrome/browser/ui/cocoa/browser_window_controller.h +++ b/chrome/browser/ui/cocoa/browser_window_controller.h @@ -460,6 +460,9 @@ class TabContents; // |source| rect doesn't fit into |target|. - (NSSize)overflowFrom:(NSRect)source to:(NSRect)target; + +// The fullscreen exit bubble controller, or nil if the bubble isn't showing. +- (FullscreenExitBubbleController*)fullscreenExitBubbleController; @end // @interface BrowserWindowController (TestingAPI) diff --git a/chrome/browser/ui/cocoa/browser_window_controller.mm b/chrome/browser/ui/cocoa/browser_window_controller.mm index f5fbb11..31db810 100644 --- a/chrome/browser/ui/cocoa/browser_window_controller.mm +++ b/chrome/browser/ui/cocoa/browser_window_controller.mm @@ -1852,6 +1852,11 @@ willAnimateFromState:(bookmarks::VisualState)oldState return NSMakeSize(x, y); } +// (Private/TestingAPI) +- (FullscreenExitBubbleController*)fullscreenExitBubbleController { + return fullscreenExitBubbleController_.get(); +} + - (void)showInstant:(TabContents*)previewContents { [previewableContentsController_ showPreview:previewContents]; [self updateBookmarkBarVisibilityWithAnimation:NO]; diff --git a/chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm b/chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm index bdbfcd2..fade662 100644 --- a/chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm +++ b/chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm @@ -93,7 +93,6 @@ const float kHideDuration = 0.7; - (void)deny:(id)sender { DCHECK(fullscreen_bubble::ShowButtonsForType(bubbleType_)); browser_->OnDenyFullscreenPermission(bubbleType_); - [self hideSoon]; } - (void)showButtons:(BOOL)show { diff --git a/chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller_unittest.mm b/chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller_unittest.mm index 6a293e2..65e3e93 100644 --- a/chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller_unittest.mm @@ -4,7 +4,14 @@ #import "chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.h" -#include "chrome/browser/ui/cocoa/cocoa_test_helper.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_window.h" +#include "chrome/browser/ui/cocoa/browser_window_controller.h" +#include "chrome/browser/ui/cocoa/cocoa_profile_test.h" +#include "chrome/browser/tabs/tab_strip_model.h" +#include "chrome/common/chrome_notification_types.h" +#include "chrome/test/base/ui_test_utils.h" +#include "content/browser/site_instance.h" #include "testing/gtest_mac.h" #include "ui/base/models/accelerator_cocoa.h" @@ -29,14 +36,16 @@ } @end -class FullscreenExitBubbleControllerTest : public CocoaTest { +class FullscreenExitBubbleControllerTest : public CocoaProfileTest { public: virtual void SetUp() { - CocoaTest::SetUp(); + CocoaProfileTest::SetUp(); + ASSERT_TRUE(profile()); + site_instance_ = SiteInstance::CreateSiteInstance(profile()); controller_.reset( [[FullscreenExitBubbleController alloc] initWithOwner:nil - browser:nil + browser:browser() url:GURL() bubbleType:FEB_TYPE_BROWSER_FULLSCREEN_EXIT_INSTRUCTION]); EXPECT_TRUE([controller_ window]); @@ -45,12 +54,52 @@ class FullscreenExitBubbleControllerTest : public CocoaTest { virtual void TearDown() { [controller_ close]; controller_.reset(); - CocoaTest::TearDown(); + CocoaProfileTest::TearDown(); } + void AppendTabToStrip() { + TabContentsWrapper* tab_contents = Browser::TabContentsFactory( + profile(), site_instance_, MSG_ROUTING_NONE, + NULL, NULL); + browser()->tabstrip_model()->AppendTabContents( + tab_contents, /*foreground=*/true); + } + + scoped_refptr site_instance_; scoped_nsobject controller_; }; +TEST_F(FullscreenExitBubbleControllerTest, DenyExitsFullscreen) { + CreateBrowserWindow(); + AppendTabToStrip(); + TabContents* fullscreen_tab = browser()->GetSelectedTabContents(); + { + base::mac::ScopedNSAutoreleasePool pool; + ui_test_utils::WindowedNotificationObserver fullscreen_observer( + chrome::NOTIFICATION_FULLSCREEN_CHANGED, + content::NotificationService::AllSources()); + browser()->ToggleFullscreenModeForTab(fullscreen_tab, true); + fullscreen_observer.Wait(); + ASSERT_TRUE(browser()->window()->IsFullscreen()); + } + + NSWindow* window = browser()->window()->GetNativeHandle(); + BrowserWindowController* bwc = [BrowserWindowController + browserWindowControllerForWindow:window]; + FullscreenExitBubbleController* bubble = [bwc fullscreenExitBubbleController]; + ASSERT_TRUE(bubble); + { + ui_test_utils::WindowedNotificationObserver fullscreen_observer( + chrome::NOTIFICATION_FULLSCREEN_CHANGED, + content::NotificationService::AllSources()); + [bubble deny:nil]; + fullscreen_observer.Wait(); + } + EXPECT_FALSE([bwc fullscreenExitBubbleController]); + EXPECT_FALSE(browser()->window()->IsFullscreen()); + CloseBrowserWindow(); +} + TEST_F(FullscreenExitBubbleControllerTest, LabelWasReplaced) { EXPECT_FALSE([controller_ exitLabelPlaceholder]); EXPECT_TRUE([controller_ exitLabel]); -- cgit v1.1