Add notification mechanism when BrowserActionButton's icon has been updated.

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

Index: chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc
diff --git a/chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc b/chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc
index abf02a06621ecea84b8c40aff572626a21a1918b..dde91eb54553b9eb83e6417d75ada241e73f04b4 100644
--- a/chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc
+++ b/chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc
@@ -21,6 +21,38 @@
 #include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/widget/widget.h"
 
+// In the browser actions container's chevron menu, a MenuItemView's icon comes
+// from BrowserActionView::GetIconWithBadge() (which comes from
+// BrowserActionButton's icon) when the MenuItemView is created. But,
+// BrowserActionButton's icon may be not yet loaded in that timing because it

[Finnur, 2014/02/10 10:41:41]

s/in that timing/in time/

+// is read from filesystem in another thread.
+// The IconStateSensitiveMenuItemViewHelper will change the MenuItemView's

[Finnur, 2014/02/10 10:41:41]

s/change/update/

+// icon when the BrowserActionButton's icon has been updated.
+class IconStateSensitiveMenuItemViewHelper

[Finnur, 2014/02/10 10:41:41]

Nit: This name is a bit long and unwieldy. How about just IconUpdater? (don't
forget to update line 29 also)

+    : public BrowserActionButton::IconObserver,
+      public views::MenuItemView::LifecycleObserver {
+ public:
+  IconStateSensitiveMenuItemViewHelper(views::MenuItemView* host,
+                                       BrowserActionButton* button)
+      : host_(host),
+        button_(button) {
+    DCHECK(host);
+    DCHECK(button);
+    button->set_icon_observer(this);
+  }
+  virtual ~IconStateSensitiveMenuItemViewHelper() {}
+  virtual void OnIconUpdated(const gfx::ImageSkia& icon) OVERRIDE {

[Finnur, 2014/02/10 10:41:41]

Add a comment:

// BrowserActionButton::IconObserver methods:

+    host_->SetIcon(icon);
+  }
+  virtual void OnDeleted(views::MenuItemView* view) OVERRIDE {

[Finnur, 2014/02/10 10:41:41]

Add a comment:

// views::MenuItemView::LifecycleObserver methods:

+    button_->set_icon_observer(NULL);
+    delete this;
+  }
+ private:
+  views::MenuItemView* host_;

[Finnur, 2014/02/10 10:41:41]

Not sure I'd use the word host_ here. Why not just menu_item_view_?

+  BrowserActionButton* button_;

[Finnur, 2014/02/10 10:41:41]

Document these two members.

+};

[Finnur, 2014/02/10 10:41:41]

Add DISABLE_COPY_AND_ASSIGN(IconStateSensitiveMenuItemViewHelper)
... or IconUpdater, as appropriate, to the end of this class.

+
 BrowserActionOverflowMenuController::BrowserActionOverflowMenuController(
     BrowserActionsContainer* owner,
     Browser* browser,
@@ -42,7 +74,7 @@ BrowserActionOverflowMenuController::BrowserActionOverflowMenuController(
   size_t command_id = 1;  // Menu id 0 is reserved, start with 1.
   for (size_t i = start_index; i < views_->size(); ++i) {
     BrowserActionView* view = (*views_)[i];
-    menu_->AppendMenuItemWithIcon(
+    views::MenuItemView* menu_item = menu_->AppendMenuItemWithIcon(
         command_id,
         base::UTF8ToUTF16(view->button()->extension()->name()),
         view->GetIconWithBadge());
@@ -54,6 +86,10 @@ BrowserActionOverflowMenuController::BrowserActionOverflowMenuController(
         GetTitle(owner_->GetCurrentTabId()));
     menu_->SetTooltip(tooltip, command_id);
 
+    IconStateSensitiveMenuItemViewHelper* helper =
+        new IconStateSensitiveMenuItemViewHelper(menu_item, view->button());
+    menu_item->set_lifecycle_observer(helper);
+
     ++command_id;
   }
 }