Refactor out duplicate code from GTK/Views extension installed bubble.

chrome/browser/ui/extensions/extension_installed_bubble.h

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_UI_VIEWS_EXTENSIONS_EXTENSION_INSTALLED_BUBBLE_H_
-#define CHROME_BROWSER_UI_VIEWS_EXTENSIONS_EXTENSION_INSTALLED_BUBBLE_H_
+#ifndef CHROME_BROWSER_UI_EXTENSIONS_EXTENSION_INSTALLED_BUBBLE_H_
+#define CHROME_BROWSER_UI_EXTENSIONS_EXTENSION_INSTALLED_BUBBLE_H_
 
 #include "base/memory/weak_ptr.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "third_party/skia/include/core/SkBitmap.h"
-#include "ui/views/bubble/bubble_delegate.h"
 
 class Browser;
 
 namespace extensions {
 class Extension;
 }
 
 // Provides feedback to the user upon successful installation of an
 // extension. Depending on the type of extension, the Bubble will
 // point to:
 //    OMNIBOX_KEYWORD-> The omnibox.
-//    BROWSER_ACTION -> The browserAction icon in the toolbar.
-//    PAGE_ACTION    -> A preview of the pageAction icon in the location
+//    BROWSER_ACTION -> The browser action icon in the toolbar.
+//    PAGE_ACTION    -> A preview of the page action icon in the location
 //                      bar which is shown while the Bubble is shown.
-//    GENERIC        -> The wrench menu. This case includes pageActions that
+//    GENERIC        -> The wrench menu. This case includes page actions that
 //                      don't specify a default icon.
-class ExtensionInstalledBubble
-    : public views::BubbleDelegateView,
-      public content::NotificationObserver {
+//
+// ExtensionInstallBubble manages its own lifetime.
+class ExtensionInstalledBubble : public content::NotificationObserver {
  public:
+  virtual ~ExtensionInstalledBubble();
+
   // The behavior and content of this Bubble comes in these varieties:
   enum BubbleType {

[sky, 2013/08/29 15:39:51]

nit: enums before constructor.

[Yoyo Zhou, 2013/08/29 21:44:46]

Done.

-    OMNIBOX_KEYWORD = 0,
+    OMNIBOX_KEYWORD,
     BROWSER_ACTION,
     PAGE_ACTION,
-    GENERIC,
+    GENERIC
   };
 
-  // Creates the ExtensionInstalledBubble and schedules it to be shown once
-  // the extension has loaded. |extension| is the installed extension. |browser|
-  // is the browser window which will host the bubble. |icon| is the install
-  // icon of the extension.
-  static void Show(const extensions::Extension* extension,
-                   Browser *browser,
-                   const SkBitmap& icon);
-
- private:
-  // Private ctor. Registers a listener for EXTENSION_LOADED.
+ protected:
+  // Protected ctor. Registers a listener for EXTENSION_LOADED.
   ExtensionInstalledBubble(const extensions::Extension* extension,
                            Browser *browser,
                            const SkBitmap& icon);
 
-  virtual ~ExtensionInstalledBubble();
+  const extensions::Extension* extension() { return extension_; }
+  Browser* browser() { return browser_; }
+  const SkBitmap& icon() { return icon_; }
+  BubbleType type() { return type_; }
+  content::NotificationRegistrar* registrar() { return &registrar_; }
 
+  // When showing the bubble for a new browser action, we may have to wait for
+  // the toolbar to finish animating to know where the item's final position
+  // will be.
+  void MaybeShowLater();
+
+ private:
   // Shows the bubble. Called internally via PostTask.
-  void ShowInternal();
+  virtual void ShowInternal() = 0;

[sky, 2013/08/29 15:39:51]

This pattern is leading to multiple inheritance of non pure virtual types,
something against the style guide. Maybe this class should be concrete but have
an ExtensionInstalledBubbleView or something that is pure.

[Yoyo Zhou, 2013/08/29 21:44:46]

I added a View delegate. This added a bit of complexity.

 
-  // content::NotificationObserver
+  // content::NotificationObserver:
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
-  // views::WidgetDelegate
-  virtual void WindowClosing() OVERRIDE;
-
-  // views::BubbleDelegate
-  virtual gfx::Rect GetAnchorRect() OVERRIDE;
-
   const extensions::Extension* extension_;
   Browser* browser_;
   SkBitmap icon_;

[sky, 2013/08/29 15:39:51]

const

[Yoyo Zhou, 2013/08/29 21:44:46]

Done.

+  BubbleType type_;
   content::NotificationRegistrar registrar_;
-  BubbleType type_;

[sky, 2013/08/29 15:39:51]

const

[Yoyo Zhou, 2013/08/29 21:44:46]

I think the type determination is too complex to put in the initializer list.

 
-  // How many times we've deferred due to animations being in progress.
+  // The number of times to retry showing the bubble if the browser action
+  // toolbar is animating.
   int animation_wait_retries_;
 
   base::WeakPtrFactory<ExtensionInstalledBubble> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionInstalledBubble);
 };
 
-#endif  // CHROME_BROWSER_UI_VIEWS_EXTENSIONS_EXTENSION_INSTALLED_BUBBLE_H_
+#endif  // CHROME_BROWSER_UI_EXTENSIONS_EXTENSION_INSTALLED_BUBBLE_H_