Fix crasher in LocationBarView when browser is closed with page actio…

…n install bubble showing.

BUG= 34282
TEST= in release version, create a page action. close browser window while page action install bubble is showing. no crash, ever.
Review URL: http://codereview.chromium.org/561028

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@38225 0039d316-1c4b-4281-b951-d872f2087c98

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;

chrome/browser/cocoa/extension_installed_bubble_controller.mm

@@ -33,7 +33,6 @@
   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 @@ void Observe(NotificationType type,
 @implementation ExtensionInstalledBubbleController
 
 @synthesize extension = extension_;
+@synthesize pageActionRemoved = pageActionRemoved_;  // Exposed for unit test.
 
 - (id)initWithParentWindow:(NSWindow*)parentWindow
                  extension:(Extension*)extension
@@ -72,10 +72,14 @@ - (id)initWithParentWindow:(NSWindow*)parentWindow
       [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 @@ - (id)initWithParentWindow:(NSWindow*)parentWindow
 
     // 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 @@ - (void)close {
   [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 @@ - (void)windowDidResignKey:(NSNotification*)notification {
   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 @@ - (IBAction)closeWindow:(id)sender {
 
 // 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*>(

chrome/browser/cocoa/extension_installed_bubble_controller_unittest.mm

@@ -160,6 +160,7 @@ SkBitmap LoadTestIcon() {
             msg2Frame.origin.y + msg2Frame.size.height +
             extension_installed_bubble::kInnerVerticalMargin);
 
+  [controller setPageActionRemoved:YES];
   [controller close];
 }
 

chrome/browser/cocoa/location_bar_view_mac.mm

@@ -348,7 +348,8 @@
   if (!browser)
     return;
   TabContents* contents = browser->GetSelectedTabContents();
-  DCHECK(contents);
+  if (!contents)
+    return;
   page_action_views_->RefreshViews();
 
   LocationBarViewMac::PageActionImageView* page_action_image_view =