Add infrastructure for Chrome Actions

chrome/browser/ui/views/toolbar/browser_actions_container.cc

Index: chrome/browser/ui/views/toolbar/browser_actions_container.cc
diff --git a/chrome/browser/ui/views/toolbar/browser_actions_container.cc b/chrome/browser/ui/views/toolbar/browser_actions_container.cc
index a1398c167f41fba1b8af89737cab218a88ed9974..55706072a72a2671d7c3740606b874f86793d055 100644
--- a/chrome/browser/ui/views/toolbar/browser_actions_container.cc
+++ b/chrome/browser/ui/views/toolbar/browser_actions_container.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/ui/views/extensions/extension_popup.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/toolbar/browser_actions_container_observer.h"
+#include "chrome/browser/ui/views/toolbar/chrome_actions_registry.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/extensions/command.h"
 #include "chrome/grit/generated_resources.h"
@@ -130,7 +131,7 @@ BrowserActionsContainer::~BrowserActionsContainer() {
   if (model_)
     model_->RemoveObserver(this);
   HideActivePopup();
-  DeleteBrowserActionViews();
+  DeleteActionViews();
 }
 
 void BrowserActionsContainer::Init() {
@@ -149,18 +150,25 @@ void BrowserActionsContainer::Init() {
 BrowserActionView* BrowserActionsContainer::GetViewForExtension(
     const Extension* extension) {
   for (BrowserActionView* view : browser_action_views_) {
-    if (view->extension() == extension)
+    if (view->GetExtensionActionViewController()->extension() == extension)
       return view;
   }
   return NULL;
 }
 
-void BrowserActionsContainer::RefreshBrowserActionViews() {
+const Extension* BrowserActionsContainer::GetExtensionAt(size_t index) const {
+  return browser_action_views_[index]->GetExtensionActionViewController()->
+      extension();
+}
+
+void BrowserActionsContainer::RefreshActionViews() {
   for (BrowserActionView* view : browser_action_views_)
     view->UpdateState();
+  for (BrowserActionView* view : chrome_action_views_)
+    view->UpdateState();
 }
 
-void BrowserActionsContainer::CreateBrowserActionViews() {
+void BrowserActionsContainer::CreateActionViews() {
   DCHECK(browser_action_views_.empty());
   if (!model_)
     return;
@@ -169,19 +177,39 @@ void BrowserActionsContainer::CreateBrowserActionViews() {
       extensions::ExtensionActionManager::Get(profile_);
   const extensions::ExtensionList& toolbar_items = model_->toolbar_items();

[sky, 2014/10/15 23:52:30]

Seems like there should be one unified presentation of all the actions to be
shown and it seems like this code shouldn't need any extension specific bits.
That's an implementation of the specific action, right?

[Devlin, 2014/10/16 16:46:38]

In a perfect world, yes.  But that will lead to a much larger change - this
class interfaces with the ExtensionToolbarModel, which currently only manages
extensions.  We could potentially move most of the chrome action vs extension
action logic into there (maybe then we also need to move it out of
c/b/extensions), but it'll be a lot more code in an already-big patch.  Would
you like me to see what it'll take?  Or is there a simpler way to go about it?

[sky, 2014/10/16 18:13:39]

I think it's possible to do this incrementally. No doubt going down the route
I'm outlying is more patches than what you have here, but in the end it'll be a
cleaner API and not an API burdened by how things were initially. Strive for a
clean architecture.

[Devlin, 2014/10/16 22:05:27]

I'm curious what you think of Patch Set 3's approach.  I don't want to drag the
component actions into the extension toolbar model yet, because I think that
*would* result in a much larger and more confusing patch, but with Patch Set 3,
we're back to only having one set of views, and not treating them quite as
specially.

   for (const scoped_refptr<const Extension>& extension : toolbar_items) {
-    BrowserActionView* view =
-        new BrowserActionView(extension.get(),
-                              action_manager->GetExtensionAction(*extension),
-                              browser_,
-                              this);
+    BrowserActionView* view = new BrowserActionView(
+        make_scoped_ptr(new ExtensionActionViewController(
+            extension.get(),
+            browser_,
+            action_manager->GetExtensionAction(*extension),
+            NULL)),
+        BrowserActionView::TYPE_EXTENSION_ACTION,
+        browser_,
+        this);
     browser_action_views_.push_back(view);
     AddChildView(view);
   }
+
+  ScopedVector<ToolbarActionViewController> chrome_actions =
+      ChromeActionsRegistry::GetChromeActions();
+  DCHECK(extensions::FeatureSwitch::extension_action_redesign()->IsEnabled() ||
+         chrome_actions.empty());
+  for (ToolbarActionViewController* controller : chrome_actions) {
+    BrowserActionView* view = new BrowserActionView(
+        make_scoped_ptr(controller),
+        BrowserActionView::TYPE_CHROME_ACTION,
+        browser_,
+        this);
+    chrome_action_views_.push_back(view);
+    AddChildView(view);
+  }
+  chrome_actions.weak_clear();
 }
 
-void BrowserActionsContainer::DeleteBrowserActionViews() {
+void BrowserActionsContainer::DeleteActionViews() {
   HideActivePopup();
   STLDeleteElements(&browser_action_views_);
+  STLDeleteElements(&chrome_action_views_);
 }
 
 size_t BrowserActionsContainer::VisibleBrowserActions() const {
@@ -254,11 +282,25 @@ void BrowserActionsContainer::HideActivePopup() {
     popup_owner_->view_controller()->HidePopup();
 }
 
-BrowserActionView* BrowserActionsContainer::GetMainViewForExtension(
-    const Extension* extension) {
-  return in_overflow_mode() ?
-      main_container_->GetViewForExtension(extension) :
-      GetViewForExtension(extension);
+BrowserActionView* BrowserActionsContainer::GetMainViewForAction(
+    BrowserActionView* view) {
+  if (!in_overflow_mode())
+    return view;  // This is the main view.
+
+  // The overflow container and main container each have the same views and
+  // view indices, so we can return the view of the index that |view| has in
+  // this container.
+  bool is_extension_action =
+      view->type() == BrowserActionView::TYPE_EXTENSION_ACTION;
+  const BrowserActionViews& views =
+      is_extension_action ? browser_action_views_ : chrome_action_views_;
+  BrowserActionViews::const_iterator iter =
+      std::find(views.begin(), views.end(), view);
+  DCHECK(iter != views.end());
+  size_t index = iter - views.begin();
+  return is_extension_action ?
+      main_container_->browser_action_views_[index] :
+      main_container_->chrome_action_views_[index];
 }
 
 void BrowserActionsContainer::AddObserver(
@@ -312,7 +354,7 @@ gfx::Size BrowserActionsContainer::GetMinimumSize() const {
 }
 
 void BrowserActionsContainer::Layout() {
-  if (browser_action_views_.empty()) {
+  if (browser_action_views_.empty() && chrome_action_views_.empty()) {
     SetVisible(false);
     return;
   }
@@ -336,6 +378,14 @@ void BrowserActionsContainer::Layout() {
     chevron_->SetVisible(false);
   }
 
+  // Construct a single vector of all views to draw to ease calculation (since
+  // this only copies pointers, it's cheap).
+  BrowserActionViews views(browser_action_views_.begin(),
+                           browser_action_views_.end());
+  views.insert(views.end(),
+               chrome_action_views_.begin(),
+               chrome_action_views_.end());
+
   // The padding before the first icon and after the last icon in the container.
   int container_padding =
       in_overflow_mode() ? kItemSpacing : ToolbarView::kStandardSpacing;
@@ -347,7 +397,7 @@ void BrowserActionsContainer::Layout() {
   // can display with the given width. We add an extra kItemSpacing because the
   // last icon doesn't need padding, but we want it to divide easily.
   size_t end_index = in_overflow_mode() ?
-      browser_action_views_.size() :
+      views.size() :
       (max_x - 2 * container_padding + kItemSpacing) / IconWidth(true);
   // The maximum length for one row of icons.
   size_t row_length =
@@ -356,8 +406,8 @@ void BrowserActionsContainer::Layout() {
   // Now draw the icons for the browser actions in the available space. Once
   // all the variables are in place, the layout works equally well for the main
   // and overflow container.
-  for (size_t i = 0u; i < browser_action_views_.size(); ++i) {
-    BrowserActionView* view = browser_action_views_[i];
+  for (size_t i = 0u; i < views.size(); ++i) {
+    BrowserActionView* view = views[i];
     if (i < start_index || i >= end_index) {
       view->SetVisible(false);
     } else {
@@ -479,8 +529,7 @@ int BrowserActionsContainer::OnPerformDrop(
     return ui::DragDropTypes::DRAG_NONE;
 
   // Make sure we have the same view as we started with.
-  DCHECK_EQ(browser_action_views_[data.index()]->extension()->id(),
-            data.id());
+  DCHECK_EQ(GetExtensionAt(data.index())->id(), data.id());
   DCHECK(model_);
 
   size_t i = drop_position_->row * icons_per_overflow_menu_row_ +
@@ -499,8 +548,7 @@ int BrowserActionsContainer::OnPerformDrop(
   // visible icons.
   bool drag_between_containers =
       !browser_action_views_[data.index()]->visible();
-  model_->MoveExtensionIcon(
-      browser_action_views_[data.index()]->extension(), i);
+  model_->MoveExtensionIcon(GetExtensionAt(data.index()), i);
 
   if (drag_between_containers) {
     // Let the main container update the model.
@@ -529,11 +577,13 @@ void BrowserActionsContainer::WriteDragDataForView(View* sender,
                                                 browser_action_views_.end(),
                                                 sender);
   DCHECK(iter != browser_action_views_.end());
-  drag_utils::SetDragImageOnDataObject((*iter)->GetIconWithBadge(),
+  ExtensionActionViewController* view_controller =
+      (*iter)->GetExtensionActionViewController();
+  drag_utils::SetDragImageOnDataObject(view_controller->GetIconWithBadge(),
                                        press_pt.OffsetFromOrigin(),
                                        data);
   // Fill in the remaining info.
-  BrowserActionDragData drag_data((*iter)->extension()->id(),
+  BrowserActionDragData drag_data(view_controller->extension()->id(),
                                   iter - browser_action_views_.begin());
   drag_data.Write(profile_, data);
 }
@@ -606,7 +656,9 @@ extensions::ActiveTabPermissionGranter*
 }
 
 ExtensionPopup* BrowserActionsContainer::TestGetPopup() {
-  return popup_owner_ ? popup_owner_->view_controller()->popup() : NULL;
+  return popup_owner_ ?
+      popup_owner_->GetExtensionActionViewController()->popup() :
+      NULL;
 }
 
 void BrowserActionsContainer::OnPaint(gfx::Canvas* canvas) {
@@ -679,7 +731,7 @@ void BrowserActionsContainer::ViewHierarchyChanged(
     // We do this here instead of in the constructor because AddBrowserAction
     // calls Layout on the Toolbar, which needs this object to be constructed
     // before its Layout function is called.
-    CreateBrowserActionViews();
+    CreateActionViews();
   }
 }
 
@@ -720,12 +772,16 @@ void BrowserActionsContainer::ToolbarExtensionAdded(const Extension* extension,
     chevron_->CloseMenu();
 
   // Add the new browser action to the vector and the view hierarchy.
-  BrowserActionView* view =
-      new BrowserActionView(extension,
-                            extensions::ExtensionActionManager::Get(profile_)->
-                                GetExtensionAction(*extension),
-                            browser_,
-                            this);
+  BrowserActionView* view = new BrowserActionView(
+      make_scoped_ptr(new ExtensionActionViewController(
+          extension,
+          browser_,
+          extensions::ExtensionActionManager::Get(profile_)->
+              GetExtensionAction(*extension),
+          NULL)),
+      BrowserActionView::TYPE_EXTENSION_ACTION,
+      browser_,
+      this);
   browser_action_views_.insert(browser_action_views_.begin() + index, view);
   AddChildViewAt(view, index);
 
@@ -765,7 +821,7 @@ void BrowserActionsContainer::ToolbarExtensionRemoved(
   size_t visible_actions = VisibleBrowserActionsAfterAnimation();
   for (BrowserActionViews::iterator i(browser_action_views_.begin());
        i != browser_action_views_.end(); ++i) {
-    if ((*i)->extension() == extension) {
+    if ((*i)->GetExtensionActionViewController()->extension() == extension) {
       delete *i;
       browser_action_views_.erase(i);
 
@@ -799,9 +855,8 @@ void BrowserActionsContainer::ToolbarExtensionMoved(const Extension* extension,
 
   BrowserActionViews::iterator iter = browser_action_views_.begin();
   while (iter != browser_action_views_.end() &&
-         (*iter)->extension() != extension) {
+         (*iter)->GetExtensionActionViewController()->extension() != extension)
     ++iter;
-  }
 
   DCHECK(iter != browser_action_views_.end());
   if (iter - browser_action_views_.begin() == index)
@@ -831,8 +886,8 @@ bool BrowserActionsContainer::ShowExtensionActionPopup(
     return false;
 
   BrowserActionView* view = GetViewForExtension(extension);
-  return view && view->view_controller()->ExecuteAction(ExtensionPopup::SHOW,
-                                                        grant_active_tab);
+  return view && view->GetExtensionActionViewController()->ExecuteAction(
+      ExtensionPopup::SHOW, grant_active_tab);
 }
 
 void BrowserActionsContainer::ToolbarVisibleCountChanged() {
@@ -846,8 +901,8 @@ void BrowserActionsContainer::ToolbarHighlightModeChanged(
   // we delete and recreate everything here, but given everything else going on
   // (the lack of highlight, n more extensions appearing, etc), it's not worth
   // the extra complexity to create and insert only the new extensions.
-  DeleteBrowserActionViews();
-  CreateBrowserActionViews();
+  DeleteActionViews();
+  CreateActionViews();
   Animate(gfx::Tween::LINEAR, GetIconCount());
 }
 
@@ -968,6 +1023,7 @@ size_t BrowserActionsContainer::GetIconCount() const {
 
   // The overflow displays any icons not shown by the main bar.
   return in_overflow_mode() ?
-      model_->toolbar_items().size() - absolute_model_visible_size :
+      model_->toolbar_items().size() - absolute_model_visible_size +
+          chrome_action_views_.size() :
       absolute_model_visible_size;
 }