Combine BrowserActionView and BrowserActionButton

No one seems to know why we have BrowserActionView and BrowserActionButton.
Let's combine them for simplicity.

BUG=402526

Review URL: https://codereview.chromium.org/411063003

Cr-Commit-Position: refs/heads/master@{#289092}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289092 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc

@@ -22,26 +22,25 @@
 #include "ui/views/widget/widget.h"
 
 // In the browser actions container's chevron menu, a menu item view's icon
-// comes from BrowserActionView::GetIconWithBadge() (which comes from the
-// browser action button's icon) when the menu item view is created. But, the
-// browser action button's icon may not be loaded in time because it is read
-// from file system in another thread.
+// comes from BrowserActionView::GetIconWithBadge() when the menu item view is
+// created. But, the browser action's icon may not be loaded in time because it
+// is read from file system in another thread.
 // The IconUpdater will update the menu item view's icon when the browser
-// action button's icon has been updated.
-class IconUpdater : public BrowserActionButton::IconObserver {
+// action's icon has been updated.
+class IconUpdater : public BrowserActionView::IconObserver {
  public:
-  IconUpdater(views::MenuItemView* menu_item_view, BrowserActionButton* button)
+  IconUpdater(views::MenuItemView* menu_item_view, BrowserActionView* view)
       : menu_item_view_(menu_item_view),
-        button_(button) {
+        view_(view) {
     DCHECK(menu_item_view);
-    DCHECK(button);
-    button->set_icon_observer(this);
+    DCHECK(view);
+    view->set_icon_observer(this);
   }
   virtual ~IconUpdater() {
-    button_->set_icon_observer(NULL);
+    view_->set_icon_observer(NULL);
   }
 
-  // Overridden from BrowserActionButton::IconObserver:
+  // Overridden from BrowserActionView::IconObserver:
   virtual void OnIconUpdated(const gfx::ImageSkia& icon) OVERRIDE {
     menu_item_view_->SetIcon(icon);
   }
@@ -50,9 +49,9 @@ class IconUpdater : public BrowserActionButton::IconObserver {
   // The menu item view whose icon might be updated.
   views::MenuItemView* menu_item_view_;
 
-  // The button to be observed. When its icon changes, update the corresponding
+  // The view to be observed. When its icon changes, update the corresponding
   // menu item view's icon.
-  BrowserActionButton* button_;
+  BrowserActionView* view_;
 
   DISALLOW_COPY_AND_ASSIGN(IconUpdater);
 };
@@ -82,16 +81,16 @@ BrowserActionOverflowMenuController::BrowserActionOverflowMenuController(
     BrowserActionView* view = (*views_)[i];
     views::MenuItemView* menu_item = menu_->AppendMenuItemWithIcon(
         command_id,
-        base::UTF8ToUTF16(view->button()->extension()->name()),
+        base::UTF8ToUTF16(view->extension()->name()),
         view->GetIconWithBadge());
 
     // Set the tooltip for this item.
     base::string16 tooltip = base::UTF8ToUTF16(
-        view->button()->extension_action()->GetTitle(
-            view->button()->view_controller()->GetCurrentTabId()));
+        view->extension_action()->GetTitle(
+            view->view_controller()->GetCurrentTabId()));
     menu_->SetTooltip(tooltip, command_id);
 
-    icon_updaters_.push_back(new IconUpdater(menu_item, view->button()));
+    icon_updaters_.push_back(new IconUpdater(menu_item, view));
 
     ++command_id;
   }
@@ -130,13 +129,12 @@ void BrowserActionOverflowMenuController::NotifyBrowserActionViewsDeleting() {
 }
 
 bool BrowserActionOverflowMenuController::IsCommandEnabled(int id) const {
-  BrowserActionButton* button = (*views_)[start_index_ + id - 1]->button();
-  return button->IsEnabled(button->view_controller()->GetCurrentTabId());
+  BrowserActionView* view = (*views_)[start_index_ + id - 1];
+  return view->IsEnabled(view->view_controller()->GetCurrentTabId());
 }
 
 void BrowserActionOverflowMenuController::ExecuteCommand(int id) {
-  BrowserActionView* view = (*views_)[start_index_ + id - 1];
-  view->button()->view_controller()->ExecuteActionByUser();
+  (*views_)[start_index_ + id - 1]->view_controller()->ExecuteActionByUser();
 }
 
 bool BrowserActionOverflowMenuController::ShowContextMenu(
@@ -144,13 +142,13 @@ bool BrowserActionOverflowMenuController::ShowContextMenu(
     int id,
     const gfx::Point& p,
     ui::MenuSourceType source_type) {
-  BrowserActionButton* button = (*views_)[start_index_ + id - 1]->button();
-  if (!button->extension()->ShowConfigureContextMenus())
+  BrowserActionView* view = (*views_)[start_index_ + id - 1];
+  if (!view->extension()->ShowConfigureContextMenus())
     return false;
 
   scoped_refptr<ExtensionContextMenuModel> context_menu_contents =
       new ExtensionContextMenuModel(
-          button->extension(), browser_, button->view_controller());
+          view->extension(), browser_, view->view_controller());
   views::MenuRunner context_menu_runner(context_menu_contents.get(),
                                         views::MenuRunner::HAS_MNEMONICS |
                                             views::MenuRunner::IS_NESTED |
@@ -245,9 +243,7 @@ void BrowserActionOverflowMenuController::WriteDragData(
     views::MenuItemView* sender, OSExchangeData* data) {
   size_t drag_index;
   BrowserActionView* view = ViewForId(sender->GetCommand(), &drag_index);
-  std::string id = view->button()->extension()->id();
-
-  BrowserActionDragData drag_data(id, drag_index);
+  BrowserActionDragData drag_data(view->extension()->id(), drag_index);
   drag_data.Write(owner_->profile(), data);
 }
 

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

@@ -37,40 +37,39 @@ int BrowserActionTestUtil::VisibleBrowserActions() {
 ExtensionAction* BrowserActionTestUtil::GetExtensionAction(int index) {
   return extensions::ExtensionActionManager::Get(browser_->profile())->
       GetBrowserAction(*GetContainer(browser_)->GetBrowserActionViewAt(index)->
-                       button()->extension());
+                       extension());
 }
 
 void BrowserActionTestUtil::InspectPopup(int index) {
-  GetContainer(browser_)->GetBrowserActionViewAt(index)->button()->
+  GetContainer(browser_)->GetBrowserActionViewAt(index)->
       view_controller()->InspectPopup();
 }
 
 bool BrowserActionTestUtil::HasIcon(int index) {
-  return !GetContainer(browser_)->GetBrowserActionViewAt(index)->button()->
+  return !GetContainer(browser_)->GetBrowserActionViewAt(index)->
       GetImage(views::Button::STATE_NORMAL).isNull();
 }
 
 gfx::Image BrowserActionTestUtil::GetIcon(int index) {
   gfx::ImageSkia icon = GetContainer(browser_)->GetBrowserActionViewAt(index)->
-      button()->GetIconForTest();
+      GetIconForTest();
   return gfx::Image(icon);
 }
 
 void BrowserActionTestUtil::Press(int index) {
   GetContainer(browser_)->GetBrowserActionViewAt(index)->
-      button()->view_controller()->ExecuteActionByUser();
+      view_controller()->ExecuteActionByUser();
 }
 
 std::string BrowserActionTestUtil::GetExtensionId(int index) {
-  BrowserActionButton* button =
-      GetContainer(browser_)->GetBrowserActionViewAt(index)->button();
-  return button->extension()->id();
+  return GetContainer(browser_)->GetBrowserActionViewAt(index)->
+      extension()->id();
 }
 
 std::string BrowserActionTestUtil::GetTooltip(int index) {
   base::string16 text;
-  GetContainer(browser_)->GetBrowserActionViewAt(index)->button()->
-    GetTooltipText(gfx::Point(), &text);
+  GetContainer(browser_)->GetBrowserActionViewAt(index)->
+      GetTooltipText(gfx::Point(), &text);
   return base::UTF16ToUTF8(text);
 }
 

chrome/browser/ui/views/toolbar/browser_action_view.h

@@ -15,7 +15,6 @@
 #include "ui/views/view.h"
 
 class Browser;
-class BrowserActionButton;
 class ExtensionAction;
 
 namespace extensions {
@@ -28,9 +27,16 @@ class Image;
 
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserActionView
-// A single entry in the browser action container. This contains the actual
-// BrowserActionButton, as well as the logic to paint the badge.
-class BrowserActionView : public views::View {
+// A wrapper around an ExtensionActionViewController to display an extension
+// action in the BrowserActionsContainer.
+// Despite its name, this class can handle either Browser Actions or Page
+// Actions.
+// TODO(devlin): Rename this and BrowserActionsContainer when more of the
+// toolbar redesign is done.
+class BrowserActionView : public views::MenuButton,
+                          public ExtensionActionViewDelegate,
+                          public views::ButtonListener,
+                          public content::NotificationObserver {
  public:
   // Need DragController here because BrowserActionView could be
   // dragged/dropped.
@@ -56,7 +62,7 @@ class BrowserActionView : public views::View {
     virtual views::View* GetOverflowReferenceView() = 0;
 
     // Sets the delegate's active popup owner to be |popup_owner|.
-    virtual void SetPopupOwner(BrowserActionButton* popup_owner) = 0;
+    virtual void SetPopupOwner(BrowserActionView* popup_owner) = 0;
 
     // Hides the active popup of the delegate, if one exists.
     virtual void HideActivePopup() = 0;
@@ -65,47 +71,6 @@ class BrowserActionView : public views::View {
     virtual ~Delegate() {}
   };
 
-  BrowserActionView(const extensions::Extension* extension,
-                    Browser* browser,
-                    Delegate* delegate);
-  virtual ~BrowserActionView();
-
-  BrowserActionButton* button() { return button_.get(); }
-
-  // Gets browser action button icon with the badge.
-  gfx::ImageSkia GetIconWithBadge();
-
-  // Overridden from views::View:
-  virtual void Layout() OVERRIDE;
-  virtual void GetAccessibleState(ui::AXViewState* state) OVERRIDE;
-  virtual gfx::Size GetPreferredSize() const OVERRIDE;
-
- protected:
-  // Overridden from views::View to paint the badge on top of children.
-  virtual void PaintChildren(gfx::Canvas* canvas,
-                             const views::CullSet& cull_set) OVERRIDE;
-
- private:
-  // Usually a container for this view.
-  Delegate* delegate_;
-
-  // The button this view contains.
-  scoped_ptr<BrowserActionButton> button_;
-
-  DISALLOW_COPY_AND_ASSIGN(BrowserActionView);
-};
-
-////////////////////////////////////////////////////////////////////////////////
-// BrowserActionButton
-
-// The BrowserActionButton is a specialization of the MenuButton class.
-// This wraps an ExtensionActionView, and has knowledge of how to render itself
-// and when to trigger the extension action.
-class BrowserActionButton : public views::MenuButton,
-                            public ExtensionActionViewDelegate,
-                            public views::ButtonListener,
-                            public content::NotificationObserver {
- public:
   // The IconObserver will receive a notification when the button's icon has
   // been updated.
   class IconObserver {
@@ -116,10 +81,10 @@ class BrowserActionButton : public views::MenuButton,
     virtual ~IconObserver() {}
   };
 
-  BrowserActionButton(const extensions::Extension* extension,
+  BrowserActionView(const extensions::Extension* extension,
                       Browser* browser,
                       BrowserActionView::Delegate* delegate);
-  virtual ~BrowserActionButton();
+  virtual ~BrowserActionView();
 
   const extensions::Extension* extension() const {
     return view_controller_->extension();
@@ -182,13 +147,15 @@ class BrowserActionButton : public views::MenuButton,
   // Returns button icon so it can be accessed during tests.
   gfx::ImageSkia GetIconForTest();
 
- protected:
+ private:
   // Overridden from views::View:
   virtual void ViewHierarchyChanged(
       const ViewHierarchyChangedDetails& details) OVERRIDE;
   virtual void OnDragDone() OVERRIDE;
+  virtual gfx::Size GetPreferredSize() const OVERRIDE;
+  virtual void PaintChildren(gfx::Canvas* canvas,
+                             const views::CullSet& cull_set) OVERRIDE;
 
- private:
   // ExtensionActionViewDelegate:
   virtual views::View* GetAsView() OVERRIDE;
   virtual bool IsShownInMenu() OVERRIDE;
@@ -218,7 +185,7 @@ class BrowserActionButton : public views::MenuButton,
   // updated.
   IconObserver* icon_observer_;
 
-  DISALLOW_COPY_AND_ASSIGN(BrowserActionButton);
+  DISALLOW_COPY_AND_ASSIGN(BrowserActionView);
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_TOOLBAR_BROWSER_ACTION_VIEW_H_

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

@@ -213,7 +213,7 @@ BrowserActionView* BrowserActionsContainer::GetBrowserActionView(
     ExtensionAction* action) {
   for (BrowserActionViews::iterator i(browser_action_views_.begin());
        i != browser_action_views_.end(); ++i) {
-    if ((*i)->button()->extension_action() == action)
+    if ((*i)->extension_action() == action)
       return *i;
   }
   return NULL;
@@ -221,7 +221,7 @@ BrowserActionView* BrowserActionsContainer::GetBrowserActionView(
 
 void BrowserActionsContainer::RefreshBrowserActionViews() {
   for (size_t i = 0; i < browser_action_views_.size(); ++i)
-    browser_action_views_[i]->button()->UpdateState();
+    browser_action_views_[i]->UpdateState();
 }
 
 void BrowserActionsContainer::CreateBrowserActionViews() {
@@ -294,7 +294,7 @@ views::View* BrowserActionsContainer::GetOverflowReferenceView() {
   return chevron_;
 }
 
-void BrowserActionsContainer::SetPopupOwner(BrowserActionButton* popup_owner) {
+void BrowserActionsContainer::SetPopupOwner(BrowserActionView* popup_owner) {
   // We should never be setting a popup owner when one already exists.
   DCHECK(!popup_owner_ || !popup_owner);
   popup_owner_ = popup_owner;
@@ -525,7 +525,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()]->button()->extension()->id(),
+  DCHECK_EQ(browser_action_views_[data.index()]->extension()->id(),
             data.id());
   DCHECK(model_);
 
@@ -545,7 +545,7 @@ int BrowserActionsContainer::OnPerformDrop(
     i = model_->IncognitoIndexToOriginal(i);
 
   model_->MoveBrowserAction(
-      browser_action_views_[data.index()]->button()->extension(), i);
+      browser_action_views_[data.index()]->extension(), i);
 
   OnDragExited();  // Perform clean up after dragging.
   FOR_EACH_OBSERVER(BrowserActionsContainerObserver,
@@ -581,17 +581,16 @@ void BrowserActionsContainer::WriteDragDataForView(View* sender,
   DCHECK(data);
 
   for (size_t i = 0; i < browser_action_views_.size(); ++i) {
-    BrowserActionButton* button = browser_action_views_[i]->button();
-    if (button == sender) {
+    BrowserActionView* view = browser_action_views_[i];
+    if (view == sender) {
       // Set the dragging image for the icon.
-      gfx::ImageSkia badge(browser_action_views_[i]->GetIconWithBadge());
+      gfx::ImageSkia badge(view->GetIconWithBadge());
       drag_utils::SetDragImageOnDataObject(badge,
                                            press_pt.OffsetFromOrigin(),
                                            data);
 
       // Fill in the remaining info.
-      BrowserActionDragData drag_data(
-          browser_action_views_[i]->button()->extension()->id(), i);
+      BrowserActionDragData drag_data(view->extension()->id(), i);
       drag_data.Write(profile_, data);
       break;
     }
@@ -793,7 +792,7 @@ void BrowserActionsContainer::BrowserActionAdded(const Extension* extension,
                                                  int index) {
 #if defined(DEBUG)
   for (size_t i = 0; i < browser_action_views_.size(); ++i) {
-    DCHECK(browser_action_views_[i]->button()->extension() != extension) <<
+    DCHECK(browser_action_views_[i]->extension() != extension) <<
            "Asked to add a browser action view for an extension that already "
            "exists.";
   }
@@ -835,7 +834,7 @@ void BrowserActionsContainer::BrowserActionRemoved(const Extension* extension) {
   size_t visible_actions = VisibleBrowserActionsAfterAnimation();
   for (BrowserActionViews::iterator i(browser_action_views_.begin());
        i != browser_action_views_.end(); ++i) {
-    if ((*i)->button()->extension() == extension) {
+    if ((*i)->extension() == extension) {
       delete *i;
       browser_action_views_.erase(i);
 
@@ -1045,9 +1044,9 @@ bool BrowserActionsContainer::ShowPopupForExtension(
 
   for (BrowserActionViews::iterator iter = browser_action_views_.begin();
        iter != browser_action_views_.end(); ++iter) {
-    BrowserActionButton* button = (*iter)->button();
-    if (button->extension() == extension)
-      return button->view_controller()->ExecuteAction(
+    BrowserActionView* view = (*iter);
+    if (view->extension() == extension)
+      return view->view_controller()->ExecuteAction(
           ExtensionPopup::SHOW, grant_tab_permissions);
   }
   return false;

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

@@ -235,7 +235,7 @@ class BrowserActionsContainer
   virtual bool ShownInsideMenu() const OVERRIDE;
   virtual void OnBrowserActionViewDragDone() OVERRIDE;
   virtual views::View* GetOverflowReferenceView() OVERRIDE;
-  virtual void SetPopupOwner(BrowserActionButton* popup_owner) OVERRIDE;
+  virtual void SetPopupOwner(BrowserActionView* popup_owner) OVERRIDE;
   virtual void HideActivePopup() OVERRIDE;
 
   // Overridden from extension::ExtensionKeybindingRegistry::Delegate:
@@ -370,9 +370,9 @@ class BrowserActionsContainer
   // the difference between main and overflow.
   BrowserActionsContainer* main_container_;
 
-  // The button that triggered the current popup (just a reference to a button
+  // The view that triggered the current popup (just a reference to a view
   // from browser_action_views_).
-  BrowserActionButton* popup_owner_;
+  BrowserActionView* popup_owner_;
 
   // The model that tracks the order of the toolbar icons.
   extensions::ExtensionToolbarModel* model_;