Update Status Icon menu when menu model changes (Linux Aura)

Status icon context menus currently do not update themselves when the menu model
changes or when the check state of a menu item changes. This patch fixes that
behavior.

BUG=263926,262395

Review URL: https://chromiumcodereview.appspot.com/20728003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@219964 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 430 insertions, 14 deletions

diff --git a/chrome/browser/status_icons/status_icon.cc b/chrome/browser/status_icons/status_icon.cc
index 52a36ab..458fb35 100644
--- a/chrome/browser/status_icons/status_icon.cc
+++ b/chrome/browser/status_icons/status_icon.cc
@@ -5,7 +5,6 @@
 #include "chrome/browser/status_icons/status_icon.h"
 
 #include "chrome/browser/status_icons/status_icon_observer.h"
-#include "ui/base/models/menu_model.h"
 
 StatusIcon::StatusIcon() {
 }
@@ -35,10 +34,10 @@ void StatusIcon::DispatchBalloonClickEvent() {
 }
 #endif
 
-void StatusIcon::SetContextMenu(ui::MenuModel* menu) {
+void StatusIcon::SetContextMenu(scoped_ptr<StatusIconMenuModel> menu) {
   // The UI may been showing a menu for the current model, don't destroy it
   // until we've notified the UI of the change.
-  scoped_ptr<ui::MenuModel> old_menu = context_menu_contents_.Pass();
-  context_menu_contents_.reset(menu);
-  UpdatePlatformContextMenu(menu);
+  scoped_ptr<StatusIconMenuModel> old_menu = context_menu_contents_.Pass();
+  context_menu_contents_ = menu.Pass();
+  UpdatePlatformContextMenu(context_menu_contents_.get());
 }
diff --git a/chrome/browser/status_icons/status_icon.h b/chrome/browser/status_icons/status_icon.h
index ccc8c2e..5e50961 100644
--- a/chrome/browser/status_icons/status_icon.h
+++ b/chrome/browser/status_icons/status_icon.h
@@ -9,15 +9,12 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/strings/string16.h"
+#include "chrome/browser/status_icons/status_icon_menu_model.h"
 
 namespace gfx {
 class ImageSkia;
 }
 
-namespace ui {
-class MenuModel;
-}
-
 class StatusIconObserver;
 
 class StatusIcon {
@@ -45,7 +42,7 @@ class StatusIcon {
 
   // Set the context menu for this icon. The icon takes ownership of the passed
   // context menu. Passing NULL results in no menu at all.
-  void SetContextMenu(ui::MenuModel* menu);
+  void SetContextMenu(scoped_ptr<StatusIconMenuModel> menu);
 
   // Adds/Removes an observer for clicks on the status icon. If an observer is
   // registered, then left clicks on the status icon will result in the observer
@@ -67,13 +64,13 @@ class StatusIcon {
   // Invoked after a call to SetContextMenu() to let the platform-specific
   // subclass update the native context menu based on the new model. If NULL is
   // passed, subclass should destroy the native context menu.
-  virtual void UpdatePlatformContextMenu(ui::MenuModel* model) = 0;
+  virtual void UpdatePlatformContextMenu(StatusIconMenuModel* model) = 0;
 
  private:
   ObserverList<StatusIconObserver> observers_;
 
   // Context menu, if any.
-  scoped_ptr<ui::MenuModel> context_menu_contents_;
+  scoped_ptr<StatusIconMenuModel> context_menu_contents_;
 
   DISALLOW_COPY_AND_ASSIGN(StatusIcon);
 };
diff --git a/chrome/browser/status_icons/status_icon_menu_model.cc b/chrome/browser/status_icons/status_icon_menu_model.cc
new file mode 100644
index 0000000..bfb7a4b
--- /dev/null
+++ b/chrome/browser/status_icons/status_icon_menu_model.cc
@@ -0,0 +1,181 @@
+// Copyright 2013 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.
+
+#include "chrome/browser/status_icons/status_icon_menu_model.h"
+
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "ui/base/accelerators/accelerator.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/gfx/image/image.h"
+
+struct StatusIconMenuModel::ItemState {
+  ItemState()
+      : checked(false),
+        enabled(true),
+        visible(true),
+        is_dynamic(false) {}
+  bool checked;
+  bool enabled;
+  bool visible;
+  bool is_dynamic;
+  ui::Accelerator accelerator;
+  base::string16 label;
+  base::string16 sublabel;
+  gfx::Image icon;
+};
+
+////////////////////////////////////////////////////////////////////////////////
+// StatusIconMenuModel::Delegate, public:
+
+void StatusIconMenuModel::Delegate::CommandIdHighlighted(int command_id) {
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// StatusIconMenuModel, public:
+
+StatusIconMenuModel::StatusIconMenuModel(Delegate* delegate)
+    : ui::SimpleMenuModel(this), delegate_(delegate) {
+}
+
+StatusIconMenuModel::~StatusIconMenuModel() {
+}
+
+void StatusIconMenuModel::SetCommandIdChecked(int command_id, bool checked) {
+  item_states_[command_id].checked = checked;
+  NotifyMenuStateChanged();
+}
+
+void StatusIconMenuModel::SetCommandIdEnabled(int command_id, bool enabled) {
+  item_states_[command_id].enabled = enabled;
+  NotifyMenuStateChanged();
+}
+
+void StatusIconMenuModel::SetCommandIdVisible(int command_id, bool visible) {
+  item_states_[command_id].visible = visible;
+  NotifyMenuStateChanged();
+}
+
+void StatusIconMenuModel::SetAcceleratorForCommandId(
+    int command_id, const ui::Accelerator* accelerator) {
+  item_states_[command_id].accelerator = *accelerator;
+  NotifyMenuStateChanged();
+}
+
+void StatusIconMenuModel::ChangeLabelForCommandId(int command_id,
+                                                  const base::string16& label) {
+  item_states_[command_id].is_dynamic = true;
+  item_states_[command_id].label = label;
+  NotifyMenuStateChanged();
+}
+
+void StatusIconMenuModel::ChangeSublabelForCommandId(
+    int command_id, const base::string16& sublabel) {
+  item_states_[command_id].is_dynamic = true;
+  item_states_[command_id].sublabel = sublabel;
+  NotifyMenuStateChanged();
+}
+
+void StatusIconMenuModel::ChangeIconForCommandId(
+    int command_id, const gfx::Image& icon) {
+  item_states_[command_id].is_dynamic = true;
+  item_states_[command_id].icon = icon;
+  NotifyMenuStateChanged();
+}
+
+void StatusIconMenuModel::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void StatusIconMenuModel::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+bool StatusIconMenuModel::IsCommandIdChecked(int command_id) const {
+  ItemStateMap::const_iterator iter = item_states_.find(command_id);
+  if (iter != item_states_.end())
+    return iter->second.checked;
+  return false;
+}
+
+bool StatusIconMenuModel::IsCommandIdEnabled(int command_id) const {
+  ItemStateMap::const_iterator iter = item_states_.find(command_id);
+  if (iter != item_states_.end())
+    return iter->second.enabled;
+  return true;
+}
+
+bool StatusIconMenuModel::IsCommandIdVisible(int command_id) const {
+  ItemStateMap::const_iterator iter = item_states_.find(command_id);
+  if (iter != item_states_.end())
+    return iter->second.visible;
+  return true;
+}
+
+bool StatusIconMenuModel::GetAcceleratorForCommandId(
+    int command_id, ui::Accelerator* accelerator) {
+  ItemStateMap::const_iterator iter = item_states_.find(command_id);
+  if (iter != item_states_.end() &&
+      iter->second.accelerator.key_code() != ui::VKEY_UNKNOWN) {
+    *accelerator = iter->second.accelerator;
+    return true;
+  }
+  return false;
+}
+
+bool StatusIconMenuModel::IsItemForCommandIdDynamic(int command_id) const {
+  ItemStateMap::const_iterator iter = item_states_.find(command_id);
+  if (iter != item_states_.end())
+    return iter->second.is_dynamic;
+  return false;
+}
+
+base::string16 StatusIconMenuModel::GetLabelForCommandId(int command_id) const {
+  ItemStateMap::const_iterator iter = item_states_.find(command_id);
+  if (iter != item_states_.end())
+    return iter->second.label;
+  return base::string16();
+}
+
+base::string16 StatusIconMenuModel::GetSublabelForCommandId(
+    int command_id) const {
+  ItemStateMap::const_iterator iter = item_states_.find(command_id);
+  if (iter != item_states_.end())
+    return iter->second.sublabel;
+  return base::string16();
+}
+
+bool StatusIconMenuModel::GetIconForCommandId(int command_id,
+                                              gfx::Image* image_skia) const {
+  ItemStateMap::const_iterator iter = item_states_.find(command_id);
+  if (iter != item_states_.end() && !iter->second.icon.IsEmpty()) {
+    *image_skia = iter->second.icon;
+    return true;
+  }
+  return false;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// StatusIconMenuModel, protected:
+
+void StatusIconMenuModel::MenuItemsChanged() {
+  NotifyMenuStateChanged();
+}
+
+void StatusIconMenuModel::NotifyMenuStateChanged() {
+  FOR_EACH_OBSERVER(Observer, observer_list_, OnMenuStateChanged());
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// StatusIconMenuModel, private:
+
+void StatusIconMenuModel::CommandIdHighlighted(int command_id) {
+  if (delegate_)
+    delegate_->CommandIdHighlighted(command_id);
+}
+
+void StatusIconMenuModel::ExecuteCommand(int command_id, int event_flags) {
+  if (delegate_)
+    delegate_->ExecuteCommand(command_id, event_flags);
+}
diff --git a/chrome/browser/status_icons/status_icon_menu_model.h b/chrome/browser/status_icons/status_icon_menu_model.h
new file mode 100644
index 0000000..fa43874
--- /dev/null
+++ b/chrome/browser/status_icons/status_icon_menu_model.h
@@ -0,0 +1,119 @@
+// Copyright 2013 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_STATUS_ICONS_STATUS_ICON_MENU_MODEL_H_
+#define CHROME_BROWSER_STATUS_ICONS_STATUS_ICON_MENU_MODEL_H_
+
+#include <map>
+
+#include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
+#include "ui/base/models/simple_menu_model.h"
+
+namespace gfx {
+class Image;
+}
+
+class StatusIconMenuModelTest;
+
+// StatusIconMenuModel contains the state of the SimpleMenuModel as well as that
+// of its delegate. This is done so that we can easily identify when the menu
+// model state has changed and can tell the status icon to update the menu. This
+// is necessary some platforms which do not notify us before showing the menu
+// (like Ubuntu Unity).
+class StatusIconMenuModel
+    : public ui::SimpleMenuModel,
+      public ui::SimpleMenuModel::Delegate,
+      public base::SupportsWeakPtr<StatusIconMenuModel> {
+ public:
+  class Delegate {
+   public:
+    // Notifies the delegate that the item with the specified command id was
+    // visually highlighted within the menu.
+    virtual void CommandIdHighlighted(int command_id);
+
+    // Performs the action associates with the specified command id.
+    // The passed |event_flags| are the flags from the event which issued this
+    // command and they can be examined to find modifier keys.
+    virtual void ExecuteCommand(int command_id, int event_flags) = 0;
+
+   protected:
+    virtual ~Delegate() {}
+  };
+
+  class Observer {
+   public:
+    // Invoked when the menu model has changed.
+    virtual void OnMenuStateChanged() {}
+
+   protected:
+    virtual ~Observer() {}
+  };
+
+  // The Delegate can be NULL.
+  explicit StatusIconMenuModel(Delegate* delegate);
+  virtual ~StatusIconMenuModel();
+
+  // Methods for seting the state of specific command ids.
+  void SetCommandIdChecked(int command_id, bool checked);
+  void SetCommandIdEnabled(int command_id, bool enabled);
+  void SetCommandIdVisible(int command_id, bool visible);
+
+  // Sets the accelerator for the specified command id.
+  void SetAcceleratorForCommandId(
+      int command_id, const ui::Accelerator* accelerator);
+
+  // Calling any of these "change" methods will mark the menu item as "dynamic"
+  // (see menu_model.h:IsItemDynamicAt) which many platforms take as a cue to
+  // refresh the label, sublabel and icon of the menu item each time the menu is
+  // shown.
+  void ChangeLabelForCommandId(int command_id, const base::string16& label);
+  void ChangeSublabelForCommandId(
+      int command_id, const base::string16& sublabel);
+  void ChangeIconForCommandId(int command_id, const gfx::Image& icon);
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  // Overridden from ui::SimpleMenuModel::Delegate:
+  virtual bool IsCommandIdChecked(int command_id) const OVERRIDE;
+  virtual bool IsCommandIdEnabled(int command_id) const OVERRIDE;
+  virtual bool IsCommandIdVisible(int command_id) const OVERRIDE;
+  virtual bool GetAcceleratorForCommandId(
+      int command_id, ui::Accelerator* accelerator) OVERRIDE;
+  virtual bool IsItemForCommandIdDynamic(int command_id) const OVERRIDE;
+  virtual base::string16 GetLabelForCommandId(int command_id) const OVERRIDE;
+  virtual base::string16 GetSublabelForCommandId(int command_id) const OVERRIDE;
+  virtual bool GetIconForCommandId(int command_id, gfx::Image* icon) const
+      OVERRIDE;
+
+ protected:
+  // Overriden from ui::SimpleMenuModel:
+  virtual void MenuItemsChanged() OVERRIDE;
+
+  void NotifyMenuStateChanged();
+
+  void set_delegate(Delegate* delegate) { delegate_ = delegate; }
+  Delegate* delegate() { return delegate_; }
+
+ private:
+  // Overridden from ui::SimpleMenuModel::Delegate:
+  virtual void CommandIdHighlighted(int command_id) OVERRIDE;
+  virtual void ExecuteCommand(int command_id, int event_flags) OVERRIDE;
+
+  struct ItemState;
+
+  // Map the properties to the command id (used as key).
+  typedef std::map<int, ItemState> ItemStateMap;
+
+  ItemStateMap item_states_;
+
+  ObserverList<Observer> observer_list_;
+
+  Delegate* delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(StatusIconMenuModel);
+};
+
+#endif  // CHROME_BROWSER_STATUS_ICONS_STATUS_ICON_MENU_MODEL_H_
diff --git a/chrome/browser/status_icons/status_icon_menu_model_unittest.cc b/chrome/browser/status_icons/status_icon_menu_model_unittest.cc
new file mode 100644
index 0000000..a81d0fa
--- /dev/null
+++ b/chrome/browser/status_icons/status_icon_menu_model_unittest.cc
@@ -0,0 +1,118 @@
+// Copyright 2013 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.
+
+#include "chrome/browser/status_icons/status_icon_menu_model.h"
+
+#include "base/compiler_specific.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/status_icons/status_icon.h"
+#include "chrome/browser/status_icons/status_tray.h"
+#include "grit/chrome_unscaled_resources.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/accelerators/accelerator.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/image/image.h"
+
+class StatusIconMenuModelTest : public testing::Test,
+                                public StatusIconMenuModel::Observer {
+ public:
+  virtual void SetUp() OVERRIDE {
+    menu_.reset(new StatusIconMenuModel(NULL));
+    menu_->AddObserver(this);
+    changed_count_ = 0;
+  }
+
+  virtual void TearDown() OVERRIDE {
+    menu_->RemoveObserver(this);
+  }
+
+  virtual int changed_count() {
+    return changed_count_;
+  }
+
+  StatusIconMenuModel* menu_model() {
+    return menu_.get();
+  }
+
+ private:
+  virtual void OnMenuStateChanged() OVERRIDE {
+    ++changed_count_;
+  }
+
+  scoped_ptr<StatusIconMenuModel> menu_;
+  int changed_count_;
+};
+
+TEST_F(StatusIconMenuModelTest, ToggleBooleanProperties) {
+  menu_model()->AddItem(0, ASCIIToUTF16("foo"));
+
+  menu_model()->SetCommandIdChecked(0, true);
+  EXPECT_TRUE(menu_model()->IsCommandIdChecked(0));
+  menu_model()->SetCommandIdChecked(0, false);
+  EXPECT_FALSE(menu_model()->IsCommandIdChecked(0));
+
+  menu_model()->SetCommandIdEnabled(0, true);
+  EXPECT_TRUE(menu_model()->IsCommandIdEnabled(0));
+  menu_model()->SetCommandIdEnabled(0, false);
+  EXPECT_FALSE(menu_model()->IsCommandIdEnabled(0));
+
+  menu_model()->SetCommandIdVisible(0, true);
+  EXPECT_TRUE(menu_model()->IsCommandIdVisible(0));
+  menu_model()->SetCommandIdVisible(0, false);
+  EXPECT_FALSE(menu_model()->IsCommandIdVisible(0));
+
+  // Menu state should have changed 7 times in this test.
+  EXPECT_EQ(7, changed_count());
+}
+
+TEST_F(StatusIconMenuModelTest, SetProperties) {
+  menu_model()->AddItem(0, ASCIIToUTF16("foo1"));
+  menu_model()->AddItem(1, ASCIIToUTF16("foo2"));
+
+  ui::Accelerator test_accel(ui::VKEY_A, ui::EF_NONE);
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  gfx::Image test_image1(*rb.GetImageSkiaNamed(IDR_STATUS_TRAY_ICON));
+  gfx::Image test_image2(*rb.GetImageSkiaNamed(IDR_STATUS_TRAY_ICON_PRESSED));
+  ui::Accelerator accel_arg;
+  gfx::Image image_arg;
+
+  EXPECT_FALSE(menu_model()->GetAcceleratorForCommandId(0, &accel_arg));
+  EXPECT_FALSE(menu_model()->GetIconForCommandId(0, &image_arg));
+  EXPECT_FALSE(menu_model()->IsItemForCommandIdDynamic(0));
+
+  // Set the accelerator and label for the first menu item.
+  menu_model()->SetAcceleratorForCommandId(0, &test_accel);
+  EXPECT_TRUE(menu_model()->GetAcceleratorForCommandId(0, &accel_arg));
+  EXPECT_EQ(test_accel, accel_arg);
+
+  // Try setting label and changing it. Also ensure that menu item is marked
+  // dynamic since the label has changed.
+  menu_model()->ChangeLabelForCommandId(0, ASCIIToUTF16("label1"));
+  EXPECT_TRUE(menu_model()->IsItemForCommandIdDynamic(0));
+  EXPECT_EQ(ASCIIToUTF16("label1"), menu_model()->GetLabelForCommandId(0));
+  menu_model()->ChangeLabelForCommandId(0, ASCIIToUTF16("label2"));
+  EXPECT_EQ(ASCIIToUTF16("label2"), menu_model()->GetLabelForCommandId(0));
+
+  // Set the sublabel and icon image for the second menu item.
+  menu_model()->ChangeSublabelForCommandId(1, ASCIIToUTF16("sublabel"));
+  EXPECT_EQ(ASCIIToUTF16("sublabel"), menu_model()->GetSublabelForCommandId(1));
+
+  // Try setting icon image and changing it.
+  menu_model()->ChangeIconForCommandId(1, test_image1);
+  EXPECT_TRUE(menu_model()->GetIconForCommandId(1, &image_arg));
+  EXPECT_EQ(image_arg.ToImageSkia(), test_image1.ToImageSkia());
+  menu_model()->ChangeIconForCommandId(1, test_image2);
+  EXPECT_TRUE(menu_model()->GetIconForCommandId(1, &image_arg));
+  EXPECT_EQ(image_arg.ToImageSkia(), test_image2.ToImageSkia());
+
+  // Ensure changes to one menu item does not affect the other menu item.
+  EXPECT_FALSE(menu_model()->GetAcceleratorForCommandId(1, &accel_arg));
+  EXPECT_EQ(string16(), menu_model()->GetLabelForCommandId(1));
+  EXPECT_EQ(string16(), menu_model()->GetSublabelForCommandId(0));
+  EXPECT_FALSE(menu_model()->GetIconForCommandId(0, &image_arg));
+
+  // Menu state should have changed 8 times in this test.
+  EXPECT_EQ(8, changed_count());
+}
diff --git a/chrome/browser/status_icons/status_icon_unittest.cc b/chrome/browser/status_icons/status_icon_unittest.cc
index 8078665..cdc22b7 100644
--- a/chrome/browser/status_icons/status_icon_unittest.cc
+++ b/chrome/browser/status_icons/status_icon_unittest.cc
@@ -21,7 +21,8 @@ class TestStatusIcon : public StatusIcon {
   virtual void SetImage(const gfx::ImageSkia& image) OVERRIDE {}
   virtual void SetPressedImage(const gfx::ImageSkia& image) OVERRIDE {}
   virtual void SetToolTip(const string16& tool_tip) OVERRIDE {}
-  virtual void UpdatePlatformContextMenu(ui::MenuModel* menu) OVERRIDE {}
+  virtual void UpdatePlatformContextMenu(
+      StatusIconMenuModel* menu) OVERRIDE {}
   virtual void DisplayBalloon(const gfx::ImageSkia& icon,
                               const string16& title,
                               const string16& contents) OVERRIDE {}
diff --git a/chrome/browser/status_icons/status_tray_unittest.cc b/chrome/browser/status_icons/status_tray_unittest.cc
index 238d8b7..674c8fc 100644
--- a/chrome/browser/status_icons/status_tray_unittest.cc
+++ b/chrome/browser/status_icons/status_tray_unittest.cc
@@ -19,7 +19,8 @@ class MockStatusIcon : public StatusIcon {
   virtual void DisplayBalloon(const gfx::ImageSkia& icon,
                               const string16& title,
                               const string16& contents) OVERRIDE {}
-  virtual void UpdatePlatformContextMenu(ui::MenuModel* menu) OVERRIDE {}
+  virtual void UpdatePlatformContextMenu(
+      StatusIconMenuModel* menu) OVERRIDE {}
 };
 
 class TestStatusTray : public StatusTray {