Change status button menu implementation from Menu2 to MenuItemView.

chrome/browser/chromeos/status/network_menu.cc

 // Copyright (c) 2011 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/chromeos/status/network_menu.h"
 
 #include <algorithm>
 
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/sim_unlock_dialog_delegate.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/views/window.h"
 #include "chrome/common/url_constants.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "net/base/escape.h"
 #include "ui/base/l10n/l10n_util.h"
 #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";
 static const char* kNetworkTypeWifi = "wifi";
 static const char* kNetworkTypeCellular = "cellular";
 static const char* kNetworkTypeOther = "other";
 
 static const char* kNetworkStatusConnected = "connected";
 static const char* kNetworkStatusConnecting = "connecting";
 static const char* kNetworkStatusDisconnected = "disconnected";
@@ skipping 45 matching lines @@
 const int NetworkMenu::kNumAnimatingImages = 10;
 
 // static
 SkBitmap NetworkMenu::kAnimatingImages[kNumAnimatingImages];
 
 // static
 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() {
 }
 
 bool NetworkMenu::GetNetworkAt(int index, NetworkInfo* info) const {
   DCHECK(info);
   bool res = true;  // True unless a network doesn't exist.
   int flags = menu_items_[index].flags;
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
@@ skipping 145 matching lines @@
       // display a notification.
       // TODO(stevenjb): Show notification.
     }
   } else if (flags & FLAG_OTHER_NETWORK) {
     ShowOther();
   }
   return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// NetworkMenu, ui::MenuModel implementation:
-
-int NetworkMenu::GetItemCount() const {
-  return static_cast<int>(menu_items_.size());
+// 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);
 }
 
-ui::MenuModel::ItemType NetworkMenu::GetTypeAt(int index) const {
-  return menu_items_[index].type;
+const gfx::Font* NetworkMenu::GetLabelFont(int id) const
+{
+  return (menu_items_[id].flags & FLAG_ASSOCIATED) ?
+      &ResourceBundle::GetSharedInstance().GetFont(ResourceBundle::BoldFont) :
+      &ResourceBundle::GetSharedInstance().GetFont(ResourceBundle::BaseFont);
 }
 
-string16 NetworkMenu::GetLabelAt(int index) const {
-  return menu_items_[index].label;
+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.

 }
 
-const gfx::Font* NetworkMenu::GetLabelFontAt(int index) const {
-  return (menu_items_[index].flags & FLAG_ASSOCIATED) ?
-      &ResourceBundle::GetSharedInstance().GetFont(ResourceBundle::BoldFont) :
-      NULL;
+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::IsItemCheckedAt(int index) const {
-  // All ui::MenuModel::TYPE_CHECK menu items are checked.
-  return true;
-}
+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.

 
-bool NetworkMenu::GetIconAt(int index, SkBitmap* icon) {
-  if (!menu_items_[index].icon.empty()) {
-    *icon = menu_items_[index].icon;
-    return true;
-  }
-  return false;
-}
-
-bool NetworkMenu::IsEnabledAt(int index) const {
-  return !(menu_items_[index].flags & FLAG_DISABLED);
-}
-
-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) {
     cros->EnableEthernetNetworkDevice(!cros->ethernet_enabled());
   } else if (flags & FLAG_TOGGLE_WIFI) {
     cros->EnableWifiNetworkDevice(!cros->wifi_enabled());
   } else if (flags & FLAG_TOGGLE_CELLULAR) {
     cros->EnableCellularNetworkDevice(!cros->cellular_enabled());
     // TODO(nkostylev): Integrate with SIM locked state. crosbug.com/12007.
     // SimUnlockDialogDelegate::ShowDialog(GetNativeWindow());
   } else if (flags & FLAG_TOGGLE_OFFLINE) {
     cros->EnableOfflineMode(!cros->offline_mode());
   } else if (flags & FLAG_ETHERNET) {
     if (cros->ethernet_connected()) {
       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;
 }
 
 // static
 const SkBitmap* NetworkMenu::IconForNetworkStrength(const WifiNetwork* wifi,
                                                     bool black) {
   DCHECK(wifi);
   if (wifi->strength() == 0) {
     return ResourceBundle::GetSharedInstance().GetBitmapNamed(
         black ? IDR_STATUSBAR_NETWORK_BARS0_BLACK :
@@ skipping 162 matching lines @@
                          kBottomRightBadgeY);
   if (top_left_badge != NULL)
     canvas.DrawBitmapInt(*top_left_badge, kTopLeftBadgeX, kTopLeftBadgeY);
   return canvas.ExtractBitmap();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // 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();
 
-  // 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);
+  // 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
+  }
+
+  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() {
   // This gets called on initialization, so any changes should be reflected
   // in CrosMock::SetNetworkLibraryStatusAreaExpectations().
 
   menu_items_.clear();
   // Populate our MenuItems with the current list of wifi networks.
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
@@ skipping 265 matching lines @@
   if (!browser)
     return;
   browser->OpenMobilePlanTabAndActivate();
 }
 
 void NetworkMenu::ShowOther() const {
   ShowNetworkConfigView(new NetworkConfigView());
 }
 
 }  // namespace chromeos