Action box menu

chrome/browser/ui/views/browser_action_view.h

Index: chrome/browser/ui/views/browser_action_view.h
diff --git a/chrome/browser/ui/views/browser_action_view.h b/chrome/browser/ui/views/browser_action_view.h
index 2cb8a93a1e318bbe8e6c50aff2940f27b11f0aa6..f6f564ea24711b43c8aa518448752ddcc378b9fc 100644
--- a/chrome/browser/ui/views/browser_action_view.h
+++ b/chrome/browser/ui/views/browser_action_view.h
@@ -11,9 +11,11 @@
 #include "content/public/browser/notification_observer.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/button/menu_button_listener.h"
+#include "ui/views/drag_controller.h"
 #include "ui/views/view.h"
 
-class BrowserActionsContainer;
+class Browser;
+class BrowserActionButton;
 class ExtensionAction;
 
 namespace views {
@@ -21,6 +23,64 @@ class MenuItemView;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// BrowserActionView
+// A single section in the browser action container. This contains the actual
+// BrowserActionButton, as well as the logic to paint the badge.
+class BrowserActionView : public views::View {
+ public:
+  class Delegate : public views::DragController {

[Peter Kasting, 2012/07/14 02:08:01]

Why is the delegate a DragController?  Either the delegate implementations
should directly inherit this themselves or you should comment here about why
this is and what functions you need.

[yefimt, 2012/07/17 18:20:37]

They could implement it by themself, but then BrowserActionButton will not be
able to call Drag* functions.

On 2012/07/14 02:08:01, Peter Kasting wrote:

+   public:
+    // Returns the browser this container is associated with.
+    virtual Browser* GetBrowser() const = 0;

[Peter Kasting, 2012/07/14 02:08:01]

Why add this getter instead of just passing the Browser* in?

[yefimt, 2012/07/17 18:20:37]

It is possible, I just didnt want to change existing implementation. Old code
used to call BrowserActionsContainer to get browser, now it does the same  to
whoever implements delagate

On 2012/07/14 02:08:01, Peter Kasting wrote:

+
+    // Returns the current tab's ID, or -1 if there is no current tab.
+    virtual int GetCurrentTabId() const = 0;
+
+    // Called when the user clicks on the browser action icon.
+    virtual void OnBrowserActionExecuted(BrowserActionButton* button) = 0;
+
+    // Called when a browser action becomes visible/hidden.
+    virtual void OnBrowserActionVisibilityChanged() = 0;
+
+    // Returns offset of a content inside the view.

[Peter Kasting, 2012/07/14 02:08:01]

Nit: "a content" is not grammatical, and regardless, I don't really understand
what this is saying.  What content in what view?

[yefimt, 2012/07/17 18:20:37]

Ok, I got a few comments about it, I guess I cannot find a better name. 
This class is a view, and it has some contents (a button). This function
supposed to return offset (position) of contents (button) relative to upper left
corner.

On 2012/07/14 02:08:01, Peter Kasting wrote:

+    virtual gfx::Size GetViewContentOffset() const = 0;
+
+   protected:
+    Delegate() {}
+    virtual ~Delegate() {}
+  };
+
+  BrowserActionView(const extensions::Extension* extension, Delegate* delegate);
+  virtual ~BrowserActionView();
+
+  BrowserActionButton* button() { return button_; }
+
+  // Allocates a canvas object on the heap and draws into it the icon for the
+  // view as well as the badge (if any). Caller is responsible for deleting the
+  // returned object.
+  gfx::Canvas* GetIconWithBadge();
+
+  // Overridden from views::View:
+  virtual void Layout() OVERRIDE;
+  virtual void GetAccessibleState(ui::AccessibleViewState* state) OVERRIDE;
+

[Peter Kasting, 2012/07/14 02:08:01]

Nit: Remove blank line

[yefimt, 2012/07/17 18:20:37]

Done.

+  virtual gfx::Size GetPreferredSize() OVERRIDE;
+
+ protected:
+  // Overridden from views::View to paint the badge on top of children.
+  virtual void PaintChildren(gfx::Canvas* canvas) OVERRIDE;
+
+ private:
+  // The container for this view.
+  Delegate* delegate_;
+
+  // The button this view contains.
+  BrowserActionButton* button_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserActionView);
+};
+
+////////////////////////////////////////////////////////////////////////////////
 // BrowserActionButton
 
 // The BrowserActionButton is a specialization of the MenuButton class.
@@ -32,7 +92,7 @@ class BrowserActionButton : public views::MenuButton,
                             public content::NotificationObserver {
  public:
   BrowserActionButton(const extensions::Extension* extension,
-                      BrowserActionsContainer* panel);
+                      BrowserActionView::Delegate* delegate);
 
   // Call this instead of delete.
   void Destroy();
@@ -88,11 +148,13 @@ class BrowserActionButton : public views::MenuButton,
   void SetButtonPushed();
   void SetButtonNotPushed();
 
+  void SetTooltipDisabled(bool disable_tooltip);
+
  protected:
   // Overridden from views::View:
   virtual void ViewHierarchyChanged(bool is_add,
-                                    View* parent,
-                                    View* child) OVERRIDE;
+    View* parent,

[Peter Kasting, 2012/07/14 02:08:01]

Nit: Why did you change the previous correct indentation to this incorrect
indentation?

[yefimt, 2012/07/17 18:20:37]

It is not exactly me, Visual Assist I have in devstudio re-formatted it when I
copy pasted that code, I fixed what I noticed but missed a few.
Fixed.

On 2012/07/14 02:08:01, Peter Kasting wrote:

+    View* child) OVERRIDE;
 
  private:
   virtual ~BrowserActionButton();
@@ -104,6 +166,9 @@ class BrowserActionButton : public views::MenuButton,
   // it is active.
   void MaybeUnregisterExtensionCommand(bool only_if_active);
 
+  // Returns tooltip for the button.
+  string16 GetTextForTooltip();
+
   // The browser action this view represents. The ExtensionAction is not owned
   // by this class.
   ExtensionAction* browser_action_;
@@ -120,57 +185,23 @@ class BrowserActionButton : public views::MenuButton,
   SkBitmap default_icon_;
 
   // The browser action shelf.
-  BrowserActionsContainer* panel_;
+  BrowserActionView::Delegate* delegate_;
 
   // The context menu.  This member is non-NULL only when the menu is shown.
   views::MenuItemView* context_menu_;
 
   content::NotificationRegistrar registrar_;
 
-  // The extension keybinding accelerator this browser action is listening for
+  // The extension key binding accelerator this browser action is listening for
   // (to show the popup).
   scoped_ptr<ui::Accelerator> keybinding_;
 
+  // True if tooltip was disabled.
+  bool disable_tooltip_;
+
   friend class base::DeleteHelper<BrowserActionButton>;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserActionButton);
 };
 
-
-////////////////////////////////////////////////////////////////////////////////
-// BrowserActionView
-// A single section in the browser action container. This contains the actual
-// BrowserActionButton, as well as the logic to paint the badge.
-
-class BrowserActionView : public views::View {
- public:
-  BrowserActionView(const extensions::Extension* extension,
-                    BrowserActionsContainer* panel);
-  virtual ~BrowserActionView();
-
-  BrowserActionButton* button() { return button_; }
-
-  // Allocates a canvas object on the heap and draws into it the icon for the
-  // view as well as the badge (if any). Caller is responsible for deleting the
-  // returned object.
-  gfx::Canvas* GetIconWithBadge();
-
-  // Overridden from views::View:
-  virtual void Layout() OVERRIDE;
-  virtual void GetAccessibleState(ui::AccessibleViewState* state) OVERRIDE;
-
- protected:
-  // Overridden from views::View to paint the badge on top of children.
-  virtual void PaintChildren(gfx::Canvas* canvas) OVERRIDE;
-
- private:
-  // The container for this view.
-  BrowserActionsContainer* panel_;
-
-  // The button this view contains.
-  BrowserActionButton* button_;
-
-  DISALLOW_COPY_AND_ASSIGN(BrowserActionView);
-};
-
 #endif  // CHROME_BROWSER_UI_VIEWS_BROWSER_ACTION_VIEW_H_