7 files changed, 95 insertions, 44 deletions

diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc
index 4ce1197..aac8caf 100644
--- a/chrome/browser/views/browser_actions_container.cc
+++ b/chrome/browser/views/browser_actions_container.cc
@@ -271,6 +271,8 @@ class BrowserActionView : public views::View {
   BrowserActionView(ExtensionAction* browser_action, Extension* extension,
                     BrowserActionsContainer* panel);
 
+  BrowserActionButton* button() { return button_; }
+
  private:
   virtual void Layout();
 
@@ -445,15 +447,30 @@ void BrowserActionsContainer::OnBrowserActionVisibilityChanged() {
 
 void BrowserActionsContainer::HidePopup() {
   if (popup_) {
-    popup_->DetachFromBrowser();
-    delete popup_;
+    // This sometimes gets called via a timer (See BubbleLostFocus), so clear
+    // the task factory. in case one is pending.
+    task_factory_.RevokeAll();
+
+    // Save these variables in local temporaries since destroying the popup
+    // calls BubbleLostFocus to be called, which will try to call HidePopup()
+    // again if popup_ is non-null.
+    ExtensionPopup* closing_popup = popup_;
+    BrowserActionButton* closing_button = popup_button_;
     popup_ = NULL;
-    popup_button_->PopupDidHide();
     popup_button_ = NULL;
+
+    closing_popup->DetachFromBrowser();
+    delete closing_popup;
+    closing_button->PopupDidHide();
     return;
   }
 }
 
+void BrowserActionsContainer::TestExecuteBrowserAction(int index) {
+  BrowserActionButton* button = browser_action_views_[index]->button();
+  OnBrowserActionExecuted(button);
+}
+
 void BrowserActionsContainer::OnBrowserActionExecuted(
     BrowserActionButton* button) {
   const ExtensionAction& browser_action = button->browser_action();
@@ -478,8 +495,7 @@ void BrowserActionsContainer::OnBrowserActionExecuted(
     rect.set_y(origin.y());
     popup_ = ExtensionPopup::Show(browser_action.popup_url(),
                                   toolbar_->browser(),
-                                  rect,
-                                  browser_action.popup_height());
+                                  rect);
     popup_->set_delegate(this);
     popup_button_ = button;
     popup_button_->PopupDidShow();
@@ -539,6 +555,9 @@ void BrowserActionsContainer::BubbleGotFocus(BrowserBubble* bubble) {
 }
 
 void BrowserActionsContainer::BubbleLostFocus(BrowserBubble* bubble) {
+  if (!popup_)
+    return;
+
   // This is a bit annoying.  If you click on the button that generated the
   // current popup, then we first get this lost focus message, and then
   // we get the click action.  This results in the popup being immediately
diff --git a/chrome/browser/views/browser_actions_container.h b/chrome/browser/views/browser_actions_container.h
index f3b3863..c849d44 100644
--- a/chrome/browser/views/browser_actions_container.h
+++ b/chrome/browser/views/browser_actions_container.h
@@ -69,11 +69,17 @@ class BrowserActionsContainer : public views::View,
   // by default irrespective of the available space to draw them.
   int GetClippedPreferredWidth(int available_width);
 
- private:
-
   // Hide the current popup.
   void HidePopup();
 
+  // Simulate a click on a browser action button.  This should only be
+  // used by unit tests.
+  void TestExecuteBrowserAction(int index);
+
+  // Retrieve the current popup.  This should only be used by unit tests.
+  ExtensionPopup* TestGetPopup() { return popup_; }
+
+ private:
   // The vector of browser actions (icons/image buttons for each action).
   std::vector<BrowserActionView*> browser_action_views_;
 
diff --git a/chrome/browser/views/extensions/extension_popup.cc b/chrome/browser/views/extensions/extension_popup.cc
index 0588664..53025e9 100644
--- a/chrome/browser/views/extensions/extension_popup.cc
+++ b/chrome/browser/views/extensions/extension_popup.cc
@@ -22,6 +22,7 @@ ExtensionPopup::ExtensionPopup(ExtensionHost* host,
     : BrowserBubble(host->view(), frame, gfx::Point()),
       relative_to_(relative_to),
       extension_host_(host) {
+  host->view()->SetContainer(this);
   registrar_.Add(this, NotificationType::EXTENSION_HOST_DID_STOP_LOADING,
                  Source<Profile>(host->profile()));
 
@@ -51,8 +52,19 @@ void ExtensionPopup::Hide() {
 }
 
 void ExtensionPopup::Show() {
+  if (visible())
+    return;
+
   ResizeToView();
 
+  // Show the border first, then the popup overlaid on top.
+  border_widget_->Show();
+  BrowserBubble::Show(true);
+}
+
+void ExtensionPopup::ResizeToView() {
+  BrowserBubble::ResizeToView();
+
   // The rounded corners cut off more of the view than the border insets claim.
   // Since we can't clip the ExtensionView's corners, we need to increase the
   // inset by half the corner radius as well as lying about the size of the
@@ -73,10 +85,6 @@ void ExtensionPopup::Show() {
   origin.set_x(origin.x() + border_insets.left() + corner_inset);
   origin.set_y(origin.y() + border_insets.top() + corner_inset);
   MoveTo(origin.x(), origin.y());
-
-  // Show the border first, then the popup overlaid on top.
-  border_widget_->Show();
-  BrowserBubble::Show(true);
 }
 
 void ExtensionPopup::Observe(NotificationType type,
@@ -92,10 +100,14 @@ void ExtensionPopup::Observe(NotificationType type,
   }
 }
 
+void ExtensionPopup::OnExtensionPreferredSizeChanged(ExtensionView* view) {
+  view->SizeToPreferredSize();
+  ResizeToView();
+}
+
 // static
 ExtensionPopup* ExtensionPopup::Show(const GURL& url, Browser* browser,
-                                     const gfx::Rect& relative_to,
-                                     int height) {
+                                     const gfx::Rect& relative_to) {
   ExtensionProcessManager* manager =
       browser->profile()->GetExtensionProcessManager();
   DCHECK(manager);
@@ -106,9 +118,6 @@ ExtensionPopup* ExtensionPopup::Show(const GURL& url, Browser* browser,
   views::Widget* frame = BrowserView::GetBrowserViewForNativeWindow(
       browser->window()->GetNativeHandle())->GetWidget();
   ExtensionPopup* popup = new ExtensionPopup(host, frame, relative_to);
-  gfx::Size sz = host->view()->GetPreferredSize();
-  sz.set_height(height);
-  host->view()->SetPreferredSize(sz);
 
   // If the host had somehow finished loading, then we'd miss the notification
   // and not show.  This seems to happen in single-process mode.
diff --git a/chrome/browser/views/extensions/extension_popup.h b/chrome/browser/views/extensions/extension_popup.h
index 1e2ff73..5915f59 100644
--- a/chrome/browser/views/extensions/extension_popup.h
+++ b/chrome/browser/views/extensions/extension_popup.h
@@ -16,7 +16,8 @@ class Browser;
 class ExtensionHost;
 
 class ExtensionPopup : public BrowserBubble,
-                       public NotificationObserver {
+                       public NotificationObserver,
+                       public ExtensionView::Container {
  public:
   virtual ~ExtensionPopup();
 
@@ -27,20 +28,25 @@ class ExtensionPopup : public BrowserBubble,
   // The actual display of the popup is delayed until the page contents
   // finish loading in order to minimize UI flashing and resizing.
   static ExtensionPopup* Show(const GURL& url, Browser* browser,
-                              const gfx::Rect& relative_to,
-                              int height);
+                              const gfx::Rect& relative_to);
 
   ExtensionHost* host() const { return extension_host_.get(); }
 
   // BrowserBubble overrides.
-  virtual void Show();
   virtual void Hide();
+  virtual void Show();
+  virtual void ResizeToView();
 
   // NotificationObserver overrides.
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
+  // ExtensionView::Container overrides.
+  virtual void OnExtensionMouseEvent(ExtensionView* view) { };
+  virtual void OnExtensionMouseLeave(ExtensionView* view) { };
+  virtual void OnExtensionPreferredSizeChanged(ExtensionView* view);
+
  private:
   ExtensionPopup(ExtensionHost* host,
                  views::Widget* frame,
diff --git a/chrome/browser/views/extensions/extension_shelf.h b/chrome/browser/views/extensions/extension_shelf.h
index 08de605..3be5791 100644
--- a/chrome/browser/views/extensions/extension_shelf.h
+++ b/chrome/browser/views/extensions/extension_shelf.h
@@ -22,7 +22,7 @@ namespace views {
 
 // A shelf that contains Extension toolstrips.
 class ExtensionShelf : public DetachableToolbarView,
-                       public ExtensionContainer,
+                       public ExtensionView::Container,
                        public ExtensionShelfModelObserver,
                        public AnimationDelegate,
                        public NotificationObserver {
diff --git a/chrome/browser/views/extensions/extension_view.cc b/chrome/browser/views/extensions/extension_view.cc
index f59d1a4..1d0cfc5 100644
--- a/chrome/browser/views/extensions/extension_view.cc
+++ b/chrome/browser/views/extensions/extension_view.cc
@@ -13,8 +13,10 @@
 #include "views/widget/widget.h"
 
 ExtensionView::ExtensionView(ExtensionHost* host, Browser* browser)
-    : host_(host), browser_(browser),
-      initialized_(false), pending_preferred_width_(0), container_(NULL),
+    : host_(host),
+      browser_(browser),
+      initialized_(false),
+      container_(NULL),
       is_clipped_(false) {
   host_->set_view(this);
 }
@@ -110,7 +112,8 @@ void ExtensionView::ShowIfCompletelyLoaded() {
       return;
     }
     SetVisible(true);
-    UpdatePreferredWidth(pending_preferred_width_);
+
+    UpdatePreferredSize(pending_preferred_size_);
   }
 }
 
@@ -131,17 +134,17 @@ void ExtensionView::SetBackground(const SkBitmap& background) {
   ShowIfCompletelyLoaded();
 }
 
-void ExtensionView::UpdatePreferredWidth(int pref_width) {
+void ExtensionView::UpdatePreferredSize(const gfx::Size& new_size) {
   // Don't actually do anything with this information until we have been shown.
   // Size changes will not be honored by lower layers while we are hidden.
-  gfx::Size preferred_size = GetPreferredSize();
   if (!IsVisible()) {
-    pending_preferred_width_ = pref_width;
-  } else if (pref_width > 0 && pref_width != preferred_size.width()) {
-    if (preferred_size.height() == 0)
-      preferred_size.set_height(height());
-    SetPreferredSize(gfx::Size(pref_width, preferred_size.height()));
+    pending_preferred_size_ = new_size;
+    return;
   }
+
+  gfx::Size preferred_size = GetPreferredSize();
+  if (new_size != preferred_size)
+    SetPreferredSize(new_size);
 }
 
 void ExtensionView::ViewHierarchyChanged(bool is_add,
@@ -168,3 +171,9 @@ void ExtensionView::RenderViewCreated() {
     pending_background_.reset();
   }
 }
+
+void ExtensionView::SetPreferredSize(const gfx::Size& size) {
+  views::NativeViewHost::SetPreferredSize(size);
+  if (container_)
+    container_->OnExtensionPreferredSizeChanged(this);
+}
diff --git a/chrome/browser/views/extensions/extension_view.h b/chrome/browser/views/extensions/extension_view.h
index 39b5851..8227771 100644
--- a/chrome/browser/views/extensions/extension_view.h
+++ b/chrome/browser/views/extensions/extension_view.h
@@ -17,21 +17,22 @@ class ExtensionHost;
 class ExtensionView;
 class RenderViewHost;
 
-// A class that represents the container that this view is in.
-// (bottom shelf, side bar, etc.)
-class ExtensionContainer {
- public:
-  // Mouse event notifications from the view. (useful for hover UI).
-  virtual void OnExtensionMouseEvent(ExtensionView* view) = 0;
-  virtual void OnExtensionMouseLeave(ExtensionView* view) = 0;
-};
-
 // This handles the display portion of an ExtensionHost.
 class ExtensionView : public views::NativeViewHost {
  public:
   ExtensionView(ExtensionHost* host, Browser* browser);
   ~ExtensionView();
 
+  // A class that represents the container that this view is in.
+  // (bottom shelf, side bar, etc.)
+  class Container {
+   public:
+    // Mouse event notifications from the view. (useful for hover UI).
+    virtual void OnExtensionMouseEvent(ExtensionView* view) = 0;
+    virtual void OnExtensionMouseLeave(ExtensionView* view) = 0;
+    virtual void OnExtensionPreferredSizeChanged(ExtensionView* view) {}
+  };
+
   ExtensionHost* host() const { return host_; }
   Browser* browser() const { return browser_; }
   Extension* extension() const;
@@ -40,7 +41,7 @@ class ExtensionView : public views::NativeViewHost {
   void SetIsClipped(bool is_clipped);
 
   // Notification from ExtensionHost.
-  void UpdatePreferredWidth(int pref_width);
+  void UpdatePreferredSize(const gfx::Size& new_size);
   void HandleMouseEvent();
   void HandleMouseLeave();
 
@@ -52,7 +53,7 @@ class ExtensionView : public views::NativeViewHost {
   void SetBackground(const SkBitmap& background);
 
   // Sets the container for this view.
-  void SetContainer(ExtensionContainer* container) { container_ = container; }
+  void SetContainer(Container* container) { container_ = container; }
 
   // Overridden from views::NativeViewHost:
   virtual void SetVisible(bool is_visible);
@@ -60,6 +61,7 @@ class ExtensionView : public views::NativeViewHost {
                                const gfx::Rect& current);
   virtual void ViewHierarchyChanged(bool is_add,
                                     views::View *parent, views::View *child);
+  virtual void SetPreferredSize(const gfx::Size& size);
 
  private:
   friend class ExtensionHost;
@@ -90,11 +92,11 @@ class ExtensionView : public views::NativeViewHost {
 
   // What we should set the preferred width to once the ExtensionView has
   // loaded.
-  int pending_preferred_width_;
+  gfx::Size pending_preferred_size_;
 
   // The container this view is in (not necessarily its direct superview).
   // Note: the view does not own its container.
-  ExtensionContainer* container_;
+  Container* container_;
 
   // Whether this extension view is clipped.
   bool is_clipped_;