IsCommandIdEnabled should not fall though if the base did handle the command id.

I missed that RVContextMenu::IsCommandIdEnabled has NOTREACHED() and my refactoring was causing DCHECK failure.
I'll refactor a bit more to make each group modular when lazyboy@ is back.

BUG=401926
TEST=covered by RenderViewContextMenuTest.IsCustomCommandIdEnabled

Review URL: https://codereview.chromium.org/453993002

Cr-Commit-Position: refs/heads/master@{#289142}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289142 0039d316-1c4b-4281-b951-d872f2087c98

athena/content/render_view_context_menu_impl.cc

@@ -153,8 +153,11 @@ bool RenderViewContextMenuImpl::IsCommandIdChecked(int command_id) const {
 }
 
 bool RenderViewContextMenuImpl::IsCommandIdEnabled(int command_id) const {
-  if (RenderViewContextMenuBase::IsCommandIdEnabled(command_id))
-    return true;
+  {
+    bool enabled = false;
+    if (RenderViewContextMenuBase::IsCommandIdKnown(command_id, &enabled))
+      return enabled;
+  }
   switch (command_id) {
     // Navigation
     case CMD_BACK:

chrome/browser/renderer_context_menu/render_view_context_menu.cc

@@ -954,8 +954,11 @@ void RenderViewContextMenu::AppendProtocolHandlerSubMenu() {
 // Menu delegate functions -----------------------------------------------------
 
 bool RenderViewContextMenu::IsCommandIdEnabled(int id) const {
-  if (RenderViewContextMenuBase::IsCommandIdEnabled(id))
-    return true;
+  {
+    bool enabled = false;
+    if (RenderViewContextMenuBase::IsCommandIdKnown(id, &enabled))
+      return enabled;
+  }
 
   CoreTabHelper* core_tab_helper =
       CoreTabHelper::FromWebContents(source_web_contents_);

chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc

@@ -289,3 +289,12 @@ TEST_F(RenderViewContextMenuPrefsTest,
   EXPECT_FALSE(
       menu->IsCommandIdEnabled(IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD));
 }
+
+// Make sure the checking custom command id that is not enabled will not
+// cause DCHECK failure.
+TEST_F(RenderViewContextMenuPrefsTest,
+       IsCustomCommandIdEnabled) {
+  scoped_ptr<TestRenderViewContextMenu> menu(CreateContextMenu());
+
+  EXPECT_FALSE(menu->IsCommandIdEnabled(IDC_CONTENT_CONTEXT_CUSTOM_FIRST));
+}

components/renderer_context_menu/render_view_context_menu_base.cc

@@ -250,25 +250,31 @@ bool RenderViewContextMenuBase::AppendCustomItems() {
   return total_items > 0;
 }
 
-// Menu delegate functions -----------------------------------------------------
-
-bool RenderViewContextMenuBase::IsCommandIdEnabled(int id) const {
+bool RenderViewContextMenuBase::IsCommandIdKnown(
+    int id,
+    bool* enabled) const {
   // If this command is is added by one of our observers, we dispatch
   // it to the observer.
   ObserverListBase<RenderViewContextMenuObserver>::Iterator it(observers_);
   RenderViewContextMenuObserver* observer;
   while ((observer = it.GetNext()) != NULL) {
-    if (observer->IsCommandIdSupported(id))
-      return observer->IsCommandIdEnabled(id);
+    if (observer->IsCommandIdSupported(id)) {
+      *enabled = observer->IsCommandIdEnabled(id);
+      return true;
+    }
   }
 
   // Custom items.
-  if (IsContentCustomCommandId(id))
-    return IsCustomItemEnabled(id);
+  if (IsContentCustomCommandId(id)) {
+    *enabled = IsCustomItemEnabled(id);
+    return true;
+  }
 
   return false;
 }
 
+// Menu delegate functions -----------------------------------------------------
+
 bool RenderViewContextMenuBase::IsCommandIdChecked(int id) const {
   // If this command is is added by one of our observers, we dispatch it to the
   // observer.

components/renderer_context_menu/render_view_context_menu_base.h

@@ -78,9 +78,13 @@ class RenderViewContextMenuBase : public ui::SimpleMenuModel::Delegate,
   const ui::SimpleMenuModel& menu_model() const { return menu_model_; }
   const content::ContextMenuParams& params() const { return params_; }
 
+  // Returns true if the specified command id is known and valid for
+  // this menu. If the command is known |enabled| is set to indicate
+  // if the command is enabled.
+  bool IsCommandIdKnown(int command_id, bool* enabled) const;
+
   // SimpleMenuModel::Delegate implementation.
   virtual bool IsCommandIdChecked(int command_id) const OVERRIDE;
-  virtual bool IsCommandIdEnabled(int command_id) const OVERRIDE;
   virtual void ExecuteCommand(int command_id, int event_flags) OVERRIDE;
   virtual void MenuWillShow(ui::SimpleMenuModel* source) OVERRIDE;
   virtual void MenuClosed(ui::SimpleMenuModel* source) OVERRIDE;