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

chrome/browser/ui/extensions/extension_installed_bubble.h

Index: chrome/browser/ui/extensions/extension_installed_bubble.h
diff --git a/chrome/browser/ui/views/extensions/extension_installed_bubble.h b/chrome/browser/ui/extensions/extension_installed_bubble.h
similarity index 54%
copy from chrome/browser/ui/views/extensions/extension_installed_bubble.h
copy to chrome/browser/ui/extensions/extension_installed_bubble.h
index 5632209eb49bc4d7e6a6ff2f6583215bacd2e238..dccb7a7b8abe3ed628587381a7db8b983e712b05 100644
--- a/chrome/browser/ui/views/extensions/extension_installed_bubble.h
+++ b/chrome/browser/ui/extensions/extension_installed_bubble.h
@@ -1,15 +1,14 @@
-// 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;
 
@@ -21,60 +20,59 @@ class Extension;
 // 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.

-  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.

+  content::NotificationRegistrar registrar_;
 
-  // 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_;
@@ -82,4 +80,4 @@ class ExtensionInstalledBubble
   DISALLOW_COPY_AND_ASSIGN(ExtensionInstalledBubble);
 };
 
-#endif  // CHROME_BROWSER_UI_VIEWS_EXTENSIONS_EXTENSION_INSTALLED_BUBBLE_H_
+#endif  // CHROME_BROWSER_UI_EXTENSIONS_EXTENSION_INSTALLED_BUBBLE_H_