Fix semantics of ExtensionWebRequestEventRouter::EventListeners.

extensions/browser/api/web_request/web_request_api.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 "extensions/browser/api/web_request/web_request_api.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 253 matching lines @@
     histogram_value = events::DECLARATIVE_WEB_REQUEST_ON_MESSAGE;
     event_name = declarative_keys::kOnMessage;
   }
 
   std::unique_ptr<Event> event(new Event(
       histogram_value, event_name, std::move(event_args), browser_context,
       GURL(), EventRouter::USER_GESTURE_UNKNOWN, event_filtering_info));
   event_router->DispatchEventToExtension(extension_id, std::move(event));
 }
 
-void RemoveEventListenerOnIOThread(
-    void* browser_context,
-    const std::string& extension_id,
-    const std::string& sub_event_name,
-    int embedder_process_id,
-    int web_view_instance_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
-      browser_context, extension_id, sub_event_name,
-      embedder_process_id, web_view_instance_id);
-}
-
 events::HistogramValue GetEventHistogramValue(const std::string& event_name) {
   // Event names will either be webRequest events, or guest view (probably web
   // view) events that map to webRequest events. Check webRequest first.
   static struct ValueAndName {
     events::HistogramValue histogram_value;
     const char* const event_name;
   } values_and_names[] = {
       {events::WEB_REQUEST_ON_BEFORE_REDIRECT, keys::kOnBeforeRedirectEvent},
       {events::WEB_REQUEST_ON_BEFORE_REQUEST,
        web_request::OnBeforeRequest::kEventName},
@@ skipping 64 matching lines @@
 }
 
 void WebRequestAPI::OnListenerRemoved(const EventListenerInfo& details) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // Note that details.event_name includes the sub-event details (e.g. "/123").
   // TODO(fsamuel): <webview> events will not be removed through this code path.
   // <webview> events will be removed in RemoveWebViewEventListeners. Ideally,
   // this code should be decoupled from extensions, we should use the host ID
   // instead, and not have two different code paths. This is a huge undertaking
   // unfortunately, so we'll resort to two code paths for now.
-  BrowserThread::PostTask(BrowserThread::IO,
-                          FROM_HERE,
-                          base::Bind(&RemoveEventListenerOnIOThread,
-                                     details.browser_context,
-                                     details.extension_id,
-                                     details.event_name,
-                                     0 /* embedder_process_id (ignored) */,
-                                     0 /* web_view_instance_id */));
+  //
+  // Note that details.event_name is actually the sub_event_name!
+  ExtensionWebRequestEventRouter::EventListener::Identifier id(
+      details.browser_context, details.extension_id, details.event_name, 0, 0);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(
+          &ExtensionWebRequestEventRouter::RemoveEventListener,
+          base::Unretained(ExtensionWebRequestEventRouter::GetInstance()), id));
 }
 
 // Represents a single unique listener to an event, along with whatever filter
 // parameters and extra_info_spec were specified at the time the listener was
 // added.
 // NOTE(benjhayden) New APIs should not use this sub_event_name trick! It does
 // not play well with event pages. See downloads.onDeterminingFilename and
 // ExtensionDownloadsEventRouter for an alternative approach.
-struct ExtensionWebRequestEventRouter::EventListener {
-  std::string extension_id;
-  std::string extension_name;
-  events::HistogramValue histogram_value;
-  std::string sub_event_name;
-  RequestFilter filter;
-  int extra_info_spec;
-  int embedder_process_id;
-  int web_view_instance_id;
-  base::WeakPtr<IPC::Sender> ipc_sender;
-  mutable std::set<uint64_t> blocked_requests;
-
-  // Comparator to work with std::set.
-  bool operator<(const EventListener& that) const {
-    if (extension_id != that.extension_id)
-      return extension_id < that.extension_id;
-
-    if (sub_event_name != that.sub_event_name)
-      return sub_event_name < that.sub_event_name;
-
-    if (web_view_instance_id != that.web_view_instance_id)
-      return web_view_instance_id < that.web_view_instance_id;
-
-    if (web_view_instance_id == 0) {
-      // Do not filter by process ID for non-webviews, because this comparator
-      // is also used to find and remove an event listener when an extension is
-      // unloaded. At this point, the event listener cannot be mapped back to
-      // the original process, so 0 is used instead of the actual process ID.
-      if (embedder_process_id == 0 || that.embedder_process_id == 0)
-        return false;
-    }
-
-    if (embedder_process_id != that.embedder_process_id)
-      return embedder_process_id < that.embedder_process_id;
-
-    return false;
-  }
-
-  EventListener()
-      : histogram_value(events::UNKNOWN),
-        extra_info_spec(0),
-        embedder_process_id(0),
-        web_view_instance_id(0) {}
-};
+ExtensionWebRequestEventRouter::EventListener::EventListener(Identifier id)
+    : id(id) {}
+ExtensionWebRequestEventRouter::EventListener::~EventListener() {}
 
 // Contains info about requests that are blocked waiting for a response from
 // an extension.
 struct ExtensionWebRequestEventRouter::BlockedRequest {
   // The request that is being blocked.
   net::URLRequest* request;
 
   // Whether the request originates from an incognito tab.
   bool is_incognito;
 
@@ skipping 183 matching lines @@
 
   // Whether to initialized |blocked_requests_|.
   bool initialize_blocked_requests = false;
 
   initialize_blocked_requests |=
       ProcessDeclarativeRules(browser_context, extension_info_map,
                               web_request::OnBeforeRequest::kEventName, request,
                               ON_BEFORE_REQUEST, NULL);
 
   int extra_info_spec = 0;
-  EventListeners listeners = GetMatchingListeners(
+  ListenerIdentifiers listeners = GetMatchingListeners(
       browser_context, extension_info_map,
       web_request::OnBeforeRequest::kEventName, request, &extra_info_spec);
   if (!listeners.empty() &&
       !GetAndSetSignaled(request->identifier(), kOnBeforeRequest)) {
     std::unique_ptr<WebRequestEventDetails> event_details(
         CreateEventDetails(request, extra_info_spec));
     event_details->SetRequestBody(request);
 
     initialize_blocked_requests |= DispatchEvent(
         browser_context, request, listeners, std::move(event_details));
@@ skipping 28 matching lines @@
   if (ShouldHideEvent(browser_context, extension_info_map, request))
     return net::OK;
 
   bool initialize_blocked_requests = false;
 
   initialize_blocked_requests |= ProcessDeclarativeRules(
       browser_context, extension_info_map, keys::kOnBeforeSendHeadersEvent,
       request, ON_BEFORE_SEND_HEADERS, NULL);
 
   int extra_info_spec = 0;
-  EventListeners listeners = GetMatchingListeners(
+  ListenerIdentifiers listeners = GetMatchingListeners(
       browser_context, extension_info_map, keys::kOnBeforeSendHeadersEvent,
       request, &extra_info_spec);
   if (!listeners.empty() &&
       !GetAndSetSignaled(request->identifier(), kOnBeforeSendHeaders)) {
     std::unique_ptr<WebRequestEventDetails> event_details(
         CreateEventDetails(request, extra_info_spec));
     event_details->SetRequestHeaders(*headers);
 
     initialize_blocked_requests |= DispatchEvent(
         browser_context, request, listeners, std::move(event_details));
@@ skipping 26 matching lines @@
     const net::HttpRequestHeaders& headers) {
   if (ShouldHideEvent(browser_context, extension_info_map, request))
     return;
 
   if (GetAndSetSignaled(request->identifier(), kOnSendHeaders))
     return;
 
   ClearSignaled(request->identifier(), kOnBeforeRedirect);
 
   int extra_info_spec = 0;
-  EventListeners listeners = GetMatchingListeners(
+  ListenerIdentifiers listeners = GetMatchingListeners(
       browser_context, extension_info_map, keys::kOnSendHeadersEvent, request,
       &extra_info_spec);
   if (listeners.empty())
     return;
 
   std::unique_ptr<WebRequestEventDetails> event_details(
       CreateEventDetails(request, extra_info_spec));
   event_details->SetRequestHeaders(headers);
 
   DispatchEvent(browser_context, request, listeners, std::move(event_details));
@@ skipping 10 matching lines @@
   if (ShouldHideEvent(browser_context, extension_info_map, request))
     return net::OK;
 
   bool initialize_blocked_requests = false;
 
   initialize_blocked_requests |= ProcessDeclarativeRules(
       browser_context, extension_info_map, keys::kOnHeadersReceivedEvent,
       request, ON_HEADERS_RECEIVED, original_response_headers);
 
   int extra_info_spec = 0;
-  EventListeners listeners = GetMatchingListeners(
+  ListenerIdentifiers listeners = GetMatchingListeners(
       browser_context, extension_info_map, keys::kOnHeadersReceivedEvent,
       request, &extra_info_spec);
 
   if (!listeners.empty() &&
       !GetAndSetSignaled(request->identifier(), kOnHeadersReceived)) {
     std::unique_ptr<WebRequestEventDetails> event_details(
         CreateEventDetails(request, extra_info_spec));
     event_details->SetResponseHeaders(request, original_response_headers);
 
     initialize_blocked_requests |= DispatchEvent(
@@ skipping 31 matching lines @@
     const net::NetworkDelegate::AuthCallback& callback,
     net::AuthCredentials* credentials) {
   // No browser_context means that this is for authentication challenges in the
   // system context. Skip in that case. Also skip sensitive requests.
   if (!browser_context ||
       WebRequestPermissions::HideRequest(extension_info_map, request)) {
     return net::NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
   }
 
   int extra_info_spec = 0;
-  EventListeners listeners = GetMatchingListeners(
+  ListenerIdentifiers listeners = GetMatchingListeners(
       browser_context, extension_info_map, keys::kOnAuthRequiredEvent, request,
       &extra_info_spec);
   if (listeners.empty())
     return net::NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
 
   std::unique_ptr<WebRequestEventDetails> event_details(
       CreateEventDetails(request, extra_info_spec));
   event_details->SetResponseHeaders(request, request->response_headers());
   event_details->SetAuthInfo(auth_info);
 
@@ skipping 21 matching lines @@
 
   if (GetAndSetSignaled(request->identifier(), kOnBeforeRedirect))
     return;
 
   ClearSignaled(request->identifier(), kOnBeforeRequest);
   ClearSignaled(request->identifier(), kOnBeforeSendHeaders);
   ClearSignaled(request->identifier(), kOnSendHeaders);
   ClearSignaled(request->identifier(), kOnHeadersReceived);
 
   int extra_info_spec = 0;
-  EventListeners listeners = GetMatchingListeners(
+  ListenerIdentifiers listeners = GetMatchingListeners(
       browser_context, extension_info_map, keys::kOnBeforeRedirectEvent,
       request, &extra_info_spec);
   if (listeners.empty())
     return;
 
   std::unique_ptr<WebRequestEventDetails> event_details(
       CreateEventDetails(request, extra_info_spec));
   event_details->SetResponseHeaders(request, request->response_headers());
   event_details->SetResponseSource(request);
   event_details->SetString(keys::kRedirectUrlKey, new_location.spec());
 
   DispatchEvent(browser_context, request, listeners, std::move(event_details));
 }
 
 void ExtensionWebRequestEventRouter::OnResponseStarted(
     void* browser_context,
     const InfoMap* extension_info_map,
     net::URLRequest* request,
     int net_error) {
   DCHECK_NE(net::ERR_IO_PENDING, net_error);
 
   if (ShouldHideEvent(browser_context, extension_info_map, request))
     return;
 
   // OnResponseStarted is even triggered, when the request was cancelled.
   if (net_error != net::OK)
     return;
 
   int extra_info_spec = 0;
-  EventListeners listeners = GetMatchingListeners(
+  ListenerIdentifiers listeners = GetMatchingListeners(
       browser_context, extension_info_map, keys::kOnResponseStartedEvent,
       request, &extra_info_spec);
   if (listeners.empty())
     return;
 
   std::unique_ptr<WebRequestEventDetails> event_details(
       CreateEventDetails(request, extra_info_spec));
   event_details->SetResponseHeaders(request, request->response_headers());
   event_details->SetResponseSource(request);
 
@@ skipping 28 matching lines @@
   request_time_tracker_->LogRequestEndTime(request->identifier(),
                                            base::Time::Now());
 
   DCHECK_EQ(net::OK, net_error);
 
   DCHECK(!GetAndSetSignaled(request->identifier(), kOnCompleted));
 
   ClearPendingCallbacks(request);
 
   int extra_info_spec = 0;
-  EventListeners listeners =
+  ListenerIdentifiers listeners =
       GetMatchingListeners(browser_context, extension_info_map,
                            keys::kOnCompletedEvent, request, &extra_info_spec);
   if (listeners.empty())
     return;
 
   std::unique_ptr<WebRequestEventDetails> event_details(
       CreateEventDetails(request, extra_info_spec));
   event_details->SetResponseHeaders(request, request->response_headers());
   event_details->SetResponseSource(request);
 
@@ skipping 30 matching lines @@
                                            base::Time::Now());
 
   DCHECK_NE(net::OK, net_error);
   DCHECK_NE(net::ERR_IO_PENDING, net_error);
 
   DCHECK(!GetAndSetSignaled(request->identifier(), kOnErrorOccurred));
 
   ClearPendingCallbacks(request);
 
   int extra_info_spec = 0;
-  EventListeners listeners = GetMatchingListeners(
+  ListenerIdentifiers listeners = GetMatchingListeners(
       browser_context, extension_info_map,
       web_request::OnErrorOccurred::kEventName, request, &extra_info_spec);
   if (listeners.empty())
     return;
 
   std::unique_ptr<WebRequestEventDetails> event_details(
       CreateEventDetails(request, extra_info_spec));
   if (started)
     event_details->SetResponseSource(request);
   else
@@ skipping 24 matching lines @@
 }
 
 void ExtensionWebRequestEventRouter::ClearPendingCallbacks(
     const net::URLRequest* request) {
   blocked_requests_.erase(request->identifier());
 }
 
 bool ExtensionWebRequestEventRouter::DispatchEvent(
     void* browser_context,
     net::URLRequest* request,
-    const std::vector<const EventListener*>& listeners,
+    const ListenerIdentifiers& listeners,
     std::unique_ptr<WebRequestEventDetails> event_details) {
   // TODO(mpcomplete): Consider consolidating common (extension_id,json_args)
   // pairs into a single message sent to a list of sub_event_names.
   int num_handlers_blocking = 0;
 
-  std::unique_ptr<std::vector<EventListener>> listeners_to_dispatch(
-      new std::vector<EventListener>());
-  listeners_to_dispatch->reserve(listeners.size());
-  for (const EventListener* listener : listeners) {
-    listeners_to_dispatch->push_back(*listener);
+  for (const EventListener::Identifier& id : listeners) {
+    EventListener* listener = FindEventListener(id);

[Devlin, 2016/09/07 19:18:00]

This makes me a bit sad, because it means we have to look up the listener again
(and this code needs to be as fast as possible).  What if we instead keep
GetMatchingListeners returning a std::vector<EventListener*>, as it does now,
but then construct the ids in this function for when we dispatch them?  So just
adding:
std::unique_ptr<ListenerIdentifiers> listener_ids(new ListenerIdentifiers()); 
// Note std::unique_ptr for reason below
listener_ids->reserve(listeners.size());
for (listener : listeners) {
  listener_ids->push_back(listener->id());
  ...
}

Would that work?

[erikchen, 2016/09/07 22:07:42]

yup, done.

     if (listener->extra_info_spec &
         (ExtraInfoSpec::BLOCKING | ExtraInfoSpec::ASYNC_BLOCKING)) {
       listener->blocked_requests.insert(request->identifier());
       // If this is the first delegate blocking the request, go ahead and log
       // it.
       if (num_handlers_blocking == 0) {
         std::string delegate_info = l10n_util::GetStringFUTF8(
             IDS_LOAD_STATE_PARAMETER_EXTENSION,
             base::UTF8ToUTF16(listener->extension_name));
         // LobAndReport allows extensions that block requests to be displayed in
         // the load status bar.
         request->LogAndReportBlockedBy(delegate_info.c_str());
       }
       ++num_handlers_blocking;
     }
   }
 
-  event_details.release()->DetermineFrameDataOnIO(base::Bind(
-      &ExtensionWebRequestEventRouter::DispatchEventToListeners, AsWeakPtr(),
-      browser_context, base::Passed(&listeners_to_dispatch)));
+  event_details.release()->DetermineFrameDataOnIO(
+      base::Bind(&ExtensionWebRequestEventRouter::DispatchEventToListeners,
+                 AsWeakPtr(), browser_context, listeners));

[Devlin, 2016/09/07 19:18:00]

Optimization: Shame about the copy this causes on listeners, even if it is
cheaper.  Why not pass ownership?  It's pretty trivial if we can do the comment
on 984, but even if not, it looks like we could pass ownership of |listeners| to
DispatchEvent() and then to this closure so that we don't have to perform any
copies.  We just need to return a unique_ptr from GetMatchingListeners().  WDYT?

[erikchen, 2016/09/07 22:07:42]

Done.

 
   if (num_handlers_blocking > 0) {
     BlockedRequest& blocked_request = blocked_requests_[request->identifier()];
     blocked_request.request = request;
     blocked_request.is_incognito |= IsIncognitoBrowserContext(browser_context);
     blocked_request.num_handlers_blocking += num_handlers_blocking;
     blocked_request.blocking_time = base::Time::Now();
     return true;
   }
 
   return false;
 }
 
 void ExtensionWebRequestEventRouter::DispatchEventToListeners(
     void* browser_context,
-    std::unique_ptr<std::vector<EventListener>> listeners,
+    const ListenerIdentifiers& listener_ids,
     std::unique_ptr<WebRequestEventDetails> event_details) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(listeners.get());
-  DCHECK_GT(listeners->size(), 0UL);
+  DCHECK_GT(listener_ids.size(), 0UL);

[Devlin, 2016/09/07 19:18:00]

nit: DCHECK(!listener_ids.empty())

[erikchen, 2016/09/07 22:07:42]

Done.

   DCHECK(event_details.get());
 
   std::string event_name =
-      EventRouter::GetBaseEventName((*listeners)[0].sub_event_name);
+      EventRouter::GetBaseEventName(listener_ids[0].sub_event_name);
   DCHECK(IsWebRequestEvent(event_name));
 
-  const std::set<EventListener>& event_listeners =
-      listeners_[browser_context][event_name];
+  const Listeners& event_listeners = listeners_[browser_context][event_name];
   void* cross_browser_context = GetCrossBrowserContext(browser_context);
-  const std::set<EventListener>* cross_event_listeners =
+  const Listeners* cross_event_listeners =
       cross_browser_context ? &listeners_[cross_browser_context][event_name]
                             : nullptr;
 
-  for (const EventListener& target : *listeners) {
-    std::set<EventListener>::const_iterator listener =
-        event_listeners.find(target);
-    // Ignore listener if it was removed between the thread hops.
-    if (listener == event_listeners.end()) {
-      if (!cross_event_listeners)
-        continue;
-      listener = cross_event_listeners->find(target);
-      if (listener == cross_event_listeners->end())
-        continue;
+  for (const EventListener::Identifier& id : listener_ids) {
+    // It's possible that the listener is no longer present. Check to make sure
+    // it's still there.
+    const EventListener* listener = nullptr;
+    for (const std::unique_ptr<EventListener>& event_listener :

[Devlin, 2016/09/07 19:18:00]

Worth making a FindEventListenerInSet() helper function that can be used here,
line 1048, and in FindEventListener()?

[erikchen, 2016/09/07 22:07:42]

yes, done.

+         event_listeners) {
+      if (event_listener->id == id) {
+        listener = event_listener.get();
+        break;
+      }
     }
+    if (!listener) {
+      for (const std::unique_ptr<EventListener>& event_listener :
+           *cross_event_listeners) {
+        if (event_listener->id == id) {
+          listener = event_listener.get();
+          break;
+        }
+      }
+    }
+    if (!listener)
+      continue;
 
     if (!listener->ipc_sender.get())
       continue;
 
     // Filter out the optional keys that this listener didn't request.
     std::unique_ptr<base::ListValue> args_filtered(new base::ListValue);
     args_filtered->Append(
         event_details->GetFilteredDict(listener->extra_info_spec));
 
     EventRouter::DispatchEventToSender(
-        listener->ipc_sender.get(), browser_context, listener->extension_id,
-        listener->histogram_value, listener->sub_event_name,
+        listener->ipc_sender.get(), browser_context, listener->id.extension_id,
+        listener->histogram_value, listener->id.sub_event_name,
         std::move(args_filtered), EventRouter::USER_GESTURE_UNKNOWN,
         EventFilteringInfo());
   }
 }
 
 void ExtensionWebRequestEventRouter::OnEventHandled(
     void* browser_context,
     const std::string& extension_id,
     const std::string& event_name,
     const std::string& sub_event_name,
     uint64_t request_id,
     EventResponse* response) {
-  // TODO(robwu): Does this also work with webviews? operator< (used by find)
-  // takes the webview ID into account, which is not set on |listener|.
-  EventListener listener;
-  listener.extension_id = extension_id;
-  listener.sub_event_name = sub_event_name;
-
-  // The listener may have been removed (e.g. due to the process going away)
-  // before we got here.
-  std::set<EventListener>::iterator found =
-      listeners_[browser_context][event_name].find(listener);
-  if (found != listeners_[browser_context][event_name].end())
-    found->blocked_requests.erase(request_id);
+  // TODO(robwu): This ignores WebViews.
+  Listeners& listeners = listeners_[browser_context][event_name];
+  EventListener::Identifier id(browser_context, extension_id, sub_event_name, 0,
+                               0);
+  for (auto it = listeners.begin(); it != listeners.end(); ++it) {
+    if ((*it)->id.LooselyMatches(id)) {
+      (*it)->blocked_requests.erase(request_id);
+    }
+  }
 
   DecrementBlockCount(
       browser_context, extension_id, event_name, request_id, response);
 }
 
 bool ExtensionWebRequestEventRouter::AddEventListener(
     void* browser_context,
     const std::string& extension_id,
     const std::string& extension_name,
     events::HistogramValue histogram_value,
     const std::string& event_name,
     const std::string& sub_event_name,
     const RequestFilter& filter,
     int extra_info_spec,
     int embedder_process_id,
     int web_view_instance_id,
     base::WeakPtr<IPC::Sender> ipc_sender) {
   if (!IsWebRequestEvent(event_name))
     return false;
 
-  EventListener listener;
-  listener.extension_id = extension_id;
-  listener.extension_name = extension_name;
-  listener.histogram_value = histogram_value;
-  listener.sub_event_name = sub_event_name;
-  listener.filter = filter;
-  listener.extra_info_spec = extra_info_spec;
-  listener.ipc_sender = ipc_sender;
-  listener.embedder_process_id = embedder_process_id;
-  listener.web_view_instance_id = web_view_instance_id;
-  if (listener.web_view_instance_id) {
+  if (event_name != EventRouter::GetBaseEventName(sub_event_name))
+    return false;
+
+  EventListener::Identifier id(browser_context, extension_id, sub_event_name,
+                               embedder_process_id, web_view_instance_id);
+  std::unique_ptr<EventListener> listener(new EventListener(id));
+  listener->extension_name = extension_name;
+  listener->histogram_value = histogram_value;
+  listener->filter = filter;
+  listener->extra_info_spec = extra_info_spec;
+  listener->ipc_sender = ipc_sender;
+  if (listener->id.web_view_instance_id) {
     content::RecordAction(
         base::UserMetricsAction("WebView.WebRequest.AddListener"));
   }
 
-  if (base::ContainsKey(listeners_[browser_context][event_name], listener)) {
+  if (FindEventListener(id) != nullptr) {
     // This is likely an abuse of the API by a malicious extension.
     return false;
   }
-  listeners_[browser_context][event_name].insert(listener);
+  listeners_[browser_context][event_name].push_back(std::move(listener));
   return true;
 }
 
+size_t ExtensionWebRequestEventRouter::GetListenerCountForTesting(
+    void* browser_context,
+    const std::string& event_name) {
+  return listeners_[browser_context][event_name].size();
+}
+
+ExtensionWebRequestEventRouter::EventListener*
+ExtensionWebRequestEventRouter::FindEventListener(
+    const EventListener::Identifier& id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  std::string event_name = EventRouter::GetBaseEventName(id.sub_event_name);
+  Listeners& listeners = listeners_[id.browser_context][event_name];
+  for (auto it = listeners.begin(); it != listeners.end(); ++it) {
+    if ((*it)->id == id) {
+      return it->get();
+    }
+  }
+  return nullptr;
+}
+
 void ExtensionWebRequestEventRouter::RemoveEventListener(
-    void* browser_context,
-    const std::string& extension_id,
-    const std::string& sub_event_name,
-    int embedder_process_id,
-    int web_view_instance_id) {
-  std::string event_name = EventRouter::GetBaseEventName(sub_event_name);
-  DCHECK(IsWebRequestEvent(event_name));
+    const EventListener::Identifier& id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  EventListener listener;
-  listener.extension_id = extension_id;
-  listener.sub_event_name = sub_event_name;
-  listener.embedder_process_id = embedder_process_id;
-  listener.web_view_instance_id = web_view_instance_id;
+  std::string event_name = EventRouter::GetBaseEventName(id.sub_event_name);
+  Listeners& listeners = listeners_[id.browser_context][event_name];
+  for (auto it = listeners.begin(); it != listeners.end(); ++it) {
+    std::unique_ptr<EventListener>& listener = *it;
+    if (listener->id.LooselyMatches(id)) {
+      // Unblock any request that this event listener may have been blocking.
+      for (uint64_t blocked_request_id : listener->blocked_requests)
+        DecrementBlockCount(listener->id.browser_context,
+                            listener->id.extension_id, event_name,
+                            blocked_request_id, NULL);
 
-  std::set<EventListener>& event_listeners =
-      listeners_[browser_context][event_name];
-  // It's possible for AddEventListener to fail asynchronously. In that case,
-  // the renderer believes the listener exists, while the browser does not.
-  // Ignore a RemoveEventListener in that case.
-  std::set<EventListener>::const_iterator it = event_listeners.find(listener);
-  if (it == event_listeners.end())
-    return;
-
-#if defined(OS_WIN)
-  // Debugging https://crbug.com/589735
-  // Please post crash reports at the following lines to the above issue.
-  unsigned event_listener_count = event_listeners.count(listener);
-  CHECK_GE(event_listener_count, 0u);
-  CHECK_GE(event_listener_count, 1u);
-  CHECK_LE(event_listener_count, 2u);
-  CHECK_EQ(event_listener_count, 1u);
-  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
-#endif  // OS_WIN
-  CHECK_EQ(event_listeners.count(listener), 1u) <<
-      "extension=" << extension_id << " event=" << event_name;
-
-  // Unblock any request that this event listener may have been blocking.
-  for (uint64_t id : it->blocked_requests)
-    DecrementBlockCount(browser_context, extension_id, event_name, id, NULL);
-
-  event_listeners.erase(listener);
-
-  helpers::ClearCacheOnNavigation();
+      listeners.erase(it);
+      helpers::ClearCacheOnNavigation();
+      return;
+    }
+  }
 }
 
 void ExtensionWebRequestEventRouter::RemoveWebViewEventListeners(
     void* browser_context,
     int embedder_process_id,
     int web_view_instance_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // Iterate over all listeners of all WebRequest events to delete
   // any listeners that belong to the provided <webview>.
   ListenerMapForBrowserContext& map_for_browser_context =
       listeners_[browser_context];
   for (const auto& event_iter : map_for_browser_context) {
     // Construct a listeners_to_delete vector so that we don't modify the set of
     // listeners as we iterate through it.
-    std::vector<EventListener> listeners_to_delete;
-    const std::set<EventListener>& listeners = event_iter.second;
+    std::vector<EventListener::Identifier> listeners_to_delete;
+    const Listeners& listeners = event_iter.second;
     for (const auto& listener : listeners) {
-      if (listener.embedder_process_id == embedder_process_id &&
-          listener.web_view_instance_id == web_view_instance_id) {
-        listeners_to_delete.push_back(listener);
+      if (listener->id.embedder_process_id == embedder_process_id &&
+          listener->id.web_view_instance_id == web_view_instance_id) {
+        listeners_to_delete.push_back(listener->id);
       }
     }
     // Remove the listeners selected for deletion.
-    for (const auto& listener : listeners_to_delete) {
-      RemoveEventListenerOnIOThread(
-          browser_context,
-          listener.extension_id,
-          listener.sub_event_name,
-          listener.embedder_process_id,
-          listener.web_view_instance_id);
-    }
+    for (const auto& listener_id : listeners_to_delete)
+      RemoveEventListener(listener_id);
   }
 }
 
 void ExtensionWebRequestEventRouter::OnOTRBrowserContextCreated(
     void* original_browser_context, void* otr_browser_context) {
   cross_browser_context_map_[original_browser_context] =
       std::make_pair(false, otr_browser_context);
   cross_browser_context_map_[otr_browser_context] =
       std::make_pair(true, original_browser_context);
 }
@@ skipping 55 matching lines @@
     const InfoMap* extension_info_map,
     bool crosses_incognito,
     const std::string& event_name,
     const GURL& url,
     int render_process_host_id,
     int routing_id,
     content::ResourceType resource_type,
     bool is_async_request,
     bool is_request_from_extension,
     int* extra_info_spec,
-    EventListeners* matching_listeners) {
+    ListenerIdentifiers* matching_listeners) {
   std::string web_request_event_name(event_name);
   WebViewRendererState::WebViewInfo web_view_info;
   bool is_web_view_guest = WebViewRendererState::GetInstance()->GetInfo(
       render_process_host_id, routing_id, &web_view_info);
   if (is_web_view_guest) {
     web_request_event_name.replace(
         0, sizeof(kWebRequestEventPrefix) - 1, webview::kWebViewEventPrefix);
   }
 
-  std::set<EventListener>& listeners =
-      listeners_[browser_context][web_request_event_name];
-  for (const EventListener& listener : listeners) {
-    if (!listener.ipc_sender.get()) {
+  Listeners& listeners = listeners_[browser_context][web_request_event_name];
+  for (const std::unique_ptr<EventListener>& listener : listeners) {
+    if (!listener->ipc_sender.get()) {
       // The IPC sender has been deleted. This listener will be removed soon
       // via a call to RemoveEventListener. For now, just skip it.
       continue;
     }
 
     if (is_web_view_guest &&
-        (listener.embedder_process_id != web_view_info.embedder_process_id ||
-         listener.web_view_instance_id != web_view_info.instance_id)) {
+        (listener->id.embedder_process_id !=
+             web_view_info.embedder_process_id ||
+         listener->id.web_view_instance_id != web_view_info.instance_id)) {
       continue;
     }
 
     // Filter requests from other extensions / apps. This does not work for
     // content scripts, or extension pages in non-extension processes.
     if (is_request_from_extension &&
-        listener.embedder_process_id != render_process_host_id) {
+        listener->id.embedder_process_id != render_process_host_id) {
       continue;
     }
 
-    if (!listener.filter.urls.is_empty() &&
-        !listener.filter.urls.MatchesURL(url)) {
+    if (!listener->filter.urls.is_empty() &&
+        !listener->filter.urls.MatchesURL(url)) {
       continue;
     }
 
     int render_process_id = -1;
     int render_frame_id = -1;
     // TODO(devlin): Figure out when one/both of these can fail, and if we
     // need to address it.
     bool found_render_frame =
         content::ResourceRequestInfo::GetRenderFrameForRequest(
             request, &render_process_id, &render_frame_id);
     UMA_HISTOGRAM_BOOLEAN("Extensions.WebRequestEventFoundFrame",
                           found_render_frame);
     ExtensionApiFrameIdMap::FrameData frame_data;
     if (found_render_frame) {
       ExtensionApiFrameIdMap::Get()->GetCachedFrameDataOnIO(
           render_process_id, render_frame_id, &frame_data);
     }
     // Check if the tab id and window id match, if they were set in the
     // listener params.
-    if ((listener.filter.tab_id != -1 &&
-         frame_data.tab_id != listener.filter.tab_id) ||
-        (listener.filter.window_id != -1 &&
-         frame_data.window_id != listener.filter.window_id)) {
+    if ((listener->filter.tab_id != -1 &&
+         frame_data.tab_id != listener->filter.tab_id) ||
+        (listener->filter.window_id != -1 &&
+         frame_data.window_id != listener->filter.window_id)) {
       continue;
     }
 
-    const std::vector<content::ResourceType>& types = listener.filter.types;
+    const std::vector<content::ResourceType>& types = listener->filter.types;
     if (!types.empty() &&
         std::find(types.begin(), types.end(), resource_type) == types.end()) {
       continue;
     }
 
     if (!is_web_view_guest) {
       PermissionsData::AccessType access =
           WebRequestPermissions::CanExtensionAccessURL(
-              extension_info_map, listener.extension_id, url, frame_data.tab_id,
-              crosses_incognito,
+              extension_info_map, listener->id.extension_id, url,
+              frame_data.tab_id, crosses_incognito,
               WebRequestPermissions::REQUIRE_HOST_PERMISSION);
       if (access != PermissionsData::ACCESS_ALLOWED) {
         if (access == PermissionsData::ACCESS_WITHHELD &&
             web_request_event_router_delegate_) {
           web_request_event_router_delegate_->NotifyWebRequestWithheld(
-              render_process_id, render_frame_id, listener.extension_id);
+              render_process_id, render_frame_id, listener->id.extension_id);
         }
         continue;
       }
     }
 
     bool blocking_listener =
-        (listener.extra_info_spec &
+        (listener->extra_info_spec &
          (ExtraInfoSpec::BLOCKING | ExtraInfoSpec::ASYNC_BLOCKING)) != 0;
 
     // We do not want to notify extensions about XHR requests that are
     // triggered by themselves. This is a workaround to prevent deadlocks
     // in case of synchronous XHR requests that block the extension renderer
     // and therefore prevent the extension from processing the request
     // handler. This is only a problem for blocking listeners.
     // http://crbug.com/105656
     bool synchronous_xhr_from_extension =
         !is_async_request && is_request_from_extension &&
         resource_type == content::RESOURCE_TYPE_XHR;
 
     // Only send webRequest events for URLs the extension has access to.
     if (blocking_listener && synchronous_xhr_from_extension)
       continue;
 
-    matching_listeners->push_back(&listener);
-    *extra_info_spec |= listener.extra_info_spec;
+    matching_listeners->push_back(listener->id);
+    *extra_info_spec |= listener->extra_info_spec;
   }
 }
 
-ExtensionWebRequestEventRouter::EventListeners
+ExtensionWebRequestEventRouter::ListenerIdentifiers
 ExtensionWebRequestEventRouter::GetMatchingListeners(
     void* browser_context,
     const InfoMap* extension_info_map,
     const std::string& event_name,
     const net::URLRequest* request,
     int* extra_info_spec) {
   // TODO(mpcomplete): handle browser_context == NULL (should collect all
   // listeners).
   *extra_info_spec = 0;
 
   const GURL& url = request->url();
   int render_process_host_id = content::ChildProcessHost::kInvalidUniqueID;
   int routing_id = MSG_ROUTING_NONE;
   content::ResourceType resource_type = content::RESOURCE_TYPE_LAST_TYPE;
   // We are conservative here and assume requests are asynchronous in case
   // we don't have an info object. We don't want to risk a deadlock.
   bool is_async_request = false;
   bool is_request_from_extension =
       IsRequestFromExtension(request, extension_info_map);
 
   const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request);
   if (info) {
     is_async_request = info->IsAsync();
     if (helpers::IsRelevantResourceType(info->GetResourceType()))
       resource_type = info->GetResourceType();
     render_process_host_id = info->GetChildID();
     routing_id = info->GetRouteID();
   }
 
-  EventListeners matching_listeners;
+  ListenerIdentifiers matching_listeners;
   GetMatchingListenersImpl(
       browser_context, request, extension_info_map, false, event_name,
       url, render_process_host_id, routing_id, resource_type,
       is_async_request, is_request_from_extension, extra_info_spec,
       &matching_listeners);
   void* cross_browser_context = GetCrossBrowserContext(browser_context);
   if (cross_browser_context) {
     GetMatchingListenersImpl(
         cross_browser_context, request, extension_info_map, true, event_name,
         url, render_process_host_id, routing_id, resource_type,
@@ skipping 198 matching lines @@
     // declarative rules to be read from disk.
     UMA_HISTOGRAM_TIMES("Extensions.NetworkDelayStartup", block_time);
   }
 
   if (num_handlers_blocking == 0) {
     blocked_request.request->LogUnblocked();
     ExecuteDeltas(browser_context, request_id, true);
   } else {
     // Update the URLRequest to make sure it's tagged with an extension that's
     // still blocking it.  This may end up being the same extension as before.
-    std::set<EventListener>& listeners =
-        listeners_[browser_context][event_name];
+    Listeners& listeners = listeners_[browser_context][event_name];
 
     for (const auto& listener : listeners) {
-      if (!base::ContainsKey(listener.blocked_requests, request_id))
+      if (!base::ContainsKey(listener->blocked_requests, request_id))
         continue;
-      std::string delegate_info =
-          l10n_util::GetStringFUTF8(IDS_LOAD_STATE_PARAMETER_EXTENSION,
-                                    base::UTF8ToUTF16(listener.extension_name));
+      std::string delegate_info = l10n_util::GetStringFUTF8(
+          IDS_LOAD_STATE_PARAMETER_EXTENSION,
+          base::UTF8ToUTF16(listener->extension_name));
       blocked_request.request->LogAndReportBlockedBy(delegate_info.c_str());
       break;
     }
   }
 }
 
 void ExtensionWebRequestEventRouter::SendMessages(
     void* browser_context,
     const BlockedRequest& blocked_request) {
   const helpers::EventResponseDeltas& deltas = blocked_request.response_deltas;
@@ skipping 578 matching lines @@
   RunWithValidation()->Execute();
 }
 
 ExtensionFunction::ResponseAction
 WebRequestHandlerBehaviorChangedFunction::Run() {
   helpers::ClearCacheOnNavigation();
   return RespondNow(NoArguments());
 }
 
 }  // namespace extensions