Create ExtensionActionView class

chrome/browser/ui/views/location_bar/page_action_image_view.cc

Index: chrome/browser/ui/views/location_bar/page_action_image_view.cc
diff --git a/chrome/browser/ui/views/location_bar/page_action_image_view.cc b/chrome/browser/ui/views/location_bar/page_action_image_view.cc
index 3ab5e7f317e767c730c505bc33557678661e0114..307a088d6c54cf5ab892f43413597bc747fdba5f 100644
--- a/chrome/browser/ui/views/location_bar/page_action_image_view.cc
+++ b/chrome/browser/ui/views/location_bar/page_action_image_view.cc
@@ -5,112 +5,34 @@
 #include "chrome/browser/ui/views/location_bar/page_action_image_view.h"
 
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/extensions/api/commands/command_service.h"
 #include "chrome/browser/extensions/extension_action.h"
-#include "chrome/browser/extensions/extension_action_icon_factory.h"
-#include "chrome/browser/extensions/extension_action_manager.h"
-#include "chrome/browser/extensions/extension_context_menu_model.h"
-#include "chrome/browser/extensions/extension_tab_util.h"
-#include "chrome/browser/extensions/location_bar_controller.h"
-#include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/sessions/session_id.h"
-#include "chrome/browser/ui/browser_list.h"
-#include "chrome/browser/ui/extensions/accelerator_priority.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
-#include "chrome/browser/ui/webui/extensions/extension_info_ui.h"
 #include "extensions/browser/extension_registry.h"
-#include "extensions/common/extension.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/events/event.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image.h"
-#include "ui/views/controls/menu/menu_runner.h"
-
-using content::WebContents;
-using extensions::LocationBarController;
-using extensions::Extension;
 
 PageActionImageView::PageActionImageView(LocationBarView* owner,
                                          ExtensionAction* page_action,
                                          Browser* browser)
-    : owner_(owner),
-      page_action_(page_action),
-      browser_(browser),
-      current_tab_id_(-1),
-      preview_enabled_(false),
-      popup_(NULL) {
-  const Extension* extension = extensions::ExtensionRegistry::Get(
-      owner_->profile())->enabled_extensions().GetByID(
-          page_action->extension_id());
-  DCHECK(extension);
-
-  icon_factory_.reset(
-      new ExtensionActionIconFactory(
-          owner_->profile(), extension, page_action, this));
-
+    : extension_action_view_(new ExtensionActionView(
+          extensions::ExtensionRegistry::Get(browser->profile())->
+              enabled_extensions().GetByID(page_action->extension_id()),
+          browser,
+          page_action,
+          this)),
+      owner_(owner),
+      preview_enabled_(false) {
   SetAccessibilityFocusable(true);
-  set_context_menu_controller(this);
-
-  extensions::CommandService* command_service =
-      extensions::CommandService::Get(browser_->profile());
-  extensions::Command page_action_command;
-  if (command_service->GetPageActionCommand(
-          extension->id(),
-          extensions::CommandService::ACTIVE_ONLY,
-          &page_action_command,
-          NULL)) {
-    page_action_keybinding_.reset(
-        new ui::Accelerator(page_action_command.accelerator()));
-    owner_->GetFocusManager()->RegisterAccelerator(
-        *page_action_keybinding_.get(),
-        GetAcceleratorPriority(page_action_command.accelerator(), extension),
-        this);
-  }
+  extension_action_view_->RegisterCommand();

[sky, 2014/08/05 22:29:47]

At the time this is invoked does GetFocusManagerForAccelerator() return a
non-null focusmanager. In particular I'm worried that during setup if owner_
isn't in a widget you'll get NULL and won't register accelerators.

[Devlin, 2014/08/05 23:51:10]

This shouldn't be any different than the existing code (I'm hoping that very
little of this patch is functionally different).  In the current code, we
register directly on line 67, without giving any more time for setup, so this
_should_ be fine.  That's assuming, of course, the current code is correct. 
Would it be better to delay registration like we do in BrowserActionButton?

[sky, 2014/08/06 00:28:25]

No need to delay if everything is good. How about a DCHECK that 'this' is in a
Widget?

[Devlin, 2014/08/06 16:19:02]

Done, via DCHECK(GetFocusManagerForAccelerator()).

+  set_context_menu_controller(extension_action_view_.get());
 }
 
 PageActionImageView::~PageActionImageView() {
-  if (owner_->GetFocusManager()) {
-    if (page_action_keybinding_.get()) {
-      owner_->GetFocusManager()->UnregisterAccelerator(
-          *page_action_keybinding_.get(), this);
-    }
-  }
-
-  if (popup_)
-    popup_->GetWidget()->RemoveObserver(this);
-  HidePopup();
-}
-
-void PageActionImageView::ExecuteAction(
-    ExtensionPopup::ShowAction show_action) {
-  WebContents* web_contents = owner_->GetWebContents();
-  if (!web_contents)
-    return;
-
-  extensions::TabHelper* extensions_tab_helper =
-      extensions::TabHelper::FromWebContents(web_contents);
-  LocationBarController* controller =
-      extensions_tab_helper->location_bar_controller();
-
-  switch (controller->OnClicked(page_action_)) {
-    case LocationBarController::ACTION_NONE:
-      break;
-
-    case LocationBarController::ACTION_SHOW_POPUP:
-      ShowPopupWithURL(page_action_->GetPopupUrl(current_tab_id_), show_action);
-      break;
-
-    case LocationBarController::ACTION_SHOW_CONTEXT_MENU:
-      // We are never passing OnClicked a right-click button, so assume that
-      // we're never going to be asked to show a context menu.
-      // TODO(kalman): if this changes, update this class to pass the real
-      // mouse button through to the LocationBarController.
-      NOTREACHED();
-      break;
-  }
 }
 
 void PageActionImageView::GetAccessibleState(ui::AXViewState* state) {
@@ -135,56 +57,22 @@ void PageActionImageView::OnMouseReleased(const ui::MouseEvent& event) {
     return;
   }
 
-  ExecuteAction(ExtensionPopup::SHOW);
+  extension_action_view_->ExecuteActionByUser();
 }
 
 bool PageActionImageView::OnKeyPressed(const ui::KeyEvent& event) {
   if (event.key_code() == ui::VKEY_SPACE ||
       event.key_code() == ui::VKEY_RETURN) {
-    ExecuteAction(ExtensionPopup::SHOW);
+    extension_action_view_->ExecuteActionByUser();
     return true;
   }
   return false;
 }
 
-void PageActionImageView::ShowContextMenuForView(
-    View* source,
-    const gfx::Point& point,
-    ui::MenuSourceType source_type) {
-  const Extension* extension = extensions::ExtensionRegistry::Get(
-      owner_->profile())->enabled_extensions().GetByID(
-          page_action()->extension_id());
-  if (!extension->ShowConfigureContextMenus())
-    return;
-
-  scoped_refptr<ExtensionContextMenuModel> context_menu_model(
-      new ExtensionContextMenuModel(extension, browser_, this));
-  menu_runner_.reset(new views::MenuRunner(
-      context_menu_model.get(),
-      views::MenuRunner::HAS_MNEMONICS | views::MenuRunner::CONTEXT_MENU));
-  gfx::Point screen_loc;
-  views::View::ConvertPointToScreen(this, &screen_loc);
-  if (menu_runner_->RunMenuAt(GetWidget(),
-                              NULL,
-                              gfx::Rect(screen_loc, size()),
-                              views::MENU_ANCHOR_TOPLEFT,
-                              source_type) == views::MenuRunner::MENU_DELETED) {
-    return;
-  }
-}
-
 bool PageActionImageView::AcceleratorPressed(
     const ui::Accelerator& accelerator) {
   DCHECK(visible());  // Should not have happened due to CanHandleAccelerator.
-
-  const std::string extension_id = page_action()->extension_id();
-  const ui::AcceleratorManager::HandlerPriority priority =
-      GetAcceleratorPriorityById(accelerator, extension_id, owner_->profile());
-  // Normal priority shortcuts must be handled via standard browser commands
-  // to be processed at the proper time.
-  if (priority == ui::AcceleratorManager::kHighPriority)
-    ExecuteAction(ExtensionPopup::SHOW);
-  return priority == ui::AcceleratorManager::kHighPriority;
+  return extension_action_view_->AcceleratorPressed(accelerator);
 }
 
 bool PageActionImageView::CanHandleAccelerators() const {
@@ -193,76 +81,67 @@ bool PageActionImageView::CanHandleAccelerators() const {
   return visible();
 }
 
-void PageActionImageView::UpdateVisibility(WebContents* contents,
-                                           const GURL& url) {
-  // Save this off so we can pass it back to the extension when the action gets
-  // executed. See PageActionImageView::OnMousePressed.
-  current_tab_id_ =
-      contents ? extensions::ExtensionTabUtil::GetTabId(contents) : -1;
-  current_url_ = url;
+void PageActionImageView::UpdateVisibility(content::WebContents* contents) {
+  int tab_id = extension_action_view_->GetCurrentTabId();
 
   if (!contents ||
-      (!preview_enabled_ && !page_action_->GetIsVisible(current_tab_id_))) {
+      tab_id == -1 ||
+      (!preview_enabled_ && !extension_action()->GetIsVisible(tab_id))) {
     SetVisible(false);
     return;
   }
 
   // Set the tooltip.
-  tooltip_ = page_action_->GetTitle(current_tab_id_);
+  tooltip_ = extension_action()->GetTitle(tab_id);
   SetTooltipText(base::UTF8ToUTF16(tooltip_));
 
   // Set the image.
-  gfx::Image icon = icon_factory_->GetIcon(current_tab_id_);
+  gfx::Image icon = extension_action_view_->GetIcon(tab_id);
   if (!icon.IsEmpty())
     SetImage(*icon.ToImageSkia());
 
   SetVisible(true);
 }
 
-void PageActionImageView::InspectPopup() {
-  ExecuteAction(ExtensionPopup::SHOW_AND_INSPECT);
-}
-
-void PageActionImageView::OnWidgetDestroying(views::Widget* widget) {
-  DCHECK_EQ(popup_->GetWidget(), widget);
-  popup_->GetWidget()->RemoveObserver(this);
-  popup_ = NULL;
+void PageActionImageView::PaintChildren(gfx::Canvas* canvas,
+                                        const views::CullSet& cull_set) {
+  View::PaintChildren(canvas, cull_set);
+  int tab_id = extension_action_view_->GetCurrentTabId();
+  if (tab_id >= 0) {
+    extension_action_view_->extension_action()->PaintBadge(
+        canvas, GetLocalBounds(), tab_id);
+  }
 }
 
 void PageActionImageView::OnIconUpdated() {
-  WebContents* web_contents = owner_->GetWebContents();
-  if (web_contents)
-    UpdateVisibility(web_contents, current_url_);
+  UpdateVisibility(GetCurrentWebContents());
 }
 
-void PageActionImageView::PaintChildren(gfx::Canvas* canvas,
-                                        const views::CullSet& cull_set) {
-  View::PaintChildren(canvas, cull_set);
-  if (current_tab_id_ >= 0)
-    page_action_->PaintBadge(canvas, GetLocalBounds(), current_tab_id_);
+views::View* PageActionImageView::GetAsView() {
+  return this;
 }
 
-void PageActionImageView::ShowPopupWithURL(
-    const GURL& popup_url,
-    ExtensionPopup::ShowAction show_action) {
-  bool popup_showing = popup_ != NULL;
+bool PageActionImageView::IsNestedView() {
+  return false;
+}
 
-  // Always hide the current popup. Only one popup at a time.
-  HidePopup();
+views::FocusManager* PageActionImageView::GetFocusManagerForAccelerator() {
+  return owner_->GetFocusManager();
+}
 
-  // If we were already showing, then treat this click as a dismiss.
-  if (popup_showing)
-    return;
+views::Widget* PageActionImageView::GetParentForContextMenu() {
+  return GetWidget();
+}
 
-  views::BubbleBorder::Arrow arrow = base::i18n::IsRTL() ?
-      views::BubbleBorder::TOP_LEFT : views::BubbleBorder::TOP_RIGHT;
+views::View* PageActionImageView::GetReferenceViewForPopup() {
+  return this;
+}
 
-  popup_ = ExtensionPopup::ShowPopup(popup_url, browser_, this, arrow,
-                                     show_action);
-  popup_->GetWidget()->AddObserver(this);
+content::WebContents* PageActionImageView::GetCurrentWebContents() {
+  return owner_->GetWebContents();
 }
 
-void PageActionImageView::HidePopup() {
-  if (popup_)
-    popup_->GetWidget()->Close();
+void PageActionImageView::HideActivePopup() {
+  // The only popup that will be active is this popup.
+  extension_action_view_->HidePopup();
 }