Upgrade BackForwardMenuModelViews to use new menu API. Also adds accelerator to the "Show Full History" item.

This requires bringing the owner-draw system for native menus over from the old code. I haven't really changed anything in it other than the format of dwItemData. This code could be improved/simplified by using gfx::Canvas more, but don't want to do it here.

BUG=none
TEST=make sure BackForwardMenuModel tests still pass, test the menu functionality in the toolbar.
Review URL: http://codereview.chromium.org/126092

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

7 files changed, 151 insertions, 98 deletions

diff --git a/chrome/browser/back_forward_menu_model.cc b/chrome/browser/back_forward_menu_model.cc
index e623ada..984d06b 100644
--- a/chrome/browser/back_forward_menu_model.cc
+++ b/chrome/browser/back_forward_menu_model.cc
@@ -7,6 +7,7 @@
 #include "chrome/browser/back_forward_menu_model.h"
 
 #include "app/l10n_util.h"
+#include "app/resource_bundle.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/metrics/user_metrics.h"
 #include "chrome/browser/tab_contents/navigation_controller.h"
@@ -14,16 +15,24 @@
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/url_constants.h"
 #include "grit/generated_resources.h"
+#include "grit/theme_resources.h"
 #include "net/base/registry_controlled_domain.h"
 
 const int BackForwardMenuModel::kMaxHistoryItems = 12;
 const int BackForwardMenuModel::kMaxChapterStops = 5;
 
+BackForwardMenuModel::BackForwardMenuModel(Browser* browser,
+                                           ModelType model_type)
+    : browser_(browser),
+      test_tab_contents_(NULL),
+      model_type_(model_type) {
+}
+
 int BackForwardMenuModel::GetHistoryItemCount() const {
   TabContents* contents = GetTabContents();
   int items = 0;
 
-  if (model_type_ == FORWARD_MENU_DELEGATE) {
+  if (model_type_ == FORWARD_MENU) {
     // Only count items from n+1 to end (if n is current entry)
     items = contents->controller().entry_count() -
             contents->controller().GetCurrentEntryIndex() - 1;
@@ -47,7 +56,7 @@ int BackForwardMenuModel::GetChapterStopCount(int history_items) const {
 
   if (history_items == kMaxHistoryItems) {
     int chapter_id = current_entry;
-    if (model_type_ == FORWARD_MENU_DELEGATE) {
+    if (model_type_ == FORWARD_MENU) {
       chapter_id += history_items;
     } else {
       chapter_id -= history_items;
@@ -55,7 +64,7 @@ int BackForwardMenuModel::GetChapterStopCount(int history_items) const {
 
     do {
       chapter_id = GetIndexOfNextChapterStop(chapter_id,
-          model_type_ == FORWARD_MENU_DELEGATE);
+          model_type_ == FORWARD_MENU);
       if (chapter_id != -1)
         ++chapter_stops;
     } while (chapter_id != -1 && chapter_stops < kMaxChapterStops);
@@ -213,14 +222,17 @@ std::wstring BackForwardMenuModel::GetItemLabel(int menu_id) const {
 const SkBitmap& BackForwardMenuModel::GetItemIcon(int menu_id) const {
   DCHECK(ItemHasIcon(menu_id));
 
+  if (menu_id == GetTotalItemCount()) {
+    return *ResourceBundle::GetSharedInstance().GetBitmapNamed(
+        IDR_HISTORY_FAVICON);
+  }
+
   NavigationEntry* entry = GetNavigationEntry(menu_id);
   return entry->favicon().bitmap();
 }
 
 bool BackForwardMenuModel::ItemHasIcon(int menu_id) const {
-  // Using "id" not "id - 1" because the last item "Show Full History"
-  // doesn't have an icon.
-  return menu_id < GetTotalItemCount() && !IsSeparator(menu_id);
+  return menu_id - 1 < GetTotalItemCount() && !IsSeparator(menu_id);
 }
 
 bool BackForwardMenuModel::ItemHasCommand(int menu_id) const {
@@ -245,7 +257,7 @@ int BackForwardMenuModel::MenuIdToNavEntryIndex(int menu_id) const {
 
   // Convert anything above the History items separator.
   if (menu_id <= history_items) {
-    if (model_type_ == FORWARD_MENU_DELEGATE) {
+    if (model_type_ == FORWARD_MENU) {
       // The |menu_id| is relative to our current position, so we need to add.
       menu_id += contents->controller().GetCurrentEntryIndex();
     } else {
@@ -262,7 +274,7 @@ int BackForwardMenuModel::MenuIdToNavEntryIndex(int menu_id) const {
 
   // This menu item is a chapter stop located between the two separators.
   menu_id = FindChapterStop(history_items,
-                            model_type_ == FORWARD_MENU_DELEGATE,
+                            model_type_ == FORWARD_MENU,
                             menu_id - history_items - 1 - 1);
 
   return menu_id;
@@ -278,7 +290,7 @@ std::wstring BackForwardMenuModel::BuildActionName(
   DCHECK(!action.empty());
   DCHECK(index >= -1);
   std::wstring metric_string;
-  if (model_type_ == FORWARD_MENU_DELEGATE)
+  if (model_type_ == FORWARD_MENU)
     metric_string += L"ForwardMenu_";
   else
     metric_string += L"BackMenu_";
diff --git a/chrome/browser/back_forward_menu_model.h b/chrome/browser/back_forward_menu_model.h
index 7d0f31c..9f85a7e 100644
--- a/chrome/browser/back_forward_menu_model.h
+++ b/chrome/browser/back_forward_menu_model.h
@@ -26,15 +26,11 @@ class BackForwardMenuModel {
   // These are IDs used to identify individual UI elements within the
   // browser window using View::GetViewByID.
   enum ModelType {
-    FORWARD_MENU_DELEGATE = 1,
-    BACKWARD_MENU_DELEGATE = 2
+    FORWARD_MENU = 1,
+    BACKWARD_MENU = 2
   };
 
-  // Factory function. Defined in back_forward_menu_model_{platform}.cc.
-  // This is only used in unit tests. In the browser we use the platform-
-  // specific constructors directly.
-  static BackForwardMenuModel* Create(Browser* browser, ModelType model_type);
-
+  BackForwardMenuModel(Browser* browser, ModelType model_type);
   virtual ~BackForwardMenuModel() { }
 
   // Returns how many history items the menu should show. For example, if the
@@ -108,10 +104,12 @@ class BackForwardMenuModel {
   // Does the item does something when you click on it?
   bool ItemHasCommand(int menu_id) const;
 
+#ifdef UNIT_TEST
   // Allows the unit test to use its own dummy tab contents.
   void set_test_tab_contents(TabContents* test_tab_contents) {
     test_tab_contents_ = test_tab_contents;
   }
+#endif
 
   // Allow the unit test to use the "Show Full History" label.
   std::wstring GetShowFullHistoryLabel() const;
@@ -128,20 +126,6 @@ class BackForwardMenuModel {
   static const int kMaxChapterStops;
 
  protected:
-  BackForwardMenuModel()
-      : browser_(NULL),
-        test_tab_contents_(NULL),
-        model_type_(FORWARD_MENU_DELEGATE) {}
-
-  Browser* browser_;
-
-  // The unit tests will provide their own TabContents to use.
-  TabContents* test_tab_contents_;
-
-  // Represents whether this is the delegate for the forward button or the
-  // back button.
-  ModelType model_type_;
-
   // Converts a menu item id, as passed in through one of the menu delegate
   // functions and converts it into an absolute index into the
   // NavigationEntryList vector. |menu_id| can point to a separator, or the
@@ -157,6 +141,15 @@ class BackForwardMenuModel {
   // An index of -1 means no index.
   std::wstring BuildActionName(const std::wstring& name, int index) const;
 
+  Browser* browser_;
+
+  // The unit tests will provide their own TabContents to use.
+  TabContents* test_tab_contents_;
+
+  // Represents whether this is the delegate for the forward button or the
+  // back button.
+  ModelType model_type_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(BackForwardMenuModel);
 };
diff --git a/chrome/browser/back_forward_menu_model_unittest.cc b/chrome/browser/back_forward_menu_model_unittest.cc
index 708847d..241a95c 100644
--- a/chrome/browser/back_forward_menu_model_unittest.cc
+++ b/chrome/browser/back_forward_menu_model_unittest.cc
@@ -65,12 +65,12 @@ class BackFwdMenuModelTest : public RenderViewHostTestHarness {
 };
 
 TEST_F(BackFwdMenuModelTest, BasicCase) {
-  scoped_ptr<BackForwardMenuModel> back_model(BackForwardMenuModel::Create(
-      NULL, BackForwardMenuModel::BACKWARD_MENU_DELEGATE));
+  scoped_ptr<BackForwardMenuModel> back_model(new BackForwardMenuModel(
+      NULL, BackForwardMenuModel::BACKWARD_MENU));
   back_model->set_test_tab_contents(contents());
 
-  scoped_ptr<BackForwardMenuModel> forward_model(BackForwardMenuModel::Create(
-      NULL, BackForwardMenuModel::FORWARD_MENU_DELEGATE));
+  scoped_ptr<BackForwardMenuModel> forward_model(new BackForwardMenuModel(
+      NULL, BackForwardMenuModel::FORWARD_MENU));
   forward_model->set_test_tab_contents(contents());
 
   EXPECT_EQ(0, back_model->GetTotalItemCount());
@@ -132,12 +132,12 @@ TEST_F(BackFwdMenuModelTest, BasicCase) {
 }
 
 TEST_F(BackFwdMenuModelTest, MaxItemsTest) {
-  scoped_ptr<BackForwardMenuModel> back_model(BackForwardMenuModel::Create(
-      NULL, BackForwardMenuModel::BACKWARD_MENU_DELEGATE));
+  scoped_ptr<BackForwardMenuModel> back_model(new BackForwardMenuModel(
+      NULL, BackForwardMenuModel::BACKWARD_MENU));
   back_model->set_test_tab_contents(contents());
 
-  scoped_ptr<BackForwardMenuModel> forward_model(BackForwardMenuModel::Create(
-      NULL, BackForwardMenuModel::FORWARD_MENU_DELEGATE));
+  scoped_ptr<BackForwardMenuModel> forward_model(new BackForwardMenuModel(
+      NULL, BackForwardMenuModel::FORWARD_MENU));
   forward_model->set_test_tab_contents(contents());
 
   // Seed the controller with 32 URLs
@@ -214,12 +214,12 @@ TEST_F(BackFwdMenuModelTest, MaxItemsTest) {
 }
 
 TEST_F(BackFwdMenuModelTest, ChapterStops) {
-  scoped_ptr<BackForwardMenuModel> back_model(BackForwardMenuModel::Create(
-    NULL, BackForwardMenuModel::BACKWARD_MENU_DELEGATE));
+  scoped_ptr<BackForwardMenuModel> back_model(new BackForwardMenuModel(
+    NULL, BackForwardMenuModel::BACKWARD_MENU));
   back_model->set_test_tab_contents(contents());
 
-  scoped_ptr<BackForwardMenuModel> forward_model(BackForwardMenuModel::Create(
-      NULL, BackForwardMenuModel::FORWARD_MENU_DELEGATE));
+  scoped_ptr<BackForwardMenuModel> forward_model(new BackForwardMenuModel(
+      NULL, BackForwardMenuModel::FORWARD_MENU));
   forward_model->set_test_tab_contents(contents());
 
   // Seed the controller with 32 URLs.
diff --git a/chrome/browser/back_forward_menu_model_views.cc b/chrome/browser/back_forward_menu_model_views.cc
index f509274..fd21241 100644
--- a/chrome/browser/back_forward_menu_model_views.cc
+++ b/chrome/browser/back_forward_menu_model_views.cc
@@ -4,59 +4,95 @@
 
 #include "chrome/browser/back_forward_menu_model_views.h"
 
+#include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/metrics/user_metrics.h"
-#include "grit/generated_resources.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "views/controls/menu/menu_2.h"
+#include "views/widget/widget.h"
 
-// static
-BackForwardMenuModel* BackForwardMenuModel::Create(Browser* browser,
-                                                   ModelType model_type) {
-  return new BackForwardMenuModelViews(browser, model_type);
-}
+////////////////////////////////////////////////////////////////////////////////
+// BackForwardMenuModelViews, public:
 
 BackForwardMenuModelViews::BackForwardMenuModelViews(Browser* browser,
-                                                     ModelType model_type) {
-  browser_ = browser;
-  model_type_ = model_type;
+                                                     ModelType model_type,
+                                                     views::Widget* frame)
+    : BackForwardMenuModel(browser, model_type),
+      frame_(frame) {
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// BackForwardMenuModelViews, views::Menu2Model implementation:
+
+bool BackForwardMenuModelViews::HasIcons() const {
+  return true;
 }
 
-std::wstring BackForwardMenuModelViews::GetLabel(int menu_id) const {
-  return GetItemLabel(menu_id);
+int BackForwardMenuModelViews::GetItemCount() const {
+  return GetTotalItemCount();
 }
 
-const SkBitmap& BackForwardMenuModelViews::GetIcon(int menu_id) const {
-  // Return NULL if the item doesn't have an icon
-  if (!ItemHasIcon(menu_id))
-    return GetEmptyIcon();
+views::Menu2Model::ItemType BackForwardMenuModelViews::GetTypeAt(
+    int index) const {
+  if (IsSeparator(GetCommandIdAt(index)))
+    return views::Menu2Model::TYPE_SEPARATOR;
+  return views::Menu2Model::TYPE_COMMAND;
+}
+
+int BackForwardMenuModelViews::GetCommandIdAt(int index) const {
+  return index + 1;
+}
+
+std::wstring BackForwardMenuModelViews::GetLabelAt(int index) const {
+  return GetItemLabel(GetCommandIdAt(index));
+}
 
-  return GetItemIcon(menu_id);
+bool BackForwardMenuModelViews::IsLabelDynamicAt(int index) const {
+  return false;
 }
 
-bool BackForwardMenuModelViews::IsItemSeparator(int menu_id) const {
-  return IsSeparator(menu_id);
+bool BackForwardMenuModelViews::GetAcceleratorAt(
+    int index,
+    views::Accelerator* accelerator) const {
+  // Look up the history accelerator.
+  if (GetCommandIdAt(index) == GetTotalItemCount())
+    return frame_->GetAccelerator(IDC_SHOW_HISTORY, accelerator);
+  return false;
 }
 
-bool BackForwardMenuModelViews::HasIcon(int menu_id) const {
-  return ItemHasIcon(menu_id);
+bool BackForwardMenuModelViews::IsItemCheckedAt(int index) const {
+  return false;
 }
 
-bool BackForwardMenuModelViews::SupportsCommand(int menu_id) const {
-  return ItemHasCommand(menu_id);
+int BackForwardMenuModelViews::GetGroupIdAt(int index) const {
+  return -1;
 }
 
-bool BackForwardMenuModelViews::IsCommandEnabled(int menu_id) const {
-  return ItemHasCommand(menu_id);
+bool BackForwardMenuModelViews::GetIconAt(int index, SkBitmap* icon) const {
+  if (ItemHasIcon(GetCommandIdAt(index))) {
+    *icon = GetItemIcon(GetCommandIdAt(index));
+    return true;
+  }
+  return false;
 }
 
-void BackForwardMenuModelViews::ExecuteCommand(int menu_id) {
-  ExecuteCommandById(menu_id);
+bool BackForwardMenuModelViews::IsEnabledAt(int index) const {
+  return true;
+}
+
+views::Menu2Model* BackForwardMenuModelViews::GetSubmenuModelAt(
+    int index) const {
+  return NULL;
+}
+
+void BackForwardMenuModelViews::HighlightChangedTo(int index) {
+}
+
+void BackForwardMenuModelViews::ActivatedAt(int index) {
+  ExecuteCommandById(GetCommandIdAt(index));
 }
 
 void BackForwardMenuModelViews::MenuWillShow() {
   UserMetrics::RecordComputedAction(BuildActionName(L"Popup", -1),
                                     browser_->profile());
 }
-
-int BackForwardMenuModelViews::GetItemCount() const {
-  return GetTotalItemCount();
-}
diff --git a/chrome/browser/back_forward_menu_model_views.h b/chrome/browser/back_forward_menu_model_views.h
index 19a36ace..e97a39a 100644
--- a/chrome/browser/back_forward_menu_model_views.h
+++ b/chrome/browser/back_forward_menu_model_views.h
@@ -8,27 +8,45 @@
 #include "base/basictypes.h"
 
 #include "chrome/browser/back_forward_menu_model.h"
-#include "views/controls/menu/menu.h"
+#include "views/controls/menu/menu_2.h"
 
 class SkBitmap;
 
+namespace views {
+class Widget;
+}
+
 class BackForwardMenuModelViews : public BackForwardMenuModel,
-                                  public views::Menu::Delegate {
+                                  public views::Menu2Model {
  public:
-  BackForwardMenuModelViews(Browser* browser, ModelType model_type);
-
-  // Menu::Delegate
-  virtual std::wstring GetLabel(int menu_id) const;
-  virtual const SkBitmap& GetIcon(int menu_id) const;
-  virtual bool SupportsCommand(int menu_id) const;
-  virtual bool IsCommandEnabled(int menu_id) const;
-  virtual bool IsItemSeparator(int menu_id) const;
-  virtual bool HasIcon(int menu_id) const;
-  virtual void ExecuteCommand(int menu_id);
-  virtual void MenuWillShow();
+  // Construct a BackForwardMenuModel. |frame| is used to locate the accelerator
+  // for the history item.
+  BackForwardMenuModelViews(Browser* browser,
+                            ModelType model_type,
+                            views::Widget* frame);
+
+  // Overridden from views::Menu2Model:
+  virtual bool HasIcons() const;
   virtual int GetItemCount() const;
+  virtual ItemType GetTypeAt(int index) const;
+  virtual int GetCommandIdAt(int index) const;
+  virtual std::wstring GetLabelAt(int index) const;
+  virtual bool IsLabelDynamicAt(int index) const;
+  virtual bool GetAcceleratorAt(int index,
+                                views::Accelerator* accelerator) const;
+  virtual bool IsItemCheckedAt(int index) const;
+  virtual int GetGroupIdAt(int index) const;
+  virtual bool GetIconAt(int index, SkBitmap* icon) const;
+  virtual bool IsEnabledAt(int index) const;
+  virtual Menu2Model* GetSubmenuModelAt(int index) const;
+  virtual void HighlightChangedTo(int index);
+  virtual void ActivatedAt(int index);
+  virtual void MenuWillShow();
 
  private:
+  // The frame we ask about accelerator info.
+  views::Widget* frame_;
+
   DISALLOW_COPY_AND_ASSIGN(BackForwardMenuModelViews);
 };
 
diff --git a/chrome/browser/gtk/back_forward_menu_model_gtk.cc b/chrome/browser/gtk/back_forward_menu_model_gtk.cc
index 4e363d0..8bce806 100644
--- a/chrome/browser/gtk/back_forward_menu_model_gtk.cc
+++ b/chrome/browser/gtk/back_forward_menu_model_gtk.cc
@@ -7,18 +7,11 @@
 #include "base/string_util.h"
 #include "chrome/browser/gtk/back_forward_button_gtk.h"
 
-// static
-BackForwardMenuModel* BackForwardMenuModel::Create(Browser* browser,
-                                                   ModelType model_type) {
-  return new BackForwardMenuModelGtk(browser, model_type, NULL);
-}
-
 BackForwardMenuModelGtk::BackForwardMenuModelGtk(Browser* browser,
                                                  ModelType model_type,
                                                  BackForwardButtonGtk* button)
-    : button_(button) {
-  browser_ = browser;
-  model_type_ = model_type;
+    : BackForwardMenuModel(browser, model_type),
+      button_(button) {
 }
 
 int BackForwardMenuModelGtk::GetItemCount() const {
diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc
index 02f31fc..8a254e7 100644
--- a/chrome/browser/views/toolbar_view.cc
+++ b/chrome/browser/views/toolbar_view.cc
@@ -179,10 +179,6 @@ ToolbarView::ToolbarView(Browser* browser)
   browser_->command_updater()->AddCommandObserver(IDC_RELOAD, this);
   browser_->command_updater()->AddCommandObserver(IDC_HOME, this);
   browser_->command_updater()->AddCommandObserver(IDC_STAR, this);
-  back_menu_model_.reset(new BackForwardMenuModelViews(
-      browser, BackForwardMenuModel::BACKWARD_MENU_DELEGATE));
-  forward_menu_model_.reset(new BackForwardMenuModelViews(
-      browser, BackForwardMenuModel::FORWARD_MENU_DELEGATE));
   if (browser->type() == Browser::TYPE_NORMAL)
     display_mode_ = DISPLAYMODE_NORMAL;
   else
@@ -199,6 +195,11 @@ ToolbarView::~ToolbarView() {
 }
 
 void ToolbarView::Init(Profile* profile) {
+  back_menu_model_.reset(new BackForwardMenuModelViews(
+      browser_, BackForwardMenuModel::BACKWARD_MENU, GetWidget()));
+  forward_menu_model_.reset(new BackForwardMenuModelViews(
+      browser_, BackForwardMenuModel::FORWARD_MENU, GetWidget()));
+
   // Create all the individual Views in the Toolbar.
   CreateLeftSideControls();
   CreateCenterStack(profile);