Separate StatusAreaView from StatusAreaViewChromeos

chrome/browser/chromeos/status/status_area_button.h

Index: chrome/browser/chromeos/status/status_area_button.h
diff --git a/chrome/browser/chromeos/status/status_area_button.h b/chrome/browser/chromeos/status/status_area_button.h
index 1aee80086091c861b9c7aa0215efad8091f62614..ad9ecaecfa4b5193a4f54370146fd821307a0462 100644
--- a/chrome/browser/chromeos/status/status_area_button.h
+++ b/chrome/browser/chromeos/status/status_area_button.h
@@ -7,17 +7,52 @@
 #pragma once
 
 #include "base/string16.h"
-#include "chrome/browser/chromeos/status/status_area_host.h"
 #include "views/controls/button/menu_button.h"
 #include "views/controls/menu/view_menu_delegate.h"
 
-namespace chromeos {
+namespace gfx {
+class Font;
+}
 
 // Button to be used to represent status and allow menus to be popped up.
 // Shows current button state by drawing a border around the current icon.
 class StatusAreaButton : public views::MenuButton {
  public:
-  StatusAreaButton(StatusAreaHost* host,
+  // Different text styles for different types of backgrounds.
+  enum TextStyle {
+    WHITE_PLAIN,
+    GRAY_PLAIN,
+    WHITE_HALOED,
+    GRAY_EMBOSSED
+  };
+
+  class Delegate {
+   public:
+    Delegate() {}

[tfarina, 2011/11/07 15:45:44]

You don't need this.

[stevenjb, 2011/11/08 00:24:22]

Was needed for DISALLOW_COPY_AND_ASSIGN, but see note below.

+
+    // |command_id| can be any int, passed from the button to the delegate.
+    virtual bool ShouldExecuteStatusAreaCommand(
+        const views::View* button_view, int command_id) const = 0;
+
+    virtual void ExecuteStatusAreaCommand(
+        const views::View* button_view, int command_id) = 0;
+
+    // Contextually adjust the button appearance.
+    virtual int GetFontStyle(const gfx::Font& font) const = 0;

[DaveMoore, 2011/11/07 15:36:18]

This would read better as ModifyFont() and return modified  a potentially
differnt Font, that typically would be the same as what was passed in.

[stevenjb, 2011/11/08 00:24:22]

Done, but named this GetStatusAreaFont(), and renamed GetTextStyle() as
GetStatusAreaTextStyle() for consistency and clarity.

+
+    virtual TextStyle GetTextStyle() const = 0;
+
+    // Handle visibility changes (e.g. resize the status area).
+    virtual void ButtonVisibilityChanged(views::View* button_view) = 0;
+
+   protected:
+    virtual ~Delegate() {}

[DaveMoore, 2011/11/07 15:36:18]

The virtual destructor shouldn't be inlined...move the implementation to the .cc
file.

[tfarina, 2011/11/07 15:45:44]

Why?

[stevenjb, 2011/11/08 00:24:22]

After some discussion I believe that it is OK (and preferred) to inline the
destructor of a pure interface class, since it has no members and no
implementation.

+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Delegate);

[tfarina, 2011/11/07 15:45:44]

Nor this.

[stevenjb, 2011/11/08 00:24:22]

We are inconsistent as to whether we need this in interface classes, and
opinions on the matter vary. I prefer conciseness so will remove both this and
the constructor.

+  };
+
+  StatusAreaButton(Delegate* button_delegate,
                    views::ViewMenuDelegate* menu_delegate);
   virtual ~StatusAreaButton() {}
   virtual void PaintButton(gfx::Canvas* canvas, PaintButtonMode mode);
@@ -26,10 +61,6 @@ class StatusAreaButton : public views::MenuButton {
   // text content so that the button size would fit the new text size.
   virtual void SetText(const string16& text);
 
-  void set_use_menu_button_paint(bool use_menu_button_paint) {
-    use_menu_button_paint_ = use_menu_button_paint;
-  }
-
   // views::MenuButton overrides.
   virtual bool Activate() OVERRIDE;
 
@@ -40,11 +71,13 @@ class StatusAreaButton : public views::MenuButton {
   virtual void SetVisible(bool visible) OVERRIDE;
   virtual bool HitTest(const gfx::Point& l) const OVERRIDE;
 
-  // Controls whether or not this status area button is able to be pressed.
-  void set_active(bool active) { active_ = active; }
-  bool active() const { return active_; }
+  void set_menu_active(bool active) { menu_active_ = active; }
+  bool menu_active() const { return menu_active_; }
 
  protected:
+  Delegate* delegate() { return delegate_; }
+  const Delegate* delegate() const { return delegate_; }
+
   // Subclasses should override these methods to return the correct dimensions.
   virtual int icon_height();
   virtual int icon_width();
@@ -53,26 +86,20 @@ class StatusAreaButton : public views::MenuButton {
   // The padding is added to both the left and right side.
   virtual int horizontal_padding();
 
-  // True if the button wants to use views::MenuButton drawings.
-  bool use_menu_button_paint_;
-
   // Insets to use for this button.
   gfx::Insets insets_;
 
-  // Indicates when this button can be pressed.  Independent of
-  // IsEnabled state, so that when IsEnabled is true, this can still
-  // be false, and vice versa.
-  bool active_;
-
-  // The status area host,
-  StatusAreaHost* host_;
+  // Controls whether or not the menu can be activated. This is independent of
+  // IsEnabled state, so that we can prevent the menu from appearing without
+  // affecting the appearance of the button.
+  bool menu_active_;
 
  private:
   void UpdateTextStyle();
 
+  Delegate* delegate_;
+
   DISALLOW_COPY_AND_ASSIGN(StatusAreaButton);
 };
 
-}  // namespace chromeos
-
 #endif  // CHROME_BROWSER_CHROMEOS_STATUS_STATUS_AREA_BUTTON_H_