Re-organize RenderViewContextMenu to the use cross-platform MenuModel class.

Benefits:
- less code
- more straightforward code
- more shared code

unresolved issues:
- RenderViewContextMenu{Gtk,Mac,Views} should not be a subclass of RenderViewContextMenu
- On Linux, IMEs are added in the wrong place (not near the other editable stuff)

BUG=31365
TEST=manual + trybots

Review URL: http://codereview.chromium.org/2017001

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

2 files changed, 76 insertions, 26 deletions

diff --git a/app/menus/simple_menu_model.cc b/app/menus/simple_menu_model.cc
index f6de9ad..0561c5a 100644
--- a/app/menus/simple_menu_model.cc
+++ b/app/menus/simple_menu_model.cc
@@ -6,6 +6,8 @@
 
 #include "app/l10n_util.h"
 
+static const int kSeparatorId = -1;
+
 namespace menus {
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -19,31 +21,40 @@ SimpleMenuModel::~SimpleMenuModel() {
 
 void SimpleMenuModel::AddItem(int command_id, const string16& label) {
   Item item = { command_id, label, SkBitmap(), TYPE_COMMAND, -1, NULL };
-  items_.push_back(item);
+  AppendItem(item);
 }
 
 void SimpleMenuModel::AddItemWithStringId(int command_id, int string_id) {
   AddItem(command_id, l10n_util::GetStringUTF16(string_id));
 }
 
+void SimpleMenuModel::AddItemIdentifiedByStringId(int string_id) {
+  AddItemWithStringId(string_id, string_id);
+}
+
 void SimpleMenuModel::AddSeparator() {
-  Item item = { -1, string16(), SkBitmap(), TYPE_SEPARATOR, -1, NULL };
-  items_.push_back(item);
+  Item item = { kSeparatorId, string16(), SkBitmap(), TYPE_SEPARATOR, -1,
+                NULL };
+  AppendItem(item);
 }
 
 void SimpleMenuModel::AddCheckItem(int command_id, const string16& label) {
   Item item = { command_id, label, SkBitmap(), TYPE_CHECK, -1, NULL };
-  items_.push_back(item);
+  AppendItem(item);
 }
 
 void SimpleMenuModel::AddCheckItemWithStringId(int command_id, int string_id) {
   AddCheckItem(command_id, l10n_util::GetStringUTF16(string_id));
 }
 
+void SimpleMenuModel::AddCheckItemIdentifiedByStringId(int string_id) {
+  AddCheckItemWithStringId(string_id, string_id);
+}
+
 void SimpleMenuModel::AddRadioItem(int command_id, const string16& label,
                                    int group_id) {
   Item item = { command_id, label, SkBitmap(), TYPE_RADIO, group_id, NULL };
-  items_.push_back(item);
+  AppendItem(item);
 }
 
 void SimpleMenuModel::AddRadioItemWithStringId(int command_id, int string_id,
@@ -51,19 +62,21 @@ void SimpleMenuModel::AddRadioItemWithStringId(int command_id, int string_id,
   AddRadioItem(command_id, l10n_util::GetStringUTF16(string_id), group_id);
 }
 
-void SimpleMenuModel::AddSubMenu(const string16& label, MenuModel* model) {
-  Item item = { -1, label, SkBitmap(), TYPE_SUBMENU, -1, model };
-  items_.push_back(item);
+void SimpleMenuModel::AddSubMenu(int command_id, const string16& label,
+                                 MenuModel* model) {
+  Item item = { command_id, label, SkBitmap(), TYPE_SUBMENU, -1, model };
+  AppendItem(item);
 }
 
-void SimpleMenuModel::AddSubMenuWithStringId(int string_id, MenuModel* model) {
-  AddSubMenu(l10n_util::GetStringUTF16(string_id), model);
+void SimpleMenuModel::AddSubMenuWithStringId(int command_id,
+                                             int string_id, MenuModel* model) {
+  AddSubMenu(command_id, l10n_util::GetStringUTF16(string_id), model);
 }
 
 void SimpleMenuModel::InsertItemAt(
     int index, int command_id, const string16& label) {
   Item item = { command_id, label, SkBitmap(), TYPE_COMMAND, -1, NULL };
-  items_.insert(items_.begin() + FlipIndex(index), item);
+  InsertItemAtIndex(item, index);
 }
 
 void SimpleMenuModel::InsertItemWithStringIdAt(
@@ -72,14 +85,15 @@ void SimpleMenuModel::InsertItemWithStringIdAt(
 }
 
 void SimpleMenuModel::InsertSeparatorAt(int index) {
-  Item item = { -1, string16(), SkBitmap(), TYPE_SEPARATOR, -1, NULL };
-  items_.insert(items_.begin() + FlipIndex(index), item);
+  Item item = { kSeparatorId, string16(), SkBitmap(), TYPE_SEPARATOR, -1,
+                NULL };
+  InsertItemAtIndex(item, index);
 }
 
 void SimpleMenuModel::InsertCheckItemAt(
     int index, int command_id, const string16& label) {
   Item item = { command_id, label, SkBitmap(), TYPE_CHECK, -1, NULL };
-  items_.insert(items_.begin() + FlipIndex(index), item);
+  InsertItemAtIndex(item, index);
 }
 
 void SimpleMenuModel::InsertCheckItemWithStringIdAt(
@@ -91,7 +105,7 @@ void SimpleMenuModel::InsertCheckItemWithStringIdAt(
 void SimpleMenuModel::InsertRadioItemAt(
     int index, int command_id, const string16& label, int group_id) {
   Item item = { command_id, label, SkBitmap(), TYPE_RADIO, group_id, NULL };
-  items_.insert(items_.begin() + FlipIndex(index), item);
+  InsertItemAtIndex(item, index);
 }
 
 void SimpleMenuModel::InsertRadioItemWithStringIdAt(
@@ -101,14 +115,15 @@ void SimpleMenuModel::InsertRadioItemWithStringIdAt(
 }
 
 void SimpleMenuModel::InsertSubMenuAt(
-    int index, const string16& label, MenuModel* model) {
-  Item item = { -1, label, SkBitmap(), TYPE_SUBMENU, -1, model };
-  items_.insert(items_.begin() + FlipIndex(index), item);
+    int index, int command_id, const string16& label, MenuModel* model) {
+  Item item = { command_id, label, SkBitmap(), TYPE_SUBMENU, -1, model };
+  InsertItemAtIndex(item, index);
 }
 
 void SimpleMenuModel::InsertSubMenuWithStringIdAt(
-    int index, int string_id, MenuModel* model) {
-  InsertSubMenuAt(index, l10n_util::GetStringUTF16(string_id), model);
+    int index, int command_id, int string_id, MenuModel* model) {
+  InsertSubMenuAt(index, command_id, l10n_util::GetStringUTF16(string_id),
+                  model);
 }
 
 void SimpleMenuModel::SetIcon(int index, const SkBitmap& icon) {
@@ -194,8 +209,7 @@ bool SimpleMenuModel::GetIconAt(int index, SkBitmap* icon) const {
 
 bool SimpleMenuModel::IsEnabledAt(int index) const {
   int command_id = GetCommandIdAt(index);
-  // Submenus have a command id of -1, they should always be enabled.
-  if (!delegate_ || command_id == -1)
+  if (!delegate_ || command_id == kSeparatorId)
     return true;
   return delegate_->IsCommandIdEnabled(command_id);
 }
@@ -214,4 +228,27 @@ MenuModel* SimpleMenuModel::GetSubmenuModelAt(int index) const {
   return items_.at(FlipIndex(index)).submenu;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// SimpleMenuModel, Private:
+
+void SimpleMenuModel::AppendItem(const Item& item) {
+  ValidateItem(item);
+  items_.push_back(item);
+}
+
+void SimpleMenuModel::InsertItemAtIndex(const Item& item, int index) {
+  ValidateItem(item);
+  items_.insert(items_.begin() + FlipIndex(index), item);
+}
+
+void SimpleMenuModel::ValidateItem(const Item& item) {
+#ifndef NDEBUG
+  if (item.type == TYPE_SEPARATOR) {
+    DCHECK_EQ(item.command_id, kSeparatorId);
+  } else {
+    DCHECK_GE(item.command_id, 0);
+  }
+#endif  // NDEBUG
+}
+
 }  // namespace menus
diff --git a/app/menus/simple_menu_model.h b/app/menus/simple_menu_model.h
index 5307f12..f183166b 100644
--- a/app/menus/simple_menu_model.h
+++ b/app/menus/simple_menu_model.h
@@ -60,8 +60,14 @@ class SimpleMenuModel : public MenuModel {
   void AddCheckItemWithStringId(int command_id, int string_id);
   void AddRadioItem(int command_id, const string16& label, int group_id);
   void AddRadioItemWithStringId(int command_id, int string_id, int group_id);
-  void AddSubMenu(const string16& label, MenuModel* model);
-  void AddSubMenuWithStringId(int string_id, MenuModel* model);
+  void AddSubMenu(int command_id, const string16& label, MenuModel* model);
+  void AddSubMenuWithStringId(int command_id, int string_id, MenuModel* model);
+
+  // Helpers for the above that use the |string_id| for the |command_id|
+  // as well.
+  // TODO(estade): remove these.
+  void AddItemIdentifiedByStringId(int string_id);
+  void AddCheckItemIdentifiedByStringId(int string_id);
 
   // Methods for inserting items into the model.
   void InsertItemAt(int index, int command_id, const string16& label);
@@ -73,8 +79,10 @@ class SimpleMenuModel : public MenuModel {
       int index, int command_id, const string16& label, int group_id);
   void InsertRadioItemWithStringIdAt(
       int index, int command_id, int string_id, int group_id);
-  void InsertSubMenuAt(int index, const string16& label, MenuModel* model);
-  void InsertSubMenuWithStringIdAt(int index, int string_id, MenuModel* model);
+  void InsertSubMenuAt(
+      int index, int command_id, const string16& label, MenuModel* model);
+  void InsertSubMenuWithStringIdAt(
+      int index, int command_id, int string_id, MenuModel* model);
 
   // Sets the icon for the item at |index|.
   void SetIcon(int index, const SkBitmap& icon);
@@ -126,6 +134,11 @@ class SimpleMenuModel : public MenuModel {
   };
   std::vector<Item> items_;
 
+  // Functions for inserting items into |items_|.
+  void AppendItem(const Item& item);
+  void InsertItemAtIndex(const Item& item, int index);
+  void ValidateItem(const Item& item);
+
   Delegate* delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(SimpleMenuModel);