<webview>: Context menu API implementation CL.

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

 // Copyright (c) 2013 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.
 
 #include "chrome/browser/extensions/api/webview/webview_api.h"
 
 #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"
 #include "chrome/common/extensions/api/webview.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/stop_find_action.h"
 #include "extensions/common/error_utils.h"
@@ skipping 40 matching lines @@
 // TODO(lazyboy): Add checks similar to
 // WebviewExtensionFunction::RunImplSafe(WebViewGuest*).
 bool WebviewContextMenusCreateFunction::RunImpl() {
   scoped_ptr<webview::ContextMenusCreate::Params> params(
       webview::ContextMenusCreate::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   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;
   } else {
     // The Generated Id is added by webview_custom_bindings.js.
     base::DictionaryValue* properties = NULL;
     EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(1, &properties));
     EXTENSION_FUNCTION_VALIDATE(
         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() {
   scoped_ptr<webview::ContextMenusUpdate::Params> params(
       webview::ContextMenusUpdate::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   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;
   else if (params->id.as_integer)
     item_id.uid = *params->id.as_integer;
   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() {
   scoped_ptr<webview::ContextMenusRemove::Params> params(
       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() {
   scoped_ptr<webview::ContextMenusRemoveAll::Params> params(
       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()
     : remove_mask_(0), bad_message_(false) {}
 
 WebviewClearDataFunction::~WebviewClearDataFunction() {}
 
 // Parses the |dataToRemove| argument to generate the remove mask. Sets
 // |bad_message_| (like EXTENSION_FUNCTION_VALIDATE would if this were a bool
 // method) if 'dataToRemove' is not present.
@@ skipping 354 matching lines @@
 
 WebviewTerminateFunction::~WebviewTerminateFunction() {
 }
 
 bool WebviewTerminateFunction::RunImplSafe(WebViewGuest* guest) {
   guest->Terminate();
   return true;
 }
 
 }  // namespace extensions