Native menu implementation for bug 5679. Followup to http://codereview.chromium.org/2928005/

views/controls/menu/native_menu_win.cc

Index: views/controls/menu/native_menu_win.cc
diff --git a/views/controls/menu/native_menu_win.cc b/views/controls/menu/native_menu_win.cc
index f639a2e6e878d43c411c03745891c11ccb026238..9f82c082548f809af86a7377fc64dbcb62b3ab74 100644
--- a/views/controls/menu/native_menu_win.cc
+++ b/views/controls/menu/native_menu_win.cc
@@ -5,6 +5,7 @@
 #include "views/controls/menu/native_menu_win.h"
 
 #include "base/logging.h"
+#include "base/message_loop.h"
 #include "base/stl_util-inl.h"
 #include "base/win/wrapped_window_proc.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -310,6 +311,7 @@ NativeMenuWin::NativeMenuWin(ui::MenuModel* model, HWND system_menu_for)
       system_menu_for_(system_menu_for),
       first_item_index_(0),
       menu_action_(MENU_ACTION_NONE) {
+  model_->SetMenuModelDelegate(this);

[sky, 2011/03/24 22:39:29]

I don't see you ever removing the delegate.

[Avi (use Gerrit), 2011/03/25 00:19:35]

Good point. Ditto for all implementations.

[dill, 2011/03/25 01:47:47]

I should do model_->SetMenuModelDelegate(NULL); in the deconstructor, correct?

[dill, 2011/03/25 19:06:56]

Ah, well that sounded good. But model_ outlives the native menu implementation,
so the next instantiation of the native menu from the scoped_ptr calls the
constructor for the new menu then calls the deconstructor of the old menu and
that sets the delegate to null for the new native menu. 

In windows I am removing the delegate at the end of RunMenuAt which blocks at
TrackPopupMenuEx until the menu closes. For Linux, the only way to do it seems
to be in OnMenuHidden which is called when the menu is dismissed or and item is
selected. Not sure about Mac yet. 

 }
 
 NativeMenuWin::~NativeMenuWin() {
@@ -339,6 +341,9 @@ void NativeMenuWin::RunMenuAt(const gfx::Point& point, int alignment) {
   // opening of the menu.
   listeners_called_ = false;
 
+  // Retrieving favicons from history requires nestable tasks.
+  MessageLoop::ScopedNestableTaskAllower allow(MessageLoop::current());
+
   // Command dispatch is done through WM_MENUCOMMAND, handled by the host
   // window.
   HWND hwnd = host_window_->hwnd();
@@ -415,6 +420,15 @@ void NativeMenuWin::SetMinimumWidth(int width) {
   NOTIMPLEMENTED();
 }
 
+void NativeMenuWin::OnIconChanged(int model_index) {
+  // Translate the model_index to the menu_index.
+  int first_item_index_ = model_->GetFirstItemIndex(GetNativeMenu());
+  int menu_index = model_index + first_item_index_;
+  // Unfortunately this repaints the entire menu.
+  SetMenuItemState(menu_index, model_->IsEnabledAt(model_index),
+                   model_->IsItemCheckedAt(model_index), false);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeMenuWin, private: