Change status button menu implementation from Menu2 to MenuItemView.

chrome/browser/chromeos/status/network_menu.cc

Index: chrome/browser/chromeos/status/network_menu.cc
diff --git a/chrome/browser/chromeos/status/network_menu.cc b/chrome/browser/chromeos/status/network_menu.cc
index d95148c2f6badf8005c709b0e2d8dacdfd998e62..368973aff1d97557f9039629c10ae901d665afb9 100644
--- a/chrome/browser/chromeos/status/network_menu.cc
+++ b/chrome/browser/chromeos/status/network_menu.cc
@@ -21,9 +21,13 @@
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas_skia.h"
 #include "ui/gfx/skbitmap_operations.h"
-#include "views/controls/menu/menu_2.h"
+#include "views/controls/menu/menu_item_view.h"
+#include "views/controls/menu/submenu_view.h"
+#include "views/widget/widget.h"
 #include "views/window/window.h"
 
+using views::MenuItemView;
+
 namespace {
 // Constants passed to Javascript:
 static const char* kNetworkTypeEthernet = "ethernet";
@@ -89,7 +93,8 @@ SkBitmap NetworkMenu::kAnimatingImagesBlack[kNumAnimatingImages];
 
 NetworkMenu::NetworkMenu()
     : min_width_(-1) {
-  network_menu_.reset(new views::Menu2(this));
+  network_menu_.reset(new views::MenuItemView(this));
+  network_menu_->set_has_icons(true);
 }
 
 NetworkMenu::~NetworkMenu() {
@@ -255,50 +260,40 @@ bool NetworkMenu::ConnectToNetworkAt(int index,
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// NetworkMenu, ui::MenuModel implementation:
-
-int NetworkMenu::GetItemCount() const {
-  return static_cast<int>(menu_items_.size());
-}
-
-ui::MenuModel::ItemType NetworkMenu::GetTypeAt(int index) const {
-  return menu_items_[index].type;
-}
-
-string16 NetworkMenu::GetLabelAt(int index) const {
-  return menu_items_[index].label;
+// NetworkMenu, views::MenuItemView implementation:
+std::wstring NetworkMenu::GetLabel(int id) const {
+  DCHECK(id >= 0 && id < static_cast<int>(menu_items_.size()));

[oshima, 2011/04/14 17:27:31]

use two DCHECK_LE (or _GT)

[rhashimoto, 2011/04/14 18:27:04]

Done.

+  return UTF16ToWide(menu_items_[id].label);
 }
 
-const gfx::Font* NetworkMenu::GetLabelFontAt(int index) const {
-  return (menu_items_[index].flags & FLAG_ASSOCIATED) ?
+const gfx::Font* NetworkMenu::GetLabelFont(int id) const
+{
+  return (menu_items_[id].flags & FLAG_ASSOCIATED) ?
       &ResourceBundle::GetSharedInstance().GetFont(ResourceBundle::BoldFont) :
-      NULL;
+      &ResourceBundle::GetSharedInstance().GetFont(ResourceBundle::BaseFont);
 }
 
-bool NetworkMenu::IsItemCheckedAt(int index) const {
-  // All ui::MenuModel::TYPE_CHECK menu items are checked.
-  return true;
+bool NetworkMenu::IsCommandEnabled(int id) const {
+  if (id < static_cast<int>(menu_items_.size()))
+    return !(menu_items_[id].flags & FLAG_DISABLED);
+  else
+    return false;

[oshima, 2011/04/14 17:27:31]

can this happen? or should we use DCHECK?

[rhashimoto, 2011/04/14 18:27:04]

This can happen.  This method is called with 0 before any menu items are added. 
Perhaps for showing the empty menu?

[oshima, 2011/04/15 22:26:51]

Hmm, that sounds odd. Can you check why this is happening?

[rhashimoto, 2011/04/15 23:44:32]

Commands are ids, not indices, and there is one namespace for commands for each
MenuItemView hierarchy - a menu is defined by a tree of MenuItemView instances
with the root instance a container for the rest.

As each MenuItemView instance is initialized, the delegate is queried to see if
it is enabled.  We get a query for the root instance (command id 0) before the
rest of the menu items are populated.

[oshima, 2011/04/15 23:52:19]

Yes, but i still don't understand why we're doing it. There may be legit reason,
but curious to know why.

 }
 
-bool NetworkMenu::GetIconAt(int index, SkBitmap* icon) {
-  if (!menu_items_[index].icon.empty()) {
-    *icon = menu_items_[index].icon;
-    return true;
-  }
-  return false;
+bool NetworkMenu::IsItemChecked(int id) const {
+  DCHECK(id >= 0 && id < static_cast<int>(menu_items_.size()));

[oshima, 2011/04/14 17:27:31]

same here

[rhashimoto, 2011/04/14 18:27:04]

Done.

+  return menu_items_[id].type == ui::MenuModel::TYPE_CHECK;
 }
 
-bool NetworkMenu::IsEnabledAt(int index) const {
-  return !(menu_items_[index].flags & FLAG_DISABLED);
-}
+void NetworkMenu::ExecuteCommand(int id) {
+  DCHECK(id >= 0 && id < static_cast<int>(menu_items_.size()));

[oshima, 2011/04/14 17:27:31]

same here

[rhashimoto, 2011/04/14 18:27:04]

Done.

 
-void NetworkMenu::ActivatedAt(int index) {
   // When we are refreshing the menu, ignore menu item activation.
   if (refreshing_menu_)
     return;
 
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
-  int flags = menu_items_[index].flags;
+  int flags = menu_items_[id].flags;
   if (flags & FLAG_OPTIONS) {
     OpenButtonOptions();
   } else if (flags & FLAG_TOGGLE_ETHERNET) {
@@ -316,28 +311,43 @@ void NetworkMenu::ActivatedAt(int index) {
       ShowTabbedNetworkSettings(cros->ethernet_network());
     }
   } else if (flags & FLAG_WIFI) {
-    ConnectToNetworkAt(index, std::string(), std::string(), -1);
+    ConnectToNetworkAt(id, std::string(), std::string(), -1);
   } else if (flags & FLAG_OTHER_NETWORK) {
-    ConnectToNetworkAt(index, std::string(), std::string(), -1);
+    ConnectToNetworkAt(id, std::string(), std::string(), -1);
   } else if (flags & FLAG_CELLULAR) {
-    ConnectToNetworkAt(index, std::string(), std::string(), -1);
+    ConnectToNetworkAt(id, std::string(), std::string(), -1);
   }
 }
 
 void NetworkMenu::SetFirstLevelMenuWidth(int width) {
   min_width_ = width;
   // This actually has no effect since menu is rebuilt before showing.
-  network_menu_->SetMinimumWidth(width);
 }
 
 void NetworkMenu::CancelMenu() {
-  network_menu_->CancelMenu();
+  network_menu_->Cancel();
 }
 
 void NetworkMenu::UpdateMenu() {
   refreshing_menu_ = true;
+
+  // Rebuild list of available networks.
   InitMenuItems();
-  network_menu_->Rebuild();
+
+  // Clear and repopulate MenuItemView.
+  network_menu_->CreateSubmenu()->RemoveAllChildViews(true);
+  for (int i = 0, n = menu_items_.size(); i < n; ++i)
+  {

[oshima, 2011/04/14 17:27:31]

move { to previous ine

[rhashimoto, 2011/04/14 18:27:04]

Done.

+    if (menu_items_[i].type == ui::MenuModel::TYPE_SEPARATOR)
+      network_menu_->AppendSeparator();
+    else
+      network_menu_->AppendMenuItemWithIcon(
+          i,
+          UTF16ToWide(menu_items_[i].label),
+          menu_items_[i].icon);
+  }
+
+  network_menu_->ChildrenChanged();
   refreshing_menu_ = false;
 }
 
@@ -520,22 +530,41 @@ SkBitmap NetworkMenu::IconForDisplay(const SkBitmap* icon,
 // NetworkMenu, views::ViewMenuDelegate implementation:
 
 void NetworkMenu::RunMenu(views::View* source, const gfx::Point& pt) {
-  refreshing_menu_ = true;
+  DCHECK(source == GetMenuButton());

[oshima, 2011/04/14 17:27:31]

DCHECK_EQ

[rhashimoto, 2011/04/14 18:27:04]

Done.

+
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   cros->RequestNetworkScan();
 
-  // Build initial menu items. They will be updated when UpdateMenu is
-  // called from NetworkChanged.
-  InitMenuItems();
-  network_menu_->Rebuild();
+  UpdateMenu();
+
+  // TODO(rhashimoto): Remove this workaround when WebUI provides a
+  // top-level widget on the ChromeOS login screen that is a window.
+  // The current BackgroundView class for the ChromeOS login screen
+  // creates a owning Widget that has a native GtkWindow but is not a
+  // Window.  This makes it impossible to get the NativeWindow via
+  // the views API.  This workaround casts the top-level NativeWidget
+  // to a NativeWindow that we can pass to MenuItemView::RunMenuAt().
+  gfx::NativeWindow window;
+  if (source->GetWindow()) {
+    // This is the normal case with a browser.
+    window = source->GetWindow()->GetNativeWindow();
+  } else {
+#if defined(OS_WIN)
+    NOTREACHED();
+#elif defined(USE_X11)
+    window = GTK_WINDOW(source->GetWidget()->GetNativeView());
+#endif
+  }
 
-  // Restore menu width, if it was set up.
-  // NOTE: width isn't checked for correctness here since all width-related
-  // logic implemented inside |network_menu_|.
-  if (min_width_ != -1)
-    network_menu_->SetMinimumWidth(min_width_);
-  refreshing_menu_ = false;
-  network_menu_->RunMenuAt(pt, views::Menu2::ALIGN_TOPRIGHT);
+  gfx::Point screen_loc;
+  views::View::ConvertPointToScreen(source, &screen_loc);
+  gfx::Rect bounds(screen_loc, source->size());
+  network_menu_->RunMenuAt(
+      window,
+      GetMenuButton(),
+      bounds,
+      base::i18n::IsRTL() ? MenuItemView::TOPLEFT : MenuItemView::TOPRIGHT,
+      true);
 }
 
 void NetworkMenu::InitMenuItems() {