Hide default context menu for platform apps.

Platform apps can provide their own context menu by overriding
the contextmenu event. If they don't override this event, the
default Chrome menu should not be shown.

BUG=106687
TEST=Manually tested, and new browser test cases added.

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

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

chrome/browser/extensions/platform_app_browsertest.cc

@@ -5,15 +5,39 @@
 #include "base/command_line.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/tab_contents/render_view_context_menu.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/models/menu_model.h"
+#include "webkit/glue/context_menu.h"
 
-class PlaformAppBrowserTest : public ExtensionBrowserTest {
+namespace {
+// Non-abstract RenderViewContextMenu class.
+class PlatformAppContextMenu : public RenderViewContextMenu {
+ public:
+  PlatformAppContextMenu(TabContents* tab_contents,
+                            const ContextMenuParams& params)
+      : RenderViewContextMenu(tab_contents, params) {}
+
+ protected:
+  // These two functions implement pure virtual methods of
+  // RenderViewContextMenu.
+  virtual bool GetAcceleratorForCommandId(int command_id,
+                                          ui::Accelerator* accelerator) {
+    return false;
+  }
+  virtual void PlatformInit() {}
+};
+
+}  // namespace
+
+class PlatformAppBrowserTest : public ExtensionBrowserTest {
  public:
   virtual void SetUpCommandLine(CommandLine* command_line) {
     ExtensionBrowserTest::SetUpCommandLine(command_line);
@@ -21,7 +45,8 @@ class PlaformAppBrowserTest : public ExtensionBrowserTest {
   }
 
   void LoadAndLaunchPlatformApp(const char* name) {
-    EXPECT_TRUE(LoadExtension(test_data_dir_.AppendASCII(name)));
+    EXPECT_TRUE(LoadExtension(test_data_dir_.AppendASCII("platform_apps").
+        AppendASCII(name)));
 
     ExtensionService* service = browser()->profile()->GetExtensionService();
     const Extension* extension = service->GetExtensionById(
@@ -42,11 +67,11 @@ class PlaformAppBrowserTest : public ExtensionBrowserTest {
   }
 };
 
-IN_PROC_BROWSER_TEST_F(PlaformAppBrowserTest, OpenAppInShellContainer) {
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, OpenAppInShellContainer) {
   // Start with one browser, new platform app will create another.
   ASSERT_EQ(1u, BrowserList::size());
 
-  LoadAndLaunchPlatformApp("platform_app");
+  LoadAndLaunchPlatformApp("empty");
 
   // The launch should have created a new browser, so there should be 2 now.
   ASSERT_EQ(2u, BrowserList::size());
@@ -62,3 +87,61 @@ IN_PROC_BROWSER_TEST_F(PlaformAppBrowserTest, OpenAppInShellContainer) {
   EXPECT_TRUE(new_browser->is_app());
   EXPECT_TRUE(new_browser->is_type_shell());
 }
+
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, EmptyContextMenu) {
+  // Start with one browser, new platform app will create another.
+  ASSERT_EQ(1u, BrowserList::size());
+
+  LoadAndLaunchPlatformApp("empty");
+
+  // The launch should have created a new browser, so there should be 2 now.
+  ASSERT_EQ(2u, BrowserList::size());
+
+  // The new browser is the last one.
+  BrowserList::const_reverse_iterator reverse_iterator(BrowserList::end());
+  Browser* new_browser = *(reverse_iterator++);
+
+  ASSERT_TRUE(new_browser);
+  ASSERT_TRUE(new_browser != browser());
+
+  // The empty app doesn't add any context menu items, so its menu should
+  // be empty.
+  TabContents* tab_contents = new_browser->GetSelectedTabContents();
+  WebKit::WebContextMenuData data;
+  ContextMenuParams params(data);
+  PlatformAppContextMenu* menu = new PlatformAppContextMenu(tab_contents,
+      params);
+  menu->Init();
+  ASSERT_FALSE(menu->menu_model().GetItemCount());
+}
+
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, AppWithContextMenu) {
+  // Start with one browser, new platform app will create another.
+  ASSERT_EQ(1u, BrowserList::size());
+
+  ExtensionTestMessageListener listener1("created item", false);
+  LoadAndLaunchPlatformApp("context_menu");
+
+  // Wait for the extension to tell us it's created an item.
+  ASSERT_TRUE(listener1.WaitUntilSatisfied());
+
+  // The launch should have created a new browser, so there should be 2 now.
+  ASSERT_EQ(2u, BrowserList::size());
+
+  // The new browser is the last one.
+  BrowserList::const_reverse_iterator reverse_iterator(BrowserList::end());
+  Browser* new_browser = *(reverse_iterator++);
+
+  ASSERT_TRUE(new_browser);
+  ASSERT_TRUE(new_browser != browser());
+
+  // The context_menu app has one context menu item. This is all that should
+  // be in the menu, there should be no seperator.
+  TabContents* tab_contents = new_browser->GetSelectedTabContents();
+  WebKit::WebContextMenuData data;
+  ContextMenuParams params(data);
+  PlatformAppContextMenu* menu = new PlatformAppContextMenu(tab_contents,
+      params);
+  menu->Init();
+  ASSERT_EQ(1, menu->menu_model().GetItemCount());
+}

chrome/browser/tab_contents/render_view_context_menu.cc

@@ -47,8 +47,10 @@
 #include "chrome/browser/translate/translate_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/prefs/prefs_tab_helper.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/search_engines/search_engine_tab_helper.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
+#include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
@@ -360,8 +362,9 @@ void RenderViewContextMenu::AppendExtensionItems(
   if (items.empty())
     return;
 
-  // If this is the first extension-provided menu item, add a separator.
-  if (*index == 0)
+  // If this is the first extension-provided menu item, and there are other
+  // items in the menu, add a separator.
+  if (*index == 0 && menu_model_.GetItemCount())
     menu_model_.AddSeparator();
 
   int menu_id = IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST + (*index)++;
@@ -511,6 +514,20 @@ void RenderViewContextMenu::InitMenu() {
   bool has_link = !params_.unfiltered_link_url.is_empty();
   bool has_selection = !params_.selection_text.empty();
 
+  Browser* active_browser = BrowserList::FindBrowserWithTabContents(
+      source_tab_contents_);
+  if (active_browser && active_browser->is_app()) {
+    const std::string ext_id = web_app::GetExtensionIdFromApplicationName(
+        active_browser->app_name());
+    const Extension* app =
+        profile_->GetExtensionService()->GetInstalledExtension(ext_id);
+    if (app && app->is_platform_app()) {
+      int index = 0;
+      AppendExtensionItems(app->id(), &index);
+      return;
+    }
+  }
+
   if (AppendCustomItems()) {
     // If there's a selection, don't early return when there are custom items,
     // but fall through to adding the normal ones after the custom ones.

chrome/browser/ui/views/tab_contents/tab_contents_view_views.cc

@@ -341,6 +341,10 @@ void TabContentsViewViews::ShowContextMenu(const ContextMenuParams& params) {
   context_menu_.reset(new RenderViewContextMenuViews(tab_contents_, params));
   context_menu_->Init();
 
+  // Don't show empty menus.
+  if (context_menu_->menu_model().GetItemCount() == 0)
+    return;
+
   gfx::Point screen_point(params.x, params.y);
   views::View::ConvertPointToScreen(GetRootView(), &screen_point);
 

chrome/test/data/extensions/platform_apps/context_menu/main.html

+<script src="test.js"></script>

chrome/test/data/extensions/platform_apps/context_menu/manifest.json

+{
+  "name" : "Platform App Context Menus Test Extension",
+  "version" : "0.1",
+  "manifest_version": 2,
+  "permissions": [ "contextMenus" ],
+  "app": {
+    "launch": {
+      "local_path": "main.html",
+      "container": "shell"
+    }
+  },
+  "platform_app": true
+}

chrome/test/data/extensions/platform_apps/context_menu/test.js

+// 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.
+
+function onclick(info) {
+}
+
+window.onload = function() {
+  chrome.contextMenus.create({"title":"Extension Item 1",
+                              "onclick": onclick}, function() {
+    if (!chrome.extension.lastError) {
+      chrome.test.sendMessage("created item");
+    }
+  });
+};