diff options
4 files changed, 31 insertions, 19 deletions
diff --git a/chrome/browser/cocoa/extension_installed_bubble_controller.h b/chrome/browser/cocoa/extension_installed_bubble_controller.h index 880db3f..d6ef22e 100644 --- a/chrome/browser/cocoa/extension_installed_bubble_controller.h +++ b/chrome/browser/cocoa/extension_installed_bubble_controller.h @@ -52,6 +52,11 @@ typedef enum { extension_installed_bubble::ExtensionType type_; + // We need to remove the page action immediately when the browser window + // closes while this bubble is still open, so the bubble's closing animation + // doesn't overlap browser destruction. + BOOL pageActionRemoved_; + // Lets us register for EXTENSION_LOADED notifications. The actual // notifications are sent to the observer object, which proxies them // back to the controller. @@ -67,6 +72,7 @@ typedef enum { } @property (readonly) Extension* extension; +@property BOOL pageActionRemoved; // Initialize the window, and then create observers to wait for the extension // to complete loading, or the browser window to close. @@ -86,7 +92,7 @@ typedef enum { @interface ExtensionInstalledBubbleController(ExposedForTesting) -- (void)removePageActionPreview; +- (void)removePageActionPreviewIfNecessary; - (NSWindow*)initializeWindow; - (int)calculateWindowHeight; - (void)setMessageFrames:(int)newWindowHeight; diff --git a/chrome/browser/cocoa/extension_installed_bubble_controller.mm b/chrome/browser/cocoa/extension_installed_bubble_controller.mm index 9bce66f..f1d4761 100644 --- a/chrome/browser/cocoa/extension_installed_bubble_controller.mm +++ b/chrome/browser/cocoa/extension_installed_bubble_controller.mm @@ -33,7 +33,6 @@ class ExtensionLoadedNotificationObserver : public NotificationObserver { ExtensionLoadedNotificationObserver( ExtensionInstalledBubbleController* controller) : controller_(controller) { - // Create a registrar and add ourselves to it. registrar_.Add(this, NotificationType::EXTENSION_LOADED, NotificationService::AllSources()); } @@ -63,6 +62,7 @@ class ExtensionLoadedNotificationObserver : public NotificationObserver { @implementation ExtensionInstalledBubbleController @synthesize extension = extension_; +@synthesize pageActionRemoved = pageActionRemoved_; // Exposed for unit test. - (id)initWithParentWindow:(NSWindow*)parentWindow extension:(Extension*)extension @@ -72,10 +72,14 @@ class ExtensionLoadedNotificationObserver : public NotificationObserver { [mac_util::MainAppBundle() pathForResource:@"ExtensionInstalledBubble" ofType:@"nib"]; if ((self = [super initWithWindowNibPath:nibPath owner:self])) { + DCHECK(parentWindow); parentWindow_ = parentWindow; + DCHECK(extension); extension_ = extension; + DCHECK(browser); browser_ = browser; icon_.reset([gfx::SkBitmapToNSImage(icon) retain]); + pageActionRemoved_ = NO; if (extension->browser_action()) { type_ = extension_installed_bubble::kBrowserAction; @@ -88,13 +92,6 @@ class ExtensionLoadedNotificationObserver : public NotificationObserver { // Start showing window only after extension has fully loaded. extensionObserver_.reset(new ExtensionLoadedNotificationObserver(self)); - - // Watch to see if the parent window closes, and close this one if so. - NSNotificationCenter* center = [NSNotificationCenter defaultCenter]; - [center addObserver:self - selector:@selector(parentWindowWillClose:) - name:NSWindowWillCloseNotification - object:parentWindow_]; } return self; } @@ -109,15 +106,13 @@ class ExtensionLoadedNotificationObserver : public NotificationObserver { [super close]; } -- (void)parentWindowWillClose:(NSNotification*)notification { - [self close]; -} - - (void)windowWillClose:(NSNotification*)notification { - // Turn off page action icon preview when the window closes. - if (extension_->page_action()) { - [self removePageActionPreview]; - } + // Turn off page action icon preview when the window closes, unless we + // already removed it when the window resigned key status. + [self removePageActionPreviewIfNecessary]; + extension_ = NULL; + browser_ = NULL; + parentWindow_ = nil; // We caught a close so we don't need to watch for the parent closing. [[NSNotificationCenter defaultCenter] removeObserver:self]; [self autorelease]; @@ -129,6 +124,11 @@ class ExtensionLoadedNotificationObserver : public NotificationObserver { NSWindow* window = [self window]; DCHECK_EQ([notification object], window); DCHECK([window isVisible]); + + // If the browser window is closing, we need to remove the page action + // immediately, otherwise the closing animation may overlap with + // browser destruction. + [self removePageActionPreviewIfNecessary]; [self close]; } @@ -139,7 +139,11 @@ class ExtensionLoadedNotificationObserver : public NotificationObserver { // Extracted to a function here so that it can be overwritten for unit // testing. -- (void)removePageActionPreview { +- (void)removePageActionPreviewIfNecessary { + DCHECK(extension_); + if (!extension_->page_action() || pageActionRemoved_) + return; + pageActionRemoved_ = YES; BrowserWindowCocoa* window = static_cast<BrowserWindowCocoa*>( browser_->window()); LocationBarViewMac* locationBarView = static_cast<LocationBarViewMac*>( diff --git a/chrome/browser/cocoa/extension_installed_bubble_controller_unittest.mm b/chrome/browser/cocoa/extension_installed_bubble_controller_unittest.mm index 6cf27c9..a8a78be 100644 --- a/chrome/browser/cocoa/extension_installed_bubble_controller_unittest.mm +++ b/chrome/browser/cocoa/extension_installed_bubble_controller_unittest.mm @@ -160,6 +160,7 @@ TEST_F(ExtensionInstalledBubbleControllerTest, PageActionTest) { msg2Frame.origin.y + msg2Frame.size.height + extension_installed_bubble::kInnerVerticalMargin); + [controller setPageActionRemoved:YES]; [controller close]; } diff --git a/chrome/browser/cocoa/location_bar_view_mac.mm b/chrome/browser/cocoa/location_bar_view_mac.mm index 6d30820..0247576 100644 --- a/chrome/browser/cocoa/location_bar_view_mac.mm +++ b/chrome/browser/cocoa/location_bar_view_mac.mm @@ -348,7 +348,8 @@ void LocationBarViewMac::SetPreviewEnabledPageAction( if (!browser) return; TabContents* contents = browser->GetSelectedTabContents(); - DCHECK(contents); + if (!contents) + return; page_action_views_->RefreshViews(); LocationBarViewMac::PageActionImageView* page_action_image_view = |