WebView context menu cleanup.

chrome/browser/extensions/menu_manager.cc

 // 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.
 
 #include "chrome/browser/extensions/menu_manager.h"
 
 #include <algorithm>
 
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/menu_manager_factory.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/chrome_web_view_internal.h"
 #include "chrome/common/extensions/api/context_menus.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/child_process_host.h"
 #include "content/public/common/context_menu_params.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/guest_view/web_view/web_view_guest.h"
 #include "extensions/browser/state_store.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_handlers/background_info.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/text_elider.h"
 
+using content::ChildProcessHost;
 using content::WebContents;
+using guest_view::kInstanceIDNone;
 
 namespace extensions {
 
 namespace {
 
 // Keys for serialization to and from Value to store in the preferences.
 const char kContextMenusKey[] = "context_menus";
 
 const char kCheckedKey[] = "checked";
 const char kContextsKey[] = "contexts";
@@ skipping 287 matching lines @@
     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 MenuItem::ExtensionKey& key = item->id().extension_key;
-  // The item must have a non-empty extension id, and not have already been
-  // added.
+
+  // The item must have a non-empty key, and not have already been added.
   if (key.empty() || ContainsKey(items_by_id_, item->id()))
     return false;
 
   DCHECK_EQ(extension->id(), key.extension_id);
 
   bool first_item = !ContainsKey(context_items_, key);
   context_items_[key].push_back(item);
   items_by_id_[item->id()] = item;
 
   if (item->type() == MenuItem::RADIO) {
@@ skipping 150 matching lines @@
 
   if (list.empty()) {
     context_items_.erase(extension_key);
     icon_manager_.RemoveIcon(extension_key.extension_id);
   }
   return result;
 }
 
 void MenuManager::RemoveAllContextItems(
     const MenuItem::ExtensionKey& extension_key) {
+  auto it = context_items_.find(extension_key);
+  if (it == context_items_.end())
+    return;
+
+  // We use the |extension_id| from the stored ExtensionKey, since the provided
+  // |extension_key| may leave it empty (if matching solely basted on the
+  // webview IDs).

[Devlin, 2015/07/08 21:10:01]

// TODO(paulmeyer): We can get rid of this hack if/when we track extension ids
at guest cleanup.

Just want to make it clear that, while not that bad, this still isn't ideal. :)

[paulmeyer, 2015/07/09 14:14:58]

Done.

+  std::string extension_id = it->first.extension_id;
+  MenuItem::List& context_items_for_key = it->second;
   MenuItem::List::iterator i;
-  for (i = context_items_[extension_key].begin();
-       i != context_items_[extension_key].end();
+  for (i = context_items_for_key.begin();
+       i != context_items_for_key.end();
        ++i) {
     MenuItem* item = *i;
     items_by_id_.erase(item->id());
 
     // Remove descendants from this item and erase them from the lookup cache.
     std::set<MenuItem::Id> removed_ids = item->RemoveAllDescendants();
     std::set<MenuItem::Id>::const_iterator j;
     for (j = removed_ids.begin(); j != removed_ids.end(); ++j) {
       items_by_id_.erase(*j);
     }
   }
-  STLDeleteElements(&context_items_[extension_key]);
+  STLDeleteElements(&context_items_for_key);
   context_items_.erase(extension_key);
-  icon_manager_.RemoveIcon(extension_key.extension_id);
+  icon_manager_.RemoveIcon(extension_id);
 }
 
 MenuItem* MenuManager::GetItemById(const MenuItem::Id& id) const {
   std::map<MenuItem::Id, MenuItem*>::const_iterator i =
       items_by_id_.find(id);
   if (i != items_by_id_.end())
     return i->second;
   else
     return NULL;
 }
@@ skipping 336 matching lines @@
       items_to_remove.insert(iter->first);
   }
 
   std::set<MenuItem::Id>::iterator remove_iter;
   for (remove_iter = items_to_remove.begin();
        remove_iter != items_to_remove.end();
        ++remove_iter)
     RemoveContextMenuItem(*remove_iter);
 }
 
-MenuItem::ExtensionKey::ExtensionKey() : webview_instance_id(0) {}
+MenuItem::ExtensionKey::ExtensionKey()
+    : webview_embedder_process_id(ChildProcessHost::kInvalidUniqueID),
+      webview_instance_id(kInstanceIDNone) {}
+
+MenuItem::ExtensionKey::ExtensionKey(const std::string& extension_id)
+    : extension_id(extension_id),
+      webview_embedder_process_id(ChildProcessHost::kInvalidUniqueID),
+      webview_instance_id(kInstanceIDNone) {
+  DCHECK(!extension_id.empty());
+}
 
 MenuItem::ExtensionKey::ExtensionKey(const std::string& extension_id,
+                                     int webview_embedder_process_id,
                                      int webview_instance_id)
-    : extension_id(extension_id), webview_instance_id(webview_instance_id) {}
-
-MenuItem::ExtensionKey::ExtensionKey(const std::string& extension_id)
-    : extension_id(extension_id), webview_instance_id(0) {}
+    : extension_id(extension_id),
+      webview_embedder_process_id(webview_embedder_process_id),
+      webview_instance_id(webview_instance_id) {
+  DCHECK(webview_embedder_process_id != ChildProcessHost::kInvalidUniqueID &&
+         webview_instance_id != kInstanceIDNone);
+}
 
 bool MenuItem::ExtensionKey::operator==(const ExtensionKey& other) const {
-  return extension_id == other.extension_id &&
-         webview_instance_id == other.webview_instance_id;
+  bool webview_ids_match = webview_instance_id == other.webview_instance_id &&
+      webview_embedder_process_id == other.webview_embedder_process_id;
+
+  // If either extension ID is empty, then these ExtensionKeys will be matched
+  // only based on the other IDs.
+  if (extension_id.empty() || other.extension_id.empty())
+    return webview_ids_match;
+
+  return extension_id == other.extension_id && webview_ids_match;
 }
 
 bool MenuItem::ExtensionKey::operator<(const ExtensionKey& other) const {
-  if (extension_id != other.extension_id)
-    return extension_id < other.extension_id;
+  if (webview_embedder_process_id != other.webview_embedder_process_id)
+    return webview_embedder_process_id < other.webview_embedder_process_id;
 
-  return webview_instance_id < other.webview_instance_id;
+  if (webview_instance_id != other.webview_instance_id)
+    return webview_instance_id < other.webview_instance_id;
+
+  // If either extension ID is empty, then these ExtensionKeys will be compared
+  // only based on the other IDs.
+  if (extension_id.empty() || other.extension_id.empty())
+    return false;
+
+  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;
+  return extension_id.empty() &&
+      webview_embedder_process_id == ChildProcessHost::kInvalidUniqueID &&
+      webview_instance_id == kInstanceIDNone;
 }
 
 MenuItem::Id::Id() : incognito(false), uid(0) {}
 
 MenuItem::Id::Id(bool incognito, const MenuItem::ExtensionKey& extension_key)
     : incognito(incognito), extension_key(extension_key), uid(0) {}
 
 MenuItem::Id::~Id() {
 }
 
@@ skipping 17 matching lines @@
       if (uid < other.uid)
         return true;
       if (uid == other.uid)
         return string_uid < other.string_uid;
     }
   }
   return false;
 }
 
 }  // namespace extensions