Add infrastructure for Chrome Actions

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

 // Copyright 2014 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_UI_VIEWS_EXTENSIONS_EXTENSION_ACTION_VIEW_DELEGATE_H_
-#define CHROME_BROWSER_UI_VIEWS_EXTENSIONS_EXTENSION_ACTION_VIEW_DELEGATE_H_
-
-namespace content {
-class WebContents;
-}
+#ifndef CHROME_BROWSER_UI_VIEWS_TOOLBAR_TOOLBAR_ACTION_VIEW_DELEGATE_H_
+#define CHROME_BROWSER_UI_VIEWS_TOOLBAR_TOOLBAR_ACTION_VIEW_DELEGATE_H_
 
 namespace views {
 class FocusManager;
 class MenuButton;
 class View;
 class Widget;
 }
 
-class ExtensionActionViewController;
+class ToolbarActionViewController;
 
-// The view that surrounds an ExtensionAction and owns the
-// ExtensionActionViewController. Since different actions can subclass
+// The view that surrounds an ToolbarAction and owns the
+// ToolbarActionViewController. Since different actions can subclass
 // different views, we don't derive views::View directly here.
-class ExtensionActionViewDelegate {
+class ToolbarActionViewDelegate {
  public:
   // Returns |this| as a view. We need this because our subclasses implement
   // different kinds of views, and inheriting View here is a really bad idea.
   virtual views::View* GetAsView() = 0;

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

Seems like you shouldn't have any view specific bits in this code. It should be
platform neutral.

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

I don't actually think that's possible for this implementation, since the
controller itself is a views implementation (and does things like run menus,
open popups, etc).  If we wanted this to be truly abstract, we could - but it'll
be a much bigger shift.  What I could think of if we wanted this would be to
shift all the platform-specific menu/popup logic from the controller into the
view delegate, so that we have something like:

ToolbarActionViewController (platform-independent)
ExtensionActionViewController (p-i)
ToolbarActionViewDelegate (p-i, has methods like ShowPopup(), RunMenu())
ToolbarActionViewDelegateViews (views-specific impl)
ToolbarActionViewDelegateCocoa (cocoa-specific impl)

This would be reasonably clean, but does lead to some complication between
sharing code between extension browser/page actions, and would result in a
*much* bigger patch.  (And, again, part of me wonders how much longer we'll have
the need for two platform implementations.)

WDYT?  Is it worth the investment to take that route?  Or is there a simpler
approach?

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

My main complaint is that consumers of the classes you have here shouldn't need
to know if the action is from extensions or non-extensions. I suspect you want
the same functionality on a mac, which is why I suggested making this p-i. If
that isn't the case, I'm ok with views types in the API.

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

I think I'd like to keep this as a views impl for the time being, and your main
concern should already be addressed (this class is fully ignorant of what type
of action it represents, as is its implementor, BrowserActionView).

 
   // Returns true if this view is being shown inside a menu.
   virtual bool IsShownInMenu() = 0;
 
   // Returns the FocusManager to use when registering accelerators.
   virtual views::FocusManager* GetFocusManagerForAccelerator() = 0;
 
   // Returns the parent for the associated context menu.
   virtual views::Widget* GetParentForContextMenu() = 0;
 
   // In some cases (such as when an action is shown in a menu), a substitute
-  // ExtensionActionViewController should be used for showing popups. This
+  // ToolbarActionViewController should be used for showing popups. This
   // returns the preferred control.
-  virtual ExtensionActionViewController* GetPreferredPopupViewController() = 0;
+  virtual ToolbarActionViewController* GetPreferredPopupViewController() = 0;
 
   // Returns the reference view for the extension action's popup.
   virtual views::View* GetReferenceViewForPopup() = 0;
 
   // Returns the MenuButton (if any) to use in showing the context menu (this
   // allows views code to update the pressed state of the button).
   virtual views::MenuButton* GetContextMenuButton() = 0;
 
-  // Returns the current web contents.
-  virtual content::WebContents* GetCurrentWebContents() = 0;
+  // Returns the current tab id.
+  virtual int GetCurrentTabId() const = 0;
 
   // Hides whatever popup is active (even if it's not this one).
   virtual void HideActivePopup() = 0;
 
   // Called when the icon is updated; this is forwarded from the icon factory.
   virtual void OnIconUpdated() = 0;
 
   // Called when a popup is shown. See ExecuteAction() for the definition of
   // |grant_tab_permissions|.
   virtual void OnPopupShown(bool grant_tab_permissions) {}
 
   // Does any additional cleanup after the popup is closed.
   virtual void CleanupPopup() {}
 
  protected:
-  virtual ~ExtensionActionViewDelegate() {}
+  virtual ~ToolbarActionViewDelegate() {}
 };
 
-#endif  // CHROME_BROWSER_UI_VIEWS_EXTENSIONS_EXTENSION_ACTION_VIEW_DELEGATE_H_
+#endif  // CHROME_BROWSER_UI_VIEWS_TOOLBAR_TOOLBAR_ACTION_VIEW_DELEGATE_H_