Tab value for context menu onclick event should be optional.

In platform apps there are no tabs, and thus no way to pass a valid Tab value in this event.

BUG=142747


Review URL: https://chromiumcodereview.appspot.com/10832348

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

chrome/browser/extensions/menu_manager.cc

@@ -640,11 +640,14 @@ void MenuManager::ExecuteCommand(Profile* profile,
   args->Append(properties);
 
   // Add the tab info to the argument list.
-  // Note: web_contents only NULL in unit tests :(
-  if (web_contents)
-    args->Append(ExtensionTabUtil::CreateTabValue(web_contents));
-  else
-    args->Append(new DictionaryValue());
+  // No tab info in a platform app.
+  if (!extension || !extension->is_platform_app()) {
+    // Note: web_contents only NULL in unit tests :(
+    if (web_contents)
+      args->Append(ExtensionTabUtil::CreateTabValue(web_contents));
+    else
+      args->Append(new DictionaryValue());
+  }
 
   if (item->type() == MenuItem::CHECKBOX ||
       item->type() == MenuItem::RADIO) {

chrome/browser/extensions/platform_app_browsertest.cc

@@ -104,6 +104,33 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, AppWithContextMenu) {
   ASSERT_FALSE(menu->HasCommandWithId(IDC_SAVE_PAGE));
 }
 
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, AppWithContextMenuClicked) {
+  ExtensionTestMessageListener launched_listener("Launched", false);
+  LoadAndLaunchPlatformApp("context_menu_click");
+
+  // Wait for the extension to tell us it's initialized its context menus and
+  // launched a window.
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+
+  // Test that the menu item shows up
+  WebContents* web_contents = GetFirstShellWindowWebContents();
+  ASSERT_TRUE(web_contents);
+  WebKit::WebContextMenuData data;
+  content::ContextMenuParams params(data);
+  params.page_url = GURL("http://foo.bar");
+  PlatformAppContextMenu* menu = new PlatformAppContextMenu(web_contents,
+      params);
+  menu->Init();
+  ASSERT_TRUE(menu->HasCommandWithId(IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST));
+
+  // Execute the menu item
+  ExtensionTestMessageListener onclicked_listener("onClicked fired for id1",
+                                                  false);
+  menu->ExecuteCommand(IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST);
+
+  ASSERT_TRUE(onclicked_listener.WaitUntilSatisfied());
+}
+
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, DisallowNavigation) {
   ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunPlatformAppTest("platform_apps/navigation")) << message_;

chrome/common/extensions/api/context_menus.json

@@ -302,7 +302,8 @@
           {
             "name": "tab",
             "$ref": "tabs.Tab",
-            "description": "The details of the tab where the click took place."
+            "description": "The details of the tab where the click took place. If the click did not take place in a tab, this parameter will be missing.",
+            "optional": true
           }
         ]
       }

chrome/common/extensions/docs/apps/contextMenus.html

@@ -1204,6 +1204,7 @@ You can find samples of this API on the
                 <!-- TYPE -->
                 <div style="display:inline">
                   (
+                    <span class="optional">optional</span>
                     <span id="typeTemplate">
                       <span>
                         <a href="tabs.html#type-tabs.Tab">tabs.Tab</a>
@@ -1213,7 +1214,7 @@ You can find samples of this API on the
                 </div>
               </em>
           </dt>
-          <dd>The details of the tab where the click took place.</dd>
+          <dd>The details of the tab where the click took place. If the click did not take place in a tab, this parameter will be missing.</dd>
           <!-- OBJECT PROPERTIES -->
           <!-- OBJECT METHODS -->
           <!-- OBJECT EVENT FIELDS -->

chrome/common/extensions/docs/extensions/contextMenus.html

@@ -1287,6 +1287,7 @@ You can find samples of this API on the
                 <!-- TYPE -->
                 <div style="display:inline">
                   (
+                    <span class="optional">optional</span>
                     <span id="typeTemplate">
                       <span>
                         <a href="tabs.html#type-tabs.Tab">tabs.Tab</a>
@@ -1296,7 +1297,7 @@ You can find samples of this API on the
                 </div>
               </em>
           </dt>
-          <dd>The details of the tab where the click took place.</dd>
+          <dd>The details of the tab where the click took place. If the click did not take place in a tab, this parameter will be missing.</dd>
           <!-- OBJECT PROPERTIES -->
           <!-- OBJECT METHODS -->
           <!-- OBJECT EVENT FIELDS -->

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

+<!--
+ * Copyright (c) 2012 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.
+-->
+<html>
+<body>
+<script src="main.js"></script>
+</body>
+</html>

chrome/test/data/extensions/platform_apps/context_menu_click/main.js

+// Copyright (c) 2012 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.
+
+chrome.test.sendMessage('Launched');

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

+{
+  "name" : "Platform App Context Menus Test Extension",
+  "version" : "0.1",
+  "manifest_version": 2,
+  "permissions": [
+    "experimental",
+    "contextMenus"
+  ],
+  "app": {
+    "background": {
+      "scripts": ["test.js"]
+    }
+  }
+}

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

+// Copyright (c) 2012 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.
+
+chrome.app.runtime.onLaunched.addListener(function() {
+  chrome.contextMenus.create({
+        id: 'id1',
+        title: 'Extension Item 1',
+      },
+      function() {
+        chrome.app.window.create('main.html', {}, function() {});
+      });
+});
+
+chrome.contextMenus.onClicked.addListener(function(info, tab) {
+  chrome.test.assertEq(undefined, tab);
+  chrome.test.sendMessage("onClicked fired for " + info.menuItemId);
+});