<webview>: Context menu API implementation CL.

chrome/browser/extensions/api/webview/webview_api.cc

Index: chrome/browser/extensions/api/webview/webview_api.cc
diff --git a/chrome/browser/extensions/api/webview/webview_api.cc b/chrome/browser/extensions/api/webview/webview_api.cc
index 669f59ac6ae0c732b0286fdfb24c9fada1588292..ed477e4e1781e26c4ada56786c8abfe57e75e190 100644
--- a/chrome/browser/extensions/api/webview/webview_api.cc
+++ b/chrome/browser/extensions/api/webview/webview_api.cc
@@ -6,6 +6,7 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/api/browsing_data/browsing_data_api.h"
+#include "chrome/browser/extensions/api/context_menus/context_menus_api.h"
 #include "chrome/browser/extensions/api/context_menus/context_menus_api_helpers.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
@@ -66,6 +67,7 @@ bool WebviewContextMenusCreateFunction::RunImpl() {
   MenuItem::Id id(
       Profile::FromBrowserContext(browser_context())->IsOffTheRecord(),
       extension_id());
+  id.extension_key.webview_instance_id = params->instance_id;

[Fady Samuel, 2014/03/05 17:54:10]

MenuItem::Id should probably take in an ExtensionKey instead.

[lazyboy, 2014/03/05 18:27:59]

Done.

[lazyboy, 2014/03/05 18:27:59]

Done.

 
   if (params->create_properties.id.get()) {
     id.string_uid = *params->create_properties.id;
@@ -77,9 +79,15 @@ bool WebviewContextMenusCreateFunction::RunImpl() {
         properties->GetInteger(helpers::kGeneratedIdKey, &id.uid));
   }
 
-  // TODO(lazyboy): Implement.
-  SendResponse(false);
-  return false;
+  bool success = extensions::context_menus_api_helpers::CreateMenuItem(
+      params->create_properties,
+      Profile::FromBrowserContext(browser_context()),
+      GetExtension(),
+      id,
+      &error_);
+
+  SendResponse(success);
+  return success;
 }
 
 bool WebviewContextMenusUpdateFunction::RunImpl() {
@@ -89,6 +97,7 @@ bool WebviewContextMenusUpdateFunction::RunImpl() {
 
   Profile* profile = Profile::FromBrowserContext(browser_context());
   MenuItem::Id item_id(profile->IsOffTheRecord(), extension_id());

[Fady Samuel, 2014/03/05 17:54:10]

As above, MenuItem::Id should probably take in an ExtensionKey

[lazyboy, 2014/03/05 18:27:59]

Done.

+  item_id.extension_key.webview_instance_id = params->instance_id;
 
   if (params->id.as_string)
     item_id.string_uid = *params->id.as_string;
@@ -97,9 +106,10 @@ bool WebviewContextMenusUpdateFunction::RunImpl() {
   else
     NOTREACHED();
 
-  // TODO(lazyboy): Implement.
-  SendResponse(false);
-  return false;
+  bool success = extensions::context_menus_api_helpers::UpdateMenuItem(
+      params->update_properties, profile, GetExtension(), item_id, &error_);
+  SendResponse(success);
+  return success;
 }
 
 bool WebviewContextMenusRemoveFunction::RunImpl() {
@@ -107,9 +117,36 @@ bool WebviewContextMenusRemoveFunction::RunImpl() {
       webview::ContextMenusRemove::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
-  // TODO(lazyboy): Implement.
-  SendResponse(false);
-  return false;
+  MenuManager* menu_manager =
+      MenuManager::Get(Profile::FromBrowserContext(browser_context()));
+
+  MenuItem::Id id(
+      Profile::FromBrowserContext(browser_context())->IsOffTheRecord(),
+      extension_id());

[Fady Samuel, 2014/03/05 17:54:10]

As above, MenuItem::Id should probably take in an ExtensionKey.

[lazyboy, 2014/03/05 18:27:59]

Done.

[lazyboy, 2014/03/05 18:27:59]

Done.

+  id.extension_key.webview_instance_id = params->instance_id;
+
+  if (params->menu_item_id.as_string) {
+    id.string_uid = *params->menu_item_id.as_string;
+  } else if (params->menu_item_id.as_integer) {
+    id.uid = *params->menu_item_id.as_integer;
+  } else {
+    NOTREACHED();
+  }
+
+  bool success = true;
+  MenuItem* item = menu_manager->GetItemById(id);
+  // Ensure one extension can't remove another's menu items.
+  if (!item || item->extension_id() != extension_id()) {

[Fady Samuel, 2014/03/05 17:54:10]

If item->extension() != extension_id() then we have a compromised renderer,
right? This should probably be a CHECK:
http://www.chromium.org/developers/coding-style

[lazyboy, 2014/03/05 18:27:59]

This is code from context_menus_api.cc and it was like this there.
The API allows you to call contextMenus.remove with any id, and this bit throws
an error if you're not allowed. Don't think we should kill the renderer here.

+    error_ = ErrorUtils::FormatErrorMessage(
+        context_menus_api_helpers::kCannotFindItemError,
+        context_menus_api_helpers::GetIDString(id));
+    success = false;
+  } else if (!menu_manager->RemoveContextMenuItem(id)) {
+    success = false;
+  }
+
+  SendResponse(success);
+  return success;
 }
 
 bool WebviewContextMenusRemoveAllFunction::RunImpl() {
@@ -117,9 +154,14 @@ bool WebviewContextMenusRemoveAllFunction::RunImpl() {
       webview::ContextMenusRemoveAll::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
-  // TODO(lazyboy): Implement.
-  SendResponse(false);
-  return false;
+  MenuManager* menu_manager =
+      MenuManager::Get(Profile::FromBrowserContext(browser_context()));
+
+  int webview_instance_id = params->instance_id;
+  menu_manager->RemoveAllContextItems(
+      MenuItem::ExtensionKey(GetExtension()->id(), webview_instance_id));
+  SendResponse(true);
+  return true;
 }
 
 WebviewClearDataFunction::WebviewClearDataFunction()