diff options
author | cevans@chromium.org <cevans@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-02 02:10:56 +0000 |
---|---|---|
committer | cevans@chromium.org <cevans@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-02 02:10:56 +0000 |
commit | d5ba62903e21848106b4ae33343fe1b0ac08b68e (patch) | |
tree | c24cd8256a07d2fe9740e3dc4955a91013ee6982 | |
parent | ee0f47a04347de4aec093e3435493dd2ce502d63 (diff) | |
download | chromium_src-d5ba62903e21848106b4ae33343fe1b0ac08b68e.zip chromium_src-d5ba62903e21848106b4ae33343fe1b0ac08b68e.tar.gz chromium_src-d5ba62903e21848106b4ae33343fe1b0ac08b68e.tar.bz2 |
Merge 107841 - 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
TBR=jeremya@chromium.org
Review URL: http://codereview.chromium.org/8439030
git-svn-id: svn://svn.chromium.org/chrome/branches/912/src@108233 0039d316-1c4b-4281-b951-d872f2087c98
5 files changed, 64 insertions, 6 deletions
diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h index 8b32925..cd5f307 100644 --- a/chrome/browser/ui/browser.h +++ b/chrome/browser/ui/browser.h @@ -851,6 +851,8 @@ class Browser : public TabHandlerDelegate, FRIEND_TEST_ALL_PREFIXES(BrowserTest, TestTabExitsItselfFromFullscreen); FRIEND_TEST_ALL_PREFIXES(BrowserTest, TestFullscreenBubbleMouseLockState); FRIEND_TEST_ALL_PREFIXES(BrowserTest, TabEntersPresentationModeFromWindowed); + FRIEND_TEST_ALL_PREFIXES(FullscreenExitBubbleControllerTest, + DenyExitsFullscreen); FRIEND_TEST_ALL_PREFIXES(BrowserInitTest, OpenAppShortcutNoPref); FRIEND_TEST_ALL_PREFIXES(BrowserInitTest, OpenAppShortcutWindowPref); FRIEND_TEST_ALL_PREFIXES(BrowserInitTest, OpenAppShortcutTabPref); 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 7dda52f2..548d61f 100644 --- a/chrome/browser/ui/cocoa/browser_window_controller.mm +++ b/chrome/browser/ui/cocoa/browser_window_controller.mm @@ -1856,6 +1856,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<SiteInstance> site_instance_; scoped_nsobject<FullscreenExitBubbleController> 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]); |