Dispatch a new event chrome.contextMenus.onClicked.

Disallow onclick create parameter for event pages.

Fix onClickData, which had incorrect types and missing fields.

BUG=123366
TEST=Register a chrome.contextMenus.onClicked handler in an event page (adding the menu items upon chrome.runtime.onInstalled). Check that it fires.

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

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

12 files changed, 216 insertions, 16 deletions

diff --git a/chrome/browser/extensions/api/context_menu/context_menu_api.cc b/chrome/browser/extensions/api/context_menu/context_menu_api.cc
index 2976b06..15dd796 100644
--- a/chrome/browser/extensions/api/context_menu/context_menu_api.cc
+++ b/chrome/browser/extensions/api/context_menu/context_menu_api.cc
@@ -21,12 +21,16 @@ const char kDocumentUrlPatternsKey[] = "documentUrlPatterns";
 const char kEnabledKey[] = "enabled";
 const char kGeneratedIdKey[] = "generatedId";
 const char kIdKey[] = "id";
+const char kOnclickKey[] = "onclick";
 const char kParentIdKey[] = "parentId";
 const char kTargetUrlPatternsKey[] = "targetUrlPatterns";
 const char kTitleKey[] = "title";
 const char kTypeKey[] = "type";
 
 const char kCannotFindItemError[] = "Cannot find menu item with id *";
+const char kOnclickDisallowedError[] = "Extensions using event pages cannot "
+    "pass an onclick parameter to chrome.contextMenus.create. Instead, use "
+    "the chrome.contextMenus.onClicked event.";
 const char kCheckedError[] =
     "Only items with type \"radio\" or \"checkbox\" can be checked";
 const char kDuplicateIDError[] =
@@ -207,6 +211,12 @@ bool CreateContextMenuFunction::RunImpl() {
     return false;
   }
 
+  if (GetExtension()->has_lazy_background_page() &&
+      properties->HasKey(kOnclickKey)) {
+    error_ = kOnclickDisallowedError;
+    return false;
+  }
+
   ExtensionMenuItem::ContextList contexts(ExtensionMenuItem::PAGE);
   if (!ParseContexts(*properties, kContextsKey, &contexts))
     return false;
diff --git a/chrome/browser/extensions/api/context_menu/context_menu_apitest.cc b/chrome/browser/extensions/api/context_menu/context_menu_apitest.cc
index 9f80598..3a76949 100644
--- a/chrome/browser/extensions/api/context_menu/context_menu_apitest.cc
+++ b/chrome/browser/extensions/api/context_menu/context_menu_apitest.cc
@@ -16,8 +16,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ContextMenus) {
   ASSERT_TRUE(RunExtensionTest("context_menus/basics")) << message_;
   ASSERT_TRUE(RunExtensionTest("context_menus/no_perms")) << message_;
   ASSERT_TRUE(RunExtensionTest("context_menus/item_ids")) << message_;
-  ASSERT_TRUE(RunExtensionTest("context_menus/event_page_item_ids"))
-      << message_;
+  ASSERT_TRUE(RunExtensionTest("context_menus/event_page")) << message_;
 }
 
 // crbug.com/51436 -- creating context menus from multiple script contexts
diff --git a/chrome/browser/extensions/extension_event_names.cc b/chrome/browser/extensions/extension_event_names.cc
index 0a0c7f8..5a8391f 100644
--- a/chrome/browser/extensions/extension_event_names.cc
+++ b/chrome/browser/extensions/extension_event_names.cc
@@ -42,6 +42,9 @@ const char kOnFileBrowserNetworkConnectionChanged[] =
 
 const char kOnInputMethodChanged[] = "inputMethodPrivate.onChanged";
 
+const char kOnContextMenus[] = "contextMenus";
+const char kOnContextMenuClicked[] = "contextMenus.onClicked";
+
 const char kOnDownloadCreated[] = "experimental.downloads.onCreated";
 const char kOnDownloadChanged[] = "experimental.downloads.onChanged";
 const char kOnDownloadErased[] = "experimental.downloads.onErased";
diff --git a/chrome/browser/extensions/extension_event_names.h b/chrome/browser/extensions/extension_event_names.h
index 76afdef..b65fbafe 100644
--- a/chrome/browser/extensions/extension_event_names.h
+++ b/chrome/browser/extensions/extension_event_names.h
@@ -46,6 +46,10 @@ extern const char kOnFileBrowserNetworkConnectionChanged[];
 // InputMethod.
 extern const char kOnInputMethodChanged[];
 
+// Context menus.
+extern const char kOnContextMenus[];
+extern const char kOnContextMenuClicked[];
+
 // Downloads.
 extern const char kOnDownloadCreated[];
 extern const char kOnDownloadChanged[];
diff --git a/chrome/browser/extensions/extension_menu_manager.cc b/chrome/browser/extensions/extension_menu_manager.cc
index 99d6fc4..d0fb437 100644
--- a/chrome/browser/extensions/extension_menu_manager.cc
+++ b/chrome/browser/extensions/extension_menu_manager.cc
@@ -12,6 +12,7 @@
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
+#include "chrome/browser/extensions/extension_event_names.h"
 #include "chrome/browser/extensions/extension_event_router.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -603,11 +604,14 @@ void ExtensionMenuManager::ExecuteCommand(
 
   std::string json_args;
   base::JSONWriter::Write(&args, &json_args);
-  std::string event_name = "contextMenus";
   event_router->DispatchEventToExtension(
-      item->extension_id(), event_name, json_args, profile, GURL(),
+      item->extension_id(), extension_event_names::kOnContextMenus,
+      json_args, profile, GURL(),
+      ExtensionEventRouter::USER_GESTURE_ENABLED);
+  event_router->DispatchEventToExtension(
+      item->extension_id(), extension_event_names::kOnContextMenuClicked,
+      json_args, profile, GURL(),
       ExtensionEventRouter::USER_GESTURE_ENABLED);
-  // TODO(yoz): dispatch another event onClicked.
 }
 
 void ExtensionMenuManager::SanitizeRadioList(
diff --git a/chrome/browser/extensions/extension_menu_manager_unittest.cc b/chrome/browser/extensions/extension_menu_manager_unittest.cc
index 5e3173c..1ce3249 100644
--- a/chrome/browser/extensions/extension_menu_manager_unittest.cc
+++ b/chrome/browser/extensions/extension_menu_manager_unittest.cc
@@ -11,6 +11,7 @@
 #include "base/scoped_temp_dir.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
+#include "chrome/browser/extensions/extension_event_names.h"
 #include "chrome/browser/extensions/extension_event_router.h"
 #include "chrome/browser/extensions/extension_menu_manager.h"
 #include "chrome/browser/extensions/test_extension_prefs.h"
@@ -29,6 +30,7 @@ using content::BrowserThread;
 using extensions::Extension;
 using testing::_;
 using testing::AtLeast;
+using testing::InSequence;
 using testing::Return;
 using testing::SaveArg;
 
@@ -476,18 +478,28 @@ TEST_F(ExtensionMenuManagerTest, ExecuteCommand) {
   // Use the magic of googlemock to save a parameter to our mock's
   // DispatchEventToExtension method into event_args.
   std::string event_args;
-  std::string expected_event_name = "contextMenus";
+  {
+    InSequence s;
+    EXPECT_CALL(*mock_event_router.get(),
+                DispatchEventToExtension(
+                    item->extension_id(),
+                  extension_event_names::kOnContextMenus,
+                  _,
+                  &profile,
+                  GURL(),
+                  ExtensionEventRouter::USER_GESTURE_ENABLED))
+      .Times(1)
+      .WillOnce(SaveArg<2>(&event_args));
   EXPECT_CALL(*mock_event_router.get(),
               DispatchEventToExtension(
                   item->extension_id(),
-                  expected_event_name,
+                  extension_event_names::kOnContextMenuClicked,
                   _,
                   &profile,
                   GURL(),
                   ExtensionEventRouter::USER_GESTURE_ENABLED))
-      .Times(1)
-      .WillOnce(SaveArg<2>(&event_args));
-
+      .Times(1);
+  }
   manager_.ExecuteCommand(&profile, NULL /* tab_contents */, params, id);
 
   // Parse the json event_args, which should turn into a 2-element list where
diff --git a/chrome/common/extensions/api/context_menus.json b/chrome/common/extensions/api/context_menus.json
index c0aae39..72ee542 100644
--- a/chrome/common/extensions/api/context_menus.json
+++ b/chrome/common/extensions/api/context_menus.json
@@ -57,8 +57,18 @@
             "description": "The text for the context selection, if any."
           },
           "editable": {
-            "type": "string",
+            "type": "boolean",
             "description": "A flag indicating whether the element is editable (text input, textarea, etc.)."
+          },
+          "wasChecked": {
+            "type": "boolean",
+            "optional": true,
+            "description": "A flag indicating the state of a checkbox or radio item before it was clicked."
+          },
+          "checked": {
+            "type": "boolean",
+            "optional": true,
+            "description": "A flag indicating the state of a checkbox or radio item after it is clicked."
           }
         }
       }
@@ -114,7 +124,7 @@
               "onclick": {
                 "type": "function",
                 "optional": true,
-                "description": "A function that will be called back when the menu item is clicked.",
+                "description": "A function that will be called back when the menu item is clicked. Event pages cannot use this; instead, they should register a listener for chrome.contextMenus.onClicked.",
                 "parameters": [
                   {
                     "name": "info",
@@ -277,6 +287,25 @@
           }
         ]
       }
+    ],
+    "events": [
+      {
+        "name": "onClicked",
+        "type": "function",
+        "description": "Fired when a context menu item is clicked.",
+        "parameters": [
+          {
+            "name": "info",
+            "$ref": "OnClickData",
+            "description": "Information about the item clicked and the context where the click happened."
+          },
+          {
+            "name": "tab",
+            "$ref": "tabs.Tab",
+            "description": "The details of the tab where the click took place."
+          }
+        ]
+      }
     ]
   }
 ]
diff --git a/chrome/common/extensions/docs/contextMenus.html b/chrome/common/extensions/docs/contextMenus.html
index 571747f2..efa5eef 100644
--- a/chrome/common/extensions/docs/contextMenus.html
+++ b/chrome/common/extensions/docs/contextMenus.html
@@ -218,6 +218,14 @@
             </ol>
           </li>
                   <li>
+            <a href="#global-events">Events</a>
+            <ol>
+              <li>
+                <a href="#event-onClicked">onClicked</a>
+              </li>
+            </ol>
+          </li>
+                  <li>
             <a href="#types">Types</a>
             <ol>
               <li>
@@ -486,7 +494,7 @@ You can find samples of this API on the
                 </div>
               </em>
           </dt>
-          <dd>A function that will be called back when the menu item is clicked.</dd>
+          <dd>A function that will be called back when the menu item is clicked. Event pages cannot use this; instead, they should register a listener for chrome.contextMenus.onClicked.</dd>
           <!-- OBJECT PROPERTIES -->
           <!-- OBJECT METHODS -->
           <!-- OBJECT EVENT FIELDS -->
@@ -1234,7 +1242,80 @@ You can find samples of this API on the
             </div>  <!-- /apiItem -->
           </div>  <!-- /apiGroup -->
           <!-- EVENTS -->
-           <!-- /apiGroup -->
+          <div id="eventsTemplate" class="apiGroup">
+            <a name="global-events"></a>
+            <h3>Events</h3>
+            <!-- iterates over all events -->
+            <div class="apiItem">
+              <a name="event-onClicked"></a>
+              <h4>onClicked</h4>
+              <div class="summary">
+                <!-- Note: intentionally longer 80 columns -->
+                <span class="subdued">chrome.contextMenus.</span><span>onClicked</span><span class="subdued">.addListener</span>(function(<span>OnClickData info, tabs.Tab tab</span>) <span class="subdued">{...}</span><span></span>);
+              </div>
+              <div class="description">
+                <p>Fired when a context menu item is clicked.</p>
+                <!-- LISTENER PARAMETERS -->
+                <div>
+                  <h4>Listener parameters</h4>
+                  <dl>
+                    <div>
+                      <div>
+          <dt>
+            <var>info</var>
+              <em>
+                <!-- TYPE -->
+                <div style="display:inline">
+                  (
+                    <span id="typeTemplate">
+                      <span>
+                        <a href="contextMenus.html#type-OnClickData">OnClickData</a>
+                      </span>
+                    </span>
+                  )
+                </div>
+              </em>
+          </dt>
+          <dd>Information about the item clicked and the context where the click happened.</dd>
+          <!-- OBJECT PROPERTIES -->
+          <!-- OBJECT METHODS -->
+          <!-- OBJECT EVENT FIELDS -->
+          <!-- FUNCTION PARAMETERS -->
+        </div>
+                    </div><div>
+                      <div>
+          <dt>
+            <var>tab</var>
+              <em>
+                <!-- TYPE -->
+                <div style="display:inline">
+                  (
+                    <span id="typeTemplate">
+                      <span>
+                        <a>tabs.Tab</a>
+                      </span>
+                    </span>
+                  )
+                </div>
+              </em>
+          </dt>
+          <dd>The details of the tab where the click took place.</dd>
+          <!-- OBJECT PROPERTIES -->
+          <!-- OBJECT METHODS -->
+          <!-- OBJECT EVENT FIELDS -->
+          <!-- FUNCTION PARAMETERS -->
+        </div>
+                    </div>
+                  </dl>
+                </div>
+                <!-- EXTRA PARAMETERS -->
+                <!-- LISTENER RETURN VALUE -->
+                <dl>
+                </dl>
+              </div> <!-- /description -->
+               <!-- /description -->
+            </div> <!-- /apiItem -->
+          </div> <!-- /apiGroup -->
           <!-- TYPES -->
           <div class="apiGroup">
             <a name="types"></a>
@@ -1462,7 +1543,7 @@ You can find samples of this API on the
                   (
                     <span id="typeTemplate">
                       <span>
-                        <span>string</span>
+                        <span>boolean</span>
                       </span>
                     </span>
                   )
@@ -1475,6 +1556,54 @@ You can find samples of this API on the
           <!-- OBJECT EVENT FIELDS -->
           <!-- FUNCTION PARAMETERS -->
         </div>
+              </div><div>
+                <div>
+          <dt>
+            <var>wasChecked</var>
+              <em>
+                <!-- TYPE -->
+                <div style="display:inline">
+                  (
+                    <span class="optional">optional</span>
+                    <span id="typeTemplate">
+                      <span>
+                        <span>boolean</span>
+                      </span>
+                    </span>
+                  )
+                </div>
+              </em>
+          </dt>
+          <dd>A flag indicating the state of a checkbox or radio item before it was clicked.</dd>
+          <!-- OBJECT PROPERTIES -->
+          <!-- OBJECT METHODS -->
+          <!-- OBJECT EVENT FIELDS -->
+          <!-- FUNCTION PARAMETERS -->
+        </div>
+              </div><div>
+                <div>
+          <dt>
+            <var>checked</var>
+              <em>
+                <!-- TYPE -->
+                <div style="display:inline">
+                  (
+                    <span class="optional">optional</span>
+                    <span id="typeTemplate">
+                      <span>
+                        <span>boolean</span>
+                      </span>
+                    </span>
+                  )
+                </div>
+              </em>
+          </dt>
+          <dd>A flag indicating the state of a checkbox or radio item after it is clicked.</dd>
+          <!-- OBJECT PROPERTIES -->
+          <!-- OBJECT METHODS -->
+          <!-- OBJECT EVENT FIELDS -->
+          <!-- FUNCTION PARAMETERS -->
+        </div>
               </div>
             </dl>
           </dd>
diff --git a/chrome/common/extensions/docs/samples.json b/chrome/common/extensions/docs/samples.json
index 7d6accb..3f87955 100644
--- a/chrome/common/extensions/docs/samples.json
+++ b/chrome/common/extensions/docs/samples.json
@@ -53,6 +53,7 @@
     "chrome.contentSettings.ContentSetting.getResourceIdentifiers": "contentSettings.html#method-ContentSetting-getResourceIdentifiers",
     "chrome.contentSettings.ContentSetting.set": "contentSettings.html#method-ContentSetting-set",
     "chrome.contextMenus.create": "contextMenus.html#method-create",
+    "chrome.contextMenus.onClicked": "contextMenus.html#event-onClicked",
     "chrome.contextMenus.remove": "contextMenus.html#method-remove",
     "chrome.contextMenus.removeAll": "contextMenus.html#method-removeAll",
     "chrome.contextMenus.update": "contextMenus.html#method-update",
@@ -241,6 +242,7 @@
     "chrome.extension.sendMessage": "extension.html#method-sendMessage",
     "chrome.extension.setUpdateUrlData": "extension.html#method-setUpdateUrlData",
     "chrome.fileBrowserHandler.onExecute": "fileBrowserHandler.html#event-onExecute",
+    "chrome.fileBrowserHandler.selectFile": "fileBrowserHandler.html#method-selectFile",
     "chrome.history.addUrl": "history.html#method-addUrl",
     "chrome.history.deleteAll": "history.html#method-deleteAll",
     "chrome.history.deleteRange": "history.html#method-deleteRange",
diff --git a/chrome/test/data/extensions/api_test/context_menus/event_page_item_ids/manifest.json b/chrome/test/data/extensions/api_test/context_menus/event_page/manifest.json
index a1d83b6..a1d83b6 100644
--- a/chrome/test/data/extensions/api_test/context_menus/event_page_item_ids/manifest.json
+++ b/chrome/test/data/extensions/api_test/context_menus/event_page/manifest.json
diff --git a/chrome/test/data/extensions/api_test/context_menus/event_page_item_ids/test.js b/chrome/test/data/extensions/api_test/context_menus/event_page/test.js
index 119fd0c..930b5e4 100644
--- a/chrome/test/data/extensions/api_test/context_menus/event_page_item_ids/test.js
+++ b/chrome/test/data/extensions/api_test/context_menus/event_page/test.js
@@ -17,5 +17,14 @@ chrome.test.runTests([
         {"title": "title2"},
         chrome.test.callbackFail("Extensions using event pages must pass an " +
                                  "id parameter to chrome.contextMenus.create"));
+  },
+
+  function noOnClick() {
+    chrome.contextMenus.create(
+        {"id": "id3", "title": "title3", "onclick": function() {}},
+        chrome.test.callbackFail(
+            "Extensions using event pages cannot pass an onclick parameter " +
+            "to chrome.contextMenus.create. Instead, use the " +
+            "chrome.contextMenus.onClicked event."));
   }
 ]);
diff --git a/chrome/test/data/extensions/platform_apps/context_menu/test.js b/chrome/test/data/extensions/platform_apps/context_menu/test.js
index a3d1f46..f762ade 100644
--- a/chrome/test/data/extensions/platform_apps/context_menu/test.js
+++ b/chrome/test/data/extensions/platform_apps/context_menu/test.js
@@ -6,7 +6,6 @@ chrome.experimental.app.onLaunched.addListener(function() {
   chrome.contextMenus.create({
         id: 'id1',
         title: 'Extension Item 1',
-        onclick: function() {}
       },
       function() {
         chrome.windows.create({