<webview>: Context menu API implementation CL.

chrome/browser/extensions/menu_manager.cc

Index: chrome/browser/extensions/menu_manager.cc
diff --git a/chrome/browser/extensions/menu_manager.cc b/chrome/browser/extensions/menu_manager.cc
index 424366390b34035298c5790bd32d417cd7ed67a8..fe8799b0d80b138c52ab2efcfcca5c70a96c8c86 100644
--- a/chrome/browser/extensions/menu_manager.cc
+++ b/chrome/browser/extensions/menu_manager.cc
@@ -19,6 +19,7 @@
 #include "chrome/browser/extensions/menu_manager_factory.h"
 #include "chrome/browser/extensions/state_store.h"
 #include "chrome/browser/extensions/tab_helper.h"
+#include "chrome/browser/guestview/guestview.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/context_menus.h"
 #include "content/public/browser/notification_details.h"
@@ -322,40 +323,38 @@ std::set<std::string> MenuManager::ExtensionIds() {
   std::set<std::string> id_set;
   for (MenuItemMap::const_iterator i = context_items_.begin();
        i != context_items_.end(); ++i) {
-    id_set.insert(i->first);
+    id_set.insert(i->first.extension_id);
   }
   return id_set;
 }
 
 const MenuItem::List* MenuManager::MenuItems(
-    const std::string& extension_id) {
-  MenuItemMap::iterator i = context_items_.find(extension_id);
+    const MenuItem::ExtensionKey& key) {
+  MenuItemMap::iterator i = context_items_.find(key);
   if (i != context_items_.end()) {
     return &(i->second);
   }
   return NULL;
 }
 
-bool MenuManager::AddContextItem(
-    const Extension* extension,
-    MenuItem* item) {
-  const std::string& extension_id = item->extension_id();
+bool MenuManager::AddContextItem(const Extension* extension, MenuItem* item) {
+  const MenuItem::ExtensionKey key(item->id());
   // The item must have a non-empty extension id, and not have already been
   // added.
-  if (extension_id.empty() || ContainsKey(items_by_id_, item->id()))
+  if (key.empty() || ContainsKey(items_by_id_, item->id()))
     return false;
 
-  DCHECK_EQ(extension->id(), extension_id);
+  DCHECK_EQ(extension->id(), key.extension_id);
 
-  bool first_item = !ContainsKey(context_items_, extension_id);
-  context_items_[extension_id].push_back(item);
+  bool first_item = !ContainsKey(context_items_, key);
+  context_items_[key].push_back(item);
   items_by_id_[item->id()] = item;
 
   if (item->type() == MenuItem::RADIO) {
     if (item->checked())
       RadioItemSelected(item);
     else
-      SanitizeRadioList(context_items_[extension_id]);
+      SanitizeRadioList(context_items_[key]);
   }
 
   // If this is the first item for this extension, start loading its icon.
@@ -370,9 +369,14 @@ bool MenuManager::AddChildItem(const MenuItem::Id& parent_id,
   MenuItem* parent = GetItemById(parent_id);
   if (!parent || parent->type() != MenuItem::NORMAL ||
       parent->incognito() != child->incognito() ||
-      parent->extension_id() != child->extension_id() ||
       ContainsKey(items_by_id_, child->id()))
     return false;
+
+  const MenuItem::ExtensionKey parent_key(parent_id);
+  const MenuItem::ExtensionKey child_key(child->id());
+  if (parent_key != child_key)
+    return false;
+
   parent->AddChild(child);
   items_by_id_[child->id()] = child;
 
@@ -424,14 +428,14 @@ bool MenuManager::ChangeParent(const MenuItem::Id& child_id,
   } else {
     // This is a top-level item, so we need to pull it out of our list of
     // top-level items.
-    MenuItemMap::iterator i = context_items_.find(child->extension_id());
+    MenuItem::ExtensionKey child_key(child->id());
+    MenuItemMap::iterator i = context_items_.find(child_key);
     if (i == context_items_.end()) {
       NOTREACHED();
       return false;
     }
     MenuItem::List& list = i->second;
-    MenuItem::List::iterator j = std::find(list.begin(), list.end(),
-                                                    child);
+    MenuItem::List::iterator j = std::find(list.begin(), list.end(), child);
     if (j == list.end()) {
       NOTREACHED();
       return false;
@@ -444,9 +448,10 @@ bool MenuManager::ChangeParent(const MenuItem::Id& child_id,
     new_parent->AddChild(child);
     SanitizeRadioList(new_parent->children());
   } else {
-    context_items_[child->extension_id()].push_back(child);
+    MenuItem::ExtensionKey child_key(child->id());
+    context_items_[child_key].push_back(child);
     child->parent_id_.reset(NULL);
-    SanitizeRadioList(context_items_[child->extension_id()]);
+    SanitizeRadioList(context_items_[child_key]);
   }
   return true;
 }
@@ -457,8 +462,8 @@ bool MenuManager::RemoveContextMenuItem(const MenuItem::Id& id) {
 
   MenuItem* menu_item = GetItemById(id);
   DCHECK(menu_item);
-  std::string extension_id = menu_item->extension_id();
-  MenuItemMap::iterator i = context_items_.find(extension_id);
+  MenuItem::ExtensionKey key(id);
+  MenuItemMap::iterator i = context_items_.find(key);
   if (i == context_items_.end()) {
     NOTREACHED();
     return false;
@@ -503,16 +508,18 @@ bool MenuManager::RemoveContextMenuItem(const MenuItem::Id& id) {
   }
 
   if (list.empty()) {
-    context_items_.erase(extension_id);
-    icon_manager_.RemoveIcon(extension_id);
+    context_items_.erase(key);
+    icon_manager_.RemoveIcon(key.extension_id);
   }
   return result;
 }
 
-void MenuManager::RemoveAllContextItems(const std::string& extension_id) {
+void MenuManager::RemoveAllContextItems(
+    const MenuItem::ExtensionKey& extension_key) {
   MenuItem::List::iterator i;
-  for (i = context_items_[extension_id].begin();
-       i != context_items_[extension_id].end(); ++i) {
+  for (i = context_items_[extension_key].begin();
+       i != context_items_[extension_key].end();
+       ++i) {
     MenuItem* item = *i;
     items_by_id_.erase(item->id());
 
@@ -523,9 +530,9 @@ void MenuManager::RemoveAllContextItems(const std::string& extension_id) {
       items_by_id_.erase(*j);
     }
   }
-  STLDeleteElements(&context_items_[extension_id]);
-  context_items_.erase(extension_id);
-  icon_manager_.RemoveIcon(extension_id);
+  STLDeleteElements(&context_items_[extension_key]);
+  context_items_.erase(extension_key);
+  icon_manager_.RemoveIcon(extension_key.extension_id);
 }
 
 MenuItem* MenuManager::GetItemById(const MenuItem::Id& id) const {
@@ -549,11 +556,12 @@ void MenuManager::RadioItemSelected(MenuItem* item) {
     }
     list = &(parent->children());
   } else {
-    if (context_items_.find(item->extension_id()) == context_items_.end()) {
+    MenuItem::ExtensionKey item_extension_key(item->id());
+    if (context_items_.find(item_extension_key) == context_items_.end()) {
       NOTREACHED();
       return;
     }
-    list = &context_items_[item->extension_id()];
+    list = &context_items_[item_extension_key];
   }
 
   // Find where |item| is in the list.
@@ -646,6 +654,10 @@ void MenuManager::ExecuteCommand(Profile* profile,
 
   properties->SetBoolean("editable", params.is_editable);
 
+  GuestView* guest_view = GuestView::FromWebContents(web_contents);

[Fady Samuel, 2014/03/04 21:14:59]

Can we make this WebViewGuest::FromWebContents instead? It's not yet clear
whether we want non-<webview> guests to have this functionality. We can change
it when the need arises.

[lazyboy, 2014/03/04 23:07:02]

Done.

Also, the webviewInstanceId thing is something I'm using in
webview_custom_bindings.js, I forgot about that. However, I'm not exposing it to
the developer.

+  if (guest_view)
+    properties->SetInteger("webviewInstanceId", guest_view->view_instance_id());
+
   args->Append(properties);
 
   // Add the tab info to the argument list.
@@ -683,6 +695,7 @@ void MenuManager::ExecuteCommand(Profile* profile,
   }
 
   {
+    // Dispatch to menu item's .onclick handler.
     scoped_ptr<Event> event(new Event(
         event_names::kOnContextMenus,
         scoped_ptr<base::ListValue>(args->DeepCopy())));
@@ -691,10 +704,15 @@ void MenuManager::ExecuteCommand(Profile* profile,
     event_router->DispatchEventToExtension(item->extension_id(), event.Pass());
   }
   {
-    scoped_ptr<Event> event(new Event(context_menus::OnClicked::kEventName,
-                                      args.Pass()));
+    // Dispatch to .contextMenus.onClicked handler.
+    scoped_ptr<Event> event(
+        new Event(guest_view ? event_names::kOnWebviewContextMenus
+                             : context_menus::OnClicked::kEventName,
+                  args.Pass()));
     event->restrict_to_browser_context = profile;
     event->user_gesture = EventRouter::USER_GESTURE_ENABLED;
+    if (guest_view)
+      event->filter_info.SetInstanceID(guest_view->view_instance_id());
     event_router->DispatchEventToExtension(item->extension_id(), event.Pass());
   }
 }
@@ -743,8 +761,8 @@ bool MenuManager::ItemUpdated(const MenuItem::Id& id) {
   if (menu_item->parent_id()) {
     SanitizeRadioList(GetItemById(*menu_item->parent_id())->children());
   } else {
-    std::string extension_id = menu_item->extension_id();
-    MenuItemMap::iterator i = context_items_.find(extension_id);
+    MenuItem::ExtensionKey key(menu_item->id());
+    MenuItemMap::iterator i = context_items_.find(key);
     if (i == context_items_.end()) {
       NOTREACHED();
       return false;
@@ -758,18 +776,21 @@ bool MenuManager::ItemUpdated(const MenuItem::Id& id) {
 void MenuManager::WriteToStorage(const Extension* extension) {
   if (!BackgroundInfo::HasLazyBackgroundPage(extension))
     return;
-  const MenuItem::List* top_items = MenuItems(extension->id());
+  const MenuItem::List* top_items =
+      MenuItems(MenuItem::ExtensionKey(extension->id()));
   MenuItem::List all_items;
   if (top_items) {
     for (MenuItem::List::const_iterator i = top_items->begin();
          i != top_items->end(); ++i) {
+      DCHECK(!(*i)->id().webview_instance_id);
       (*i)->GetFlattenedSubtree(&all_items);
     }
   }
 
-  if (store_)
+  if (store_) {
     store_->SetExtensionValue(extension->id(), kContextMenusKey,
                               MenuItemsToValue(all_items));
+  }
 }
 
 void MenuManager::ReadFromStorage(const std::string& extension_id,
@@ -804,8 +825,9 @@ void MenuManager::Observe(int type,
       // Remove menu items for disabled/uninstalled extensions.
       const Extension* extension =
           content::Details<UnloadedExtensionInfo>(details)->extension;
-      if (ContainsKey(context_items_, extension->id())) {
-        RemoveAllContextItems(extension->id());
+      MenuItem::ExtensionKey extension_key(extension->id());
+      if (ContainsKey(context_items_, extension_key)) {
+        RemoveAllContextItems(extension_key);
       }
       break;
     }
@@ -859,19 +881,52 @@ void MenuManager::RemoveAllIncognitoContextItems() {
     RemoveContextMenuItem(*remove_iter);
 }
 
-MenuItem::Id::Id() : incognito(false), uid(0) {}
+MenuItem::ExtensionKey::ExtensionKey(const std::string& extension_id,
+                                     int webview_instance_id)
+    : extension_id(extension_id), webview_instance_id(webview_instance_id) {}
+
+MenuItem::ExtensionKey::ExtensionKey(const MenuItem::Id& item_id)

[Fady Samuel, 2014/03/04 21:14:59]

We can probably get rid of this if MenuItem::Id has an ExtensionKey.

[lazyboy, 2014/03/04 23:07:02]

Done.

+    : extension_id(item_id.extension_id),
+      webview_instance_id(item_id.webview_instance_id) {}
+
+MenuItem::ExtensionKey::ExtensionKey(const std::string& extension_id)
+    : extension_id(extension_id), webview_instance_id(0) {}
+
+bool MenuItem::ExtensionKey::operator==(const ExtensionKey& other) const {
+  return extension_id == other.extension_id &&
+         webview_instance_id == other.webview_instance_id;
+}
+
+bool MenuItem::ExtensionKey::operator<(const ExtensionKey& other) const {
+  if (webview_instance_id != other.webview_instance_id)
+    return webview_instance_id < other.webview_instance_id;
+
+  return extension_id < other.extension_id;
+}
+
+bool MenuItem::ExtensionKey::operator!=(const ExtensionKey& other) const {
+  return !(*this == other);
+}
+
+bool MenuItem::ExtensionKey::empty() const {
+  return extension_id.empty() && !webview_instance_id;
+}
+
+MenuItem::Id::Id() : incognito(false), uid(0), webview_instance_id(0) {}
 
 MenuItem::Id::Id(bool incognito, const std::string& extension_id)
-    : incognito(incognito), extension_id(extension_id), uid(0) {}
+    : incognito(incognito),
+      extension_id(extension_id),

[Fady Samuel, 2014/03/04 21:14:59]

So MenuItem now has an extension_id, and a webview_instance_id. Can this simply
be an ExtensionKey?

[lazyboy, 2014/03/04 23:07:02]

Done.

+      uid(0),
+      webview_instance_id(0) {}
 
 MenuItem::Id::~Id() {
 }
 
 bool MenuItem::Id::operator==(const Id& other) const {
-  return (incognito == other.incognito &&
-          extension_id == other.extension_id &&
-          uid == other.uid &&
-          string_uid == other.string_uid);
+  return (incognito == other.incognito && extension_id == other.extension_id &&
+          uid == other.uid && string_uid == other.string_uid &&
+          webview_instance_id == other.webview_instance_id);

[Fady Samuel, 2014/03/04 21:14:59]

one less condition to check if this thing was an ExtensionKey.

[lazyboy, 2014/03/04 23:07:02]

Done.

 }
 
 bool MenuItem::Id::operator!=(const Id& other) const {
@@ -885,10 +940,15 @@ bool MenuItem::Id::operator<(const Id& other) const {
     if (extension_id < other.extension_id)
       return true;
     if (extension_id == other.extension_id) {
-      if (uid < other.uid)
+      if (webview_instance_id < other.webview_instance_id)

[Fady Samuel, 2014/03/04 21:14:59]

If this was an ExtensionKey you could do both comparisons in one line.

[lazyboy, 2014/03/04 23:07:02]

Done.

         return true;
-      if (uid == other.uid)
-        return string_uid < other.string_uid;
+
+      if (webview_instance_id == other.webview_instance_id) {
+        if (uid < other.uid)
+          return true;
+        if (uid == other.uid)
+          return string_uid < other.string_uid;
+      }
     }
   }
   return false;