Submitting change from http://codereview.chromium.org/276029/show

BUG=none
TEST=none


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

13 files changed, 389 insertions, 32 deletions

diff --git a/chrome/browser/extensions/extension_function_dispatcher.cc b/chrome/browser/extensions/extension_function_dispatcher.cc
index d7105cb..a3f8a4c 100644
--- a/chrome/browser/extensions/extension_function_dispatcher.cc
+++ b/chrome/browser/extensions/extension_function_dispatcher.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/extensions/extension_message_service.h"
 #include "chrome/browser/extensions/extension_page_actions_module.h"
 #include "chrome/browser/extensions/extension_page_actions_module_constants.h"
+#include "chrome/browser/extensions/extension_popup_api.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_tabs_module.h"
 #include "chrome/browser/extensions/extension_tabs_module_constants.h"
@@ -139,6 +140,9 @@ void FactoryRegistry::ResetFunctions() {
   // I18N.
   RegisterFunction<GetAcceptLanguagesFunction>();
 
+  // Popup API.
+  RegisterFunction<PopupShowFunction>();
+
   // Test.
   RegisterFunction<ExtensionTestPassFunction>();
   RegisterFunction<ExtensionTestFailFunction>();
diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc
index 2f94612..9435913 100644
--- a/chrome/browser/extensions/extension_host.cc
+++ b/chrome/browser/extensions/extension_host.cc
@@ -18,12 +18,16 @@
 #include "chrome/browser/dom_ui/dom_ui_factory.h"
 #include "chrome/browser/extensions/extension_message_service.h"
 #include "chrome/browser/extensions/extension_tabs_module.h"
+#include "chrome/browser/extensions/extension_popup_api.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/renderer_host/render_widget_host.h"
 #include "chrome/browser/renderer_host/render_widget_host_view.h"
 #include "chrome/browser/renderer_host/site_instance.h"
+#if defined(TOOLKIT_VIEWS)
+#include "chrome/browser/views/extensions/extension_popup.h"
+#endif
 #include "chrome/common/bindings_policy.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/notification_service.h"
@@ -111,14 +115,25 @@ ExtensionHost::ExtensionHost(Extension* extension, SiteInstance* site_instance,
       did_stop_loading_(false),
       document_element_available_(false),
       url_(url),
+#if defined(TOOLKIT_VIEWS)
+      child_popup_(NULL),
+#endif
       extension_host_type_(host_type) {
   render_view_host_ = new RenderViewHost(site_instance, this, MSG_ROUTING_NONE);
   render_view_host_->AllowBindings(BindingsPolicy::EXTENSION);
   if (enable_dom_automation_)
     render_view_host_->AllowBindings(BindingsPolicy::DOM_AUTOMATION);
+
+#if defined(TOOLKIT_VIEWS)
+  // Listen for view close requests, so that we can dismiss a hosted pop-up
+  // view, if necessary.
+  registrar_.Add(this, NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE,
+                 Source<Profile>(profile_));
+#endif
 }
 
 ExtensionHost::~ExtensionHost() {
+  DismissPopup();
   NotificationService::current()->Notify(
       NotificationType::EXTENSION_HOST_DESTROYED,
       Source<Profile>(profile_),
@@ -208,6 +223,15 @@ void ExtensionHost::Observe(NotificationType type,
     NavigateToURL(url_);
   } else if (type == NotificationType::BROWSER_THEME_CHANGED) {
     InsertThemeCSS();
+#if defined(TOOLKIT_VIEWS)
+  } else if (type == NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE) {
+    // If we aren't the host of the popup, then disregard the notification.
+    if (!child_popup_ ||
+        Details<ExtensionHost>(child_popup_->host()) != details)
+      return;
+
+    DismissPopup();
+#endif
   } else {
     NOTREACHED();
   }
@@ -294,6 +318,38 @@ void ExtensionHost::InsertThemeCSS() {
   render_view_host()->InsertCSSInWebFrame(L"", css, "ToolstripThemeCSS");
 }
 
+void ExtensionHost::DismissPopup() {
+#if defined(TOOLKIT_VIEWS)
+  if (child_popup_) {
+    child_popup_->Hide();
+    child_popup_->DetachFromBrowser();
+    delete child_popup_;
+    child_popup_ = NULL;
+
+    PopupEventRouter::OnPopupClosed(GetBrowser()->profile(),
+                                    view()->render_view_host()->routing_id());
+  }
+#endif
+}
+
+#if defined(TOOLKIT_VIEWS)
+void ExtensionHost::BubbleBrowserWindowMoved(BrowserBubble* bubble) {
+  DismissPopup();
+}
+void ExtensionHost::BubbleBrowserWindowClosing(BrowserBubble* bubble) {
+  DismissPopup();
+}
+
+void ExtensionHost::BubbleGotFocus(BrowserBubble* bubble) {
+}
+
+void ExtensionHost::BubbleLostFocus(BrowserBubble* bubble) {
+  // TODO(twiz):  Dismiss the pop-up upon loss of focus of the bubble, but not
+  // if the focus is transitioning to the host which owns the popup!
+  // DismissPopup();
+}
+#endif  // defined(TOOLKIT_VIEWS)
+
 void ExtensionHost::DidStopLoading() {
   bool notify = !did_stop_loading_;
   did_stop_loading_ = true;
@@ -325,6 +381,13 @@ void ExtensionHost::DocumentAvailableInMainFrame(RenderViewHost* rvh) {
     registrar_.Add(this, NotificationType::BROWSER_THEME_CHANGED,
                    NotificationService::AllSources());
   }
+
+  if (ViewType::EXTENSION_POPUP == GetRenderViewType()) {
+    NotificationService::current()->Notify(
+        NotificationType::EXTENSION_POPUP_VIEW_READY,
+        Source<Profile>(profile_),
+        Details<ExtensionHost>(this));
+  }
 }
 
 void ExtensionHost::RunJavaScriptMessage(const std::wstring& message,
diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h
index ea9660d..bf66579 100644
--- a/chrome/browser/extensions/extension_host.h
+++ b/chrome/browser/extensions/extension_host.h
@@ -12,6 +12,7 @@
 #include "chrome/browser/renderer_host/render_view_host_delegate.h"
 #include "chrome/browser/tab_contents/render_view_host_delegate_helper.h"
 #if defined(TOOLKIT_VIEWS)
+#include "chrome/browser/views/browser_bubble.h"
 #include "chrome/browser/views/extensions/extension_view.h"
 #elif defined(OS_LINUX)
 #include "chrome/browser/gtk/extension_view_gtk.h"
@@ -20,8 +21,12 @@
 #endif
 #include "chrome/common/notification_registrar.h"
 
+
 class Browser;
 class Extension;
+#if defined(TOOLKIT_VIEWS)
+class ExtensionPopup;
+#endif
 class ExtensionProcessManager;
 class RenderProcessHost;
 class RenderWidgetHost;
@@ -33,7 +38,11 @@ struct WebPreferences;
 // It handles setting up the renderer process, if needed, with special
 // privileges available to extensions.  It may have a view to be shown in the
 // in the browser UI, or it may be hidden.
-class ExtensionHost : public RenderViewHostDelegate,
+class ExtensionHost :  // NOLINT
+#if defined(TOOLKIT_VIEWS)
+                      public BrowserBubble::Delegate,
+#endif
+                      public RenderViewHostDelegate,
                       public RenderViewHostDelegate::View,
                       public ExtensionFunctionDispatcher::Delegate,
                       public NotificationObserver {
@@ -72,6 +81,14 @@ class ExtensionHost : public RenderViewHostDelegate,
   }
   Profile* profile() const { return profile_; }
 
+#if defined(TOOLKIT_VIEWS)
+  ExtensionPopup* child_popup() const { return child_popup_; }
+  void set_child_popup(ExtensionPopup* popup) { child_popup_ = popup; }
+#endif
+
+  // Dismiss the hosted pop-up, if one is present.
+  void DismissPopup();
+
   // Sets the the ViewType of this host (e.g. mole, toolstrip).
   void SetRenderViewType(ViewType::Type type);
 
@@ -89,6 +106,22 @@ class ExtensionHost : public RenderViewHostDelegate,
   // Insert the theme CSS for a toolstrip/mole.
   void InsertThemeCSS();
 
+#if defined(TOOLKIT_VIEWS)
+  // BrowserBubble::Delegate implementation.
+  // Called when the Browser Window that this bubble is attached to moves.
+  virtual void BubbleBrowserWindowMoved(BrowserBubble* bubble);
+
+  // Called with the Browser Window that this bubble is attached to is
+  // about to close.
+  virtual void BubbleBrowserWindowClosing(BrowserBubble* bubble);
+
+  // Called when the bubble became active / got focus.
+  virtual void BubbleGotFocus(BrowserBubble* bubble);
+
+  // Called when the bubble became inactive / lost focus.
+  virtual void BubbleLostFocus(BrowserBubble* bubble);
+#endif  // defined(TOOLKIT_VIEWS)
+
   // RenderViewHostDelegate implementation.
   virtual RenderViewHostDelegate::View* GetViewDelegate();
   virtual const GURL& GetURL() const { return url_; }
@@ -192,12 +225,18 @@ class ExtensionHost : public RenderViewHostDelegate,
   // The URL being hosted.
   GURL url_;
 
+#if defined(TOOLKIT_VIEWS)
+  // A popup view that is anchored to and owned by this ExtensionHost.  However,
+  // the popup contains its own separate ExtensionHost
+  ExtensionPopup* child_popup_;
+#endif
+
   NotificationRegistrar registrar_;
 
   scoped_ptr<ExtensionFunctionDispatcher> extension_function_dispatcher_;
 
-  // Only EXTENSION_TOOLSTRIP and EXTENSION_BACKGROUND_PAGE are used here,
-  // others are not hostd by ExtensionHost.
+  // Only EXTENSION_TOOLSTRIP, EXTENSION_POPUP, and EXTENSION_BACKGROUND_PAGE
+  // are used here, others are not hosted by ExtensionHost.
   ViewType::Type extension_host_type_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionHost);
diff --git a/chrome/browser/extensions/extension_popup_api.cc b/chrome/browser/extensions/extension_popup_api.cc
new file mode 100644
index 0000000..3059079
--- /dev/null
+++ b/chrome/browser/extensions/extension_popup_api.cc
@@ -0,0 +1,164 @@
+// Copyright (c) 2009 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.
+
+#include "chrome/browser/extensions/extension_popup_api.h"
+
+#include "base/gfx/point.h"
+#include "base/json/json_writer.h"
+#include "base/string_util.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/notification_details.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_source.h"
+#include "chrome/common/notification_type.h"
+#include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/extensions/extension_message_service.h"
+#include "chrome/browser/browser.h"
+#include "chrome/browser/profile.h"
+#if defined(TOOLKIT_VIEWS)
+#include "chrome/browser/views/extensions/extension_popup.h"
+#include "views/view.h"
+#endif
+
+namespace extension_popup_module_events {
+
+const char kOnPopupClosed[] = "experimental.popup.onClosed.%d";
+
+}  // namespace extension_popup_module_events
+
+namespace {
+
+// Errors.
+const char kBadAnchorArgument[] = "Invalid anchor argument.";
+const char kInvalidURLError[] = "Invalid URL.";
+
+// Keys.
+const wchar_t kUrlKey[] = L"url";
+const wchar_t kWidthKey[] = L"width";
+const wchar_t kHeightKey[] = L"height";
+const wchar_t kTopKey[] = L"top";
+const wchar_t kLeftKey[] = L"left";
+
+};  // namespace
+
+PopupShowFunction::PopupShowFunction()
+#if defined (TOOLKIT_VIEWS)
+    : popup_(NULL)
+#endif
+{}
+
+void PopupShowFunction::Run() {
+#if defined(TOOLKIT_VIEWS)
+  if (!RunImpl()) {
+    SendResponse(false);
+  } else {
+    // If the contents of the popup are already available, then immediately
+    // send the response.  Otherwise wait for the EXTENSION_POPUP_VIEW_READY
+    // notification.
+    if (popup_->host() && popup_->host()->document_element_available()) {
+      SendResponse(true);
+    } else {
+      AddRef();
+      registrar_.Add(this, NotificationType::EXTENSION_POPUP_VIEW_READY,
+                     NotificationService::AllSources());
+      registrar_.Add(this, NotificationType::EXTENSION_HOST_DESTROYED,
+                     NotificationService::AllSources());
+    }
+  }
+#else
+  SendResponse(false);
+#endif
+}
+
+bool PopupShowFunction::RunImpl() {
+  EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_LIST));
+  const ListValue* args = static_cast<const ListValue*>(args_);
+
+  DictionaryValue* popup_info = NULL;
+  EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(0, &popup_info));
+
+  std::string url_string;
+  EXTENSION_FUNCTION_VALIDATE(popup_info->GetString(kUrlKey,
+                                                    &url_string));
+
+  DictionaryValue* dom_anchor = NULL;
+  EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(1, &dom_anchor));
+
+  int dom_top, dom_left;
+  EXTENSION_FUNCTION_VALIDATE(dom_anchor->GetInteger(kTopKey,
+                                                     &dom_top));
+  EXTENSION_FUNCTION_VALIDATE(dom_anchor->GetInteger(kLeftKey,
+                                                     &dom_left));
+
+  int dom_width, dom_height;
+  EXTENSION_FUNCTION_VALIDATE(dom_anchor->GetInteger(kWidthKey,
+                                                     &dom_width));
+  EXTENSION_FUNCTION_VALIDATE(dom_anchor->GetInteger(kHeightKey,
+                                                     &dom_height));
+  EXTENSION_FUNCTION_VALIDATE(dom_top >= 0 && dom_left >= 0 &&
+                              dom_width >= 0 && dom_height >= 0);
+
+  GURL url = dispatcher()->url().Resolve(url_string);
+  if (!url.is_valid()) {
+    error_ = kInvalidURLError;
+    return false;
+  }
+
+  // Disallow non-extension requests, or requests outside of the requesting
+  // extension view's extension.
+  const std::string& extension_id = url.host();
+  if (extension_id != dispatcher()->GetExtension()->id() ||
+      !url.SchemeIs("chrome-extension")) {
+    error_ = kInvalidURLError;
+    return false;
+  }
+
+#if defined(TOOLKIT_VIEWS)
+  views::View* extension_view = dispatcher()->GetExtensionHost()->view();
+  gfx::Point origin(dom_left, dom_top);
+  views::View::ConvertPointToScreen(extension_view, &origin);
+  gfx::Rect rect(origin.x(), origin.y(), dom_width, dom_height);
+
+  popup_ = ExtensionPopup::Show(url, dispatcher()->GetBrowser(), rect,
+                                BubbleBorder::BOTTOM_LEFT);
+
+  dispatcher()->GetExtensionHost()->set_child_popup(popup_);
+  popup_->set_delegate(dispatcher()->GetExtensionHost());
+#endif
+  return true;
+}
+
+void PopupShowFunction::Observe(NotificationType type,
+                                const NotificationSource& source,
+                                const NotificationDetails& details) {
+#if defined(TOOLKIT_VIEWS)
+  DCHECK(type == NotificationType::EXTENSION_POPUP_VIEW_READY ||
+         type == NotificationType::EXTENSION_HOST_DESTROYED);
+  DCHECK(popup_ != NULL);
+
+  if (popup_ && type == NotificationType::EXTENSION_POPUP_VIEW_READY &&
+      Details<ExtensionHost>(popup_->host()) == details) {
+    SendResponse(true);
+    Release();  // Balanced in Run().
+  } else if (popup_ && type == NotificationType::EXTENSION_HOST_DESTROYED &&
+             Details<ExtensionHost>(popup_->host()) == details) {
+    // If the host was destroyed, then report failure, and release the remaining
+    // reference.
+    SendResponse(false);
+    Release();  // Balanced in Run().
+  }
+#endif
+}
+
+// static
+void PopupEventRouter::OnPopupClosed(Profile* profile,
+                                     int routing_id) {
+  std::string full_event_name = StringPrintf(
+      extension_popup_module_events::kOnPopupClosed,
+      routing_id);
+
+  profile->GetExtensionMessageService()->DispatchEventToRenderers(
+      full_event_name,
+      base::JSONWriter::kEmptyArray);
+}
diff --git a/chrome/browser/extensions/extension_popup_api.h b/chrome/browser/extensions/extension_popup_api.h
new file mode 100644
index 0000000..96d6d53
--- /dev/null
+++ b/chrome/browser/extensions/extension_popup_api.h
@@ -0,0 +1,50 @@
+// Copyright (c) 2009 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_EXTENSIONS_EXTENSION_POPUP_API_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_POPUP_API_H_
+
+#include "chrome/browser/extensions/extension_function.h"
+#include "chrome/common/notification_registrar.h"
+
+class Profile;
+class ExtensionPopup;
+
+// This extension function shows a pop-up extension view.  It is asynchronous
+// because the callback must be invoked only after the associated render
+// process/view has been created and fully initialized.
+class PopupShowFunction : public AsyncExtensionFunction,
+                          public NotificationObserver {
+ public:
+  PopupShowFunction();
+
+  virtual void Run();
+  virtual bool RunImpl();
+  DECLARE_EXTENSION_FUNCTION_NAME("experimental.popup.show")
+
+ private:
+  // NotificationObserver methods.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+  NotificationRegistrar registrar_;
+
+#if defined(TOOLKIT_VIEWS)
+  // The pop-up view created by this function, saved for access during
+  // event notification.  The pop-up is not owned by the PopupShowFunction
+  // instance.
+  ExtensionPopup* popup_;
+#endif
+};
+
+// Event router class for events related to the chrome.popup.* set of APIs.
+class PopupEventRouter {
+ public:
+  static void OnPopupClosed(Profile* profile,
+                            int routing_id);
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PopupEventRouter);
+};
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_POPUP_API_H_
diff --git a/chrome/browser/extensions/extension_popup_apitest.cc b/chrome/browser/extensions/extension_popup_apitest.cc
new file mode 100644
index 0000000..cfd8404
--- /dev/null
+++ b/chrome/browser/extensions/extension_popup_apitest.cc
@@ -0,0 +1,13 @@
+// Copyright (c) 2009 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.
+
+#include "base/command_line.h"
+#include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/common/chrome_switches.h"
+
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Popup) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableExperimentalExtensionApis);
+  ASSERT_TRUE(RunExtensionTest("popup_api")) << message_;
+}
diff --git a/chrome/browser/gtk/extension_view_gtk.h b/chrome/browser/gtk/extension_view_gtk.h
index 16b7892..2bb2923 100644
--- a/chrome/browser/gtk/extension_view_gtk.h
+++ b/chrome/browser/gtk/extension_view_gtk.h
@@ -38,9 +38,9 @@ class ExtensionViewGtk {
   // connection.
   void RenderViewCreated();
 
- private:
   RenderViewHost* render_view_host() const;
 
+ private:
   void CreateWidgetHostView();
 
   // True if the contents are being displayed inside the extension shelf.
diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc
index 92e541a..eb7b2e4 100644
--- a/chrome/browser/views/browser_actions_container.cc
+++ b/chrome/browser/views/browser_actions_container.cc
@@ -275,7 +275,7 @@ void BrowserActionsContainer::AddBrowserAction(Extension* extension) {
   BrowserActionView* view = new BrowserActionView(extension, this);
   browser_action_views_.push_back(view);
   AddChildView(view);
-  if (GetParent()) 
+  if (GetParent())
     GetParent()->SchedulePaint();
 }
 
@@ -289,7 +289,7 @@ void BrowserActionsContainer::RemoveBrowserAction(Extension* extension) {
     if ((*iter)->button()->extension() == extension) {
       RemoveChildView(*iter);
       browser_action_views_.erase(iter);
-      if (GetParent()) 
+      if (GetParent())
         GetParent()->SchedulePaint();
       return;
     }
@@ -362,7 +362,8 @@ void BrowserActionsContainer::OnBrowserActionExecuted(
     rect.set_y(origin.y());
     popup_ = ExtensionPopup::Show(browser_action->popup_url(),
                                   toolbar_->browser(),
-                                  rect);
+                                  rect,
+                                  BubbleBorder::TOP_RIGHT);
     popup_->set_delegate(this);
     popup_button_ = button;
     popup_button_->PopupDidShow();
@@ -414,7 +415,8 @@ void BrowserActionsContainer::Observe(NotificationType type,
       break;
 
     case NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE:
-      if (Details<ExtensionHost>(popup_->host()) != details)
+      // If we aren't the host of the popup, then disregard the notification.
+      if (!popup_ || Details<ExtensionHost>(popup_->host()) != details)
         return;
 
       HidePopup();
diff --git a/chrome/browser/views/extensions/extension_popup.cc b/chrome/browser/views/extensions/extension_popup.cc
index a84176e..0be410c 100644
--- a/chrome/browser/views/extensions/extension_popup.cc
+++ b/chrome/browser/views/extensions/extension_popup.cc
@@ -28,12 +28,16 @@ const int ExtensionPopup::kMaxHeight = 600;
 
 ExtensionPopup::ExtensionPopup(ExtensionHost* host,
                                Widget* frame,
-                               const gfx::Rect& relative_to)
-    : BrowserBubble(host->view(), frame, gfx::Point()),
+                               const gfx::Rect& relative_to,
+                               BubbleBorder::ArrowLocation arrow_location)
+    : 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,
+  registrar_.Add(this,
+                 NotificationType::EXTENSION_HOST_DID_STOP_LOADING,
                  Source<Profile>(host->profile()));
 
   // TODO(erikkay) Some of this border code is derived from InfoBubble.
@@ -44,8 +48,10 @@ ExtensionPopup::ExtensionPopup(ExtensionHost* host,
                                              Widget::DeleteOnDestroy);
   gfx::NativeView native_window = frame->GetNativeView();
   border_widget_->Init(native_window, bounds());
+
   border_ = new BubbleBorder;
-  border_->set_arrow_location(BubbleBorder::TOP_RIGHT);
+  border_->set_arrow_location(arrow_location);
+
   border_view_ = new views::View;
   border_view_->set_background(new BubbleBackground(border_));
   border_view_->set_border(border_);
@@ -129,8 +135,10 @@ void ExtensionPopup::OnExtensionPreferredSizeChanged(ExtensionView* view) {
 }
 
 // static
-ExtensionPopup* ExtensionPopup::Show(const GURL& url, Browser* browser,
-                                     const gfx::Rect& relative_to) {
+ExtensionPopup* ExtensionPopup::Show(
+    const GURL& url, Browser* browser,
+    const gfx::Rect& relative_to,
+    BubbleBorder::ArrowLocation arrow_location) {
   ExtensionProcessManager* manager =
       browser->profile()->GetExtensionProcessManager();
   DCHECK(manager);
@@ -140,7 +148,8 @@ ExtensionPopup* ExtensionPopup::Show(const GURL& url, Browser* browser,
   ExtensionHost* host = manager->CreatePopup(url, browser);
   views::Widget* frame = BrowserView::GetBrowserViewForNativeWindow(
       browser->window()->GetNativeHandle())->GetWidget();
-  ExtensionPopup* popup = new ExtensionPopup(host, frame, relative_to);
+  ExtensionPopup* popup = new ExtensionPopup(host, frame, relative_to,
+                                             arrow_location);
 
   // 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 6ab880d..7db472a 100644
--- a/chrome/browser/views/extensions/extension_popup.h
+++ b/chrome/browser/views/extensions/extension_popup.h
@@ -7,6 +7,7 @@
 
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/views/browser_bubble.h"
+#include "chrome/browser/views/extensions/extension_view.h"
 #include "chrome/browser/views/bubble_border.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
@@ -21,14 +22,19 @@ class ExtensionPopup : public BrowserBubble,
  public:
   virtual ~ExtensionPopup();
 
-  // Create and show a popup with |url| positioned below |relative_to| in
-  // screen coordinates. This is anchored to the lower middle of the rect,
-  // extending to the left, just like the wrench and page menus.
+  // Create and show a popup with |url| positioned adjacent to |relative_to| in
+  // screen coordinates.
+  // The positioning of the pop-up is determined by |arrow_location| according
+  // to the following logic:  The popup is anchored so that the corner indicated
+  // by value of |arrow_location| remains fixed during popup resizes.
+  // If |arrow_location| is BOTTOM_*, then the popup 'pops up', otherwise
+  // the popup 'drops down'.
   //
   // 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);
+                              const gfx::Rect& relative_to,
+                              BubbleBorder::ArrowLocation arrow_location);
 
   ExtensionHost* host() const { return extension_host_.get(); }
 
@@ -43,8 +49,8 @@ class ExtensionPopup : public BrowserBubble,
                        const NotificationDetails& details);
 
   // ExtensionView::Container overrides.
-  virtual void OnExtensionMouseEvent(ExtensionView* view) { };
-  virtual void OnExtensionMouseLeave(ExtensionView* view) { };
+  virtual void OnExtensionMouseEvent(ExtensionView* view) { }
+  virtual void OnExtensionMouseLeave(ExtensionView* view) { }
   virtual void OnExtensionPreferredSizeChanged(ExtensionView* view);
 
   // The min/max height of popups.
@@ -56,7 +62,8 @@ class ExtensionPopup : public BrowserBubble,
  private:
   ExtensionPopup(ExtensionHost* host,
                  views::Widget* frame,
-                 const gfx::Rect& relative_to);
+                 const gfx::Rect& relative_to,
+                 BubbleBorder::ArrowLocation);
 
   // The area on the screen that the popup should be positioned relative to.
   gfx::Rect relative_to_;
@@ -77,4 +84,4 @@ class ExtensionPopup : public BrowserBubble,
   DISALLOW_COPY_AND_ASSIGN(ExtensionPopup);
 };
 
-#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_POPUP_H_
+#endif  // CHROME_BROWSER_VIEWS_EXTENSIONS_EXTENSION_POPUP_H_
diff --git a/chrome/browser/views/extensions/extension_view.cc b/chrome/browser/views/extensions/extension_view.cc
index 1d0cfc5..059d391 100644
--- a/chrome/browser/views/extensions/extension_view.cc
+++ b/chrome/browser/views/extensions/extension_view.cc
@@ -155,6 +155,13 @@ void ExtensionView::ViewHierarchyChanged(bool is_add,
     CreateWidgetHostView();
 }
 
+void ExtensionView::PreferredSizeChanged() {
+  View::PreferredSizeChanged();
+  if (container_) {
+    container_->OnExtensionPreferredSizeChanged(this);
+  }
+}
+
 void ExtensionView::HandleMouseEvent() {
   if (container_)
     container_->OnExtensionMouseEvent(this);
@@ -171,9 +178,3 @@ 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 8227771..bd63d73 100644
--- a/chrome/browser/views/extensions/extension_view.h
+++ b/chrome/browser/views/extensions/extension_view.h
@@ -27,6 +27,7 @@ class ExtensionView : public views::NativeViewHost {
   // (bottom shelf, side bar, etc.)
   class Container {
    public:
+    virtual ~Container() {}
     // Mouse event notifications from the view. (useful for hover UI).
     virtual void OnExtensionMouseEvent(ExtensionView* view) = 0;
     virtual void OnExtensionMouseLeave(ExtensionView* view) = 0;
@@ -61,7 +62,10 @@ 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);
+
+ protected:
+  // Overridden from views::View.
+  virtual void PreferredSizeChanged();
 
  private:
   friend class ExtensionHost;
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index edbe5f2..f159d7e 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -208,7 +208,7 @@ void LocationBarView::Init() {
 #else
                                location_entry_->widget()
 #endif
-                               );
+                               );  // NOLINT
 
   AddChildView(&selected_keyword_view_);
   selected_keyword_view_.SetFont(font_);
@@ -1304,7 +1304,8 @@ void LocationBarView::PageActionImageView::ExecuteAction(int button) {
     Browser* browser = BrowserList::GetLastActiveWithProfile(profile_);
     if (!browser)
       browser = BrowserList::FindBrowserWithProfile(profile_);
-    ExtensionPopup::Show(page_action_->popup_url(), browser, rect);
+    ExtensionPopup::Show(page_action_->popup_url(), browser, rect,
+                         BubbleBorder::TOP_RIGHT);
   } else {
     ExtensionBrowserEventRouter::GetInstance()->PageActionExecuted(
         profile_, page_action_->extension_id(), page_action_->id(),