diff options
author | shess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-06 19:14:45 +0000 |
---|---|---|
committer | shess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-06 19:14:45 +0000 |
commit | 2391c42b05c58cd8e00a541c4b6c856f00d1a119 (patch) | |
tree | e838517b6929cf117525b8b998c061b95ce370c2 | |
parent | 062968e1ceec44a3cc3c11562463d9dcbe6f49ce (diff) | |
download | chromium_src-2391c42b05c58cd8e00a541c4b6c856f00d1a119.zip chromium_src-2391c42b05c58cd8e00a541c4b6c856f00d1a119.tar.gz chromium_src-2391c42b05c58cd8e00a541c4b6c856f00d1a119.tar.bz2 |
[Mac] Tear down extension-installed bubble before main window closes.
The main window closed before the bubble, so when the bubble tried to
remove itself as a child window, it called a stale reference.
Additionally, revise a couple uses of this pattern to be more robust
by asking the window being closed to remove itself from its current
parent window (which would be nil in this case).
BUG=94172
TEST=see bug, monitor crash server.
Review URL: http://codereview.chromium.org/7740060
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@99784 0039d316-1c4b-4281-b951-d872f2087c98
4 files changed, 51 insertions, 11 deletions
diff --git a/chrome/browser/ui/cocoa/base_bubble_controller.mm b/chrome/browser/ui/cocoa/base_bubble_controller.mm index 73c5911..808ec8e 100644 --- a/chrome/browser/ui/cocoa/base_bubble_controller.mm +++ b/chrome/browser/ui/cocoa/base_bubble_controller.mm @@ -157,7 +157,7 @@ class Bridge : public NotificationObserver { } - (void)close { - [parentWindow_ removeChildWindow:[self window]]; + [[[self window] parentWindow] removeChildWindow:[self window]]; [super close]; } diff --git a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm index 6099ccf..e0df44e 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm @@ -107,6 +107,13 @@ class ExtensionLoadedNotificationObserver : public NotificationObserver { // Start showing window only after extension has fully loaded. extensionObserver_.reset(new ExtensionLoadedNotificationObserver( self, browser->profile())); + + // Watch to see if the parent window closes, and if so, close this one. + NSNotificationCenter* center = [NSNotificationCenter defaultCenter]; + [center addObserver:self + selector:@selector(parentWindowWillClose:) + name:NSWindowWillCloseNotification + object:parentWindow_]; } return self; } @@ -117,10 +124,14 @@ class ExtensionLoadedNotificationObserver : public NotificationObserver { } - (void)close { - [parentWindow_ removeChildWindow:[self window]]; + [[[self window] parentWindow] removeChildWindow:[self window]]; [super close]; } +- (void)parentWindowWillClose:(NSNotification*)notification { + [self close]; +} + - (void)windowWillClose:(NSNotification*)notification { // Turn off page action icon preview when the window closes, unless we // already removed it when the window resigned key status. diff --git a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller_unittest.mm b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller_unittest.mm index a6ccdcb..697ef5d 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller_unittest.mm @@ -14,9 +14,12 @@ #import "chrome/browser/ui/cocoa/browser_test_helper.h" #import "chrome/browser/ui/cocoa/cocoa_test_helper.h" #import "chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h" +#import "chrome/browser/ui/cocoa/info_bubble_window.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/extension_constants.h" +#include "third_party/ocmock/gtest_support.h" +#import "third_party/ocmock/OCMock/OCMock.h" #include "webkit/glue/image_decoder.h" // ExtensionInstalledBubbleController with removePageActionPreview overridden @@ -200,3 +203,37 @@ TEST_F(ExtensionInstalledBubbleControllerTest, BrowserActionTest) { [controller close]; } + +TEST_F(ExtensionInstalledBubbleControllerTest, ParentClose) { + extension_ = CreateExtension(extension_installed_bubble::kBrowserAction); + ExtensionInstalledBubbleControllerForTest* controller = + [[ExtensionInstalledBubbleControllerForTest alloc] + initWithParentWindow:window_ + extension:extension_.get() + browser:browser_ + icon:icon_]; + EXPECT_TRUE(controller); + + // Bring up the window and disable close animation. + [controller showWindow:nil]; + NSWindow* bubbleWindow = [controller window]; + ASSERT_TRUE([bubbleWindow isKindOfClass:[InfoBubbleWindow class]]); + [static_cast<InfoBubbleWindow*>(bubbleWindow) setDelayOnClose:NO]; + + // Observe whether the bubble window closes. + NSString* notification = NSWindowWillCloseNotification; + id observer = [OCMockObject observerMock]; + [[observer expect] notificationWithName:notification object:bubbleWindow]; + [[NSNotificationCenter defaultCenter] + addMockObserver:observer name:notification object:bubbleWindow]; + + // The bubble window goes from visible to not-visible. + EXPECT_TRUE([bubbleWindow isVisible]); + [window_ close]; + EXPECT_FALSE([bubbleWindow isVisible]); + + [[NSNotificationCenter defaultCenter] removeObserver:observer]; + + // Check that the appropriate notification was received. + EXPECT_OCMOCK_VERIFY(observer); +} diff --git a/chrome/browser/ui/cocoa/extensions/extension_popup_controller.mm b/chrome/browser/ui/cocoa/extensions/extension_popup_controller.mm index d5e22c3..a42aa3d 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_popup_controller.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_popup_controller.mm @@ -171,15 +171,7 @@ class DevtoolsNotificationBridge : public NotificationObserver { } - (void)close { - [parentWindow_ removeChildWindow:[self window]]; - - // No longer have a parent window, so nil out the pointer and deregister for - // notifications. - NSNotificationCenter* center = [NSNotificationCenter defaultCenter]; - [center removeObserver:self - name:NSWindowWillCloseNotification - object:parentWindow_]; - parentWindow_ = nil; + [[[self window] parentWindow] removeChildWindow:[self window]]; [super close]; } |