Modernize some extensions code.

extensions/browser/event_router.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/event_router.h"
 
 #include <stddef.h>
 
 #include <utility>
 
 #include "base/atomic_sequence_num.h"
 #include "base/bind.h"
-#include "base/command_line.h"
-#include "base/lazy_instance.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
 #include "base/values.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "extensions/browser/api_activity_monitor.h"
 #include "extensions/browser/event_router_factory.h"
 #include "extensions/browser/extension_host.h"
@@ skipping 162 matching lines @@
 }
 
 void EventRouter::RegisterObserver(Observer* observer,
                                    const std::string& event_name) {
   // Observing sub-event names like "foo.onBar/123" is not allowed.
   DCHECK(event_name.find('/') == std::string::npos);
   observers_[event_name] = observer;
 }
 
 void EventRouter::UnregisterObserver(Observer* observer) {
   std::vector<ObserverMap::iterator> iters_to_remove;

[Devlin, 2017/05/20 00:49:16]

It seems a shame to construct a vector just for this.  How about instead:
for (ObserverMap::iterator iter = observers_.begin();
     iter != observers_.end();) {
  if (iter->second == observer)
    iter = observers_.erase(iter);
  else
    ++iter;
}
?

[Lei Zhang, 2017/05/20 01:04:14]

Sure.

I see people do it both ways and I don't have a strong pref either way. So I was
just leaving it as is.

   for (ObserverMap::iterator iter = observers_.begin();
        iter != observers_.end(); ++iter) {
     if (iter->second == observer)
       iters_to_remove.push_back(iter);
   }
-  for (size_t i = 0; i < iters_to_remove.size(); ++i)
-    observers_.erase(iters_to_remove[i]);
+  for (auto& iter : iters_to_remove)
+    observers_.erase(iter);
 }
 
 void EventRouter::OnListenerAdded(const EventListener* listener) {
   const EventListenerInfo details(listener->event_name(),
                                   listener->extension_id(),
                                   listener->listener_url(),
                                   listener->GetBrowserContext());
   std::string base_event_name = GetBaseEventName(listener->event_name());
   ObserverMap::iterator observer = observers_.find(base_event_name);
   if (observer != observers_.end())
@@ skipping 61 matching lines @@
 
 void EventRouter::AddFilteredEventListener(const std::string& event_name,
                                            content::RenderProcessHost* process,
                                            const std::string& extension_id,
                                            const base::DictionaryValue& filter,
                                            bool add_lazy_listener) {
   listeners_.AddListener(EventListener::ForExtension(
       event_name, extension_id, process,
       std::unique_ptr<DictionaryValue>(filter.DeepCopy())));
 
-  if (add_lazy_listener) {
-    bool added = listeners_.AddListener(EventListener::ForExtension(
-        event_name, extension_id, NULL,
-        std::unique_ptr<DictionaryValue>(filter.DeepCopy())));
+  if (!add_lazy_listener)
+    return;
 
-    if (added)
-      AddFilterToEvent(event_name, extension_id, &filter);
-  }
+  bool added = listeners_.AddListener(EventListener::ForExtension(
+      event_name, extension_id, nullptr,
+      std::unique_ptr<DictionaryValue>(filter.DeepCopy())));
+  if (added)
+    AddFilterToEvent(event_name, extension_id, &filter);
 }
 
 void EventRouter::RemoveFilteredEventListener(
     const std::string& event_name,
     content::RenderProcessHost* process,
     const std::string& extension_id,
     const base::DictionaryValue& filter,
     bool remove_lazy_listener) {
   std::unique_ptr<EventListener> listener = EventListener::ForExtension(
       event_name, extension_id, process,
       std::unique_ptr<DictionaryValue>(filter.DeepCopy()));
 
   listeners_.RemoveListener(listener.get());
 
   if (remove_lazy_listener) {
     listener->MakeLazy();
     bool removed = listeners_.RemoveListener(listener.get());
 
     if (removed)
       RemoveFilterFromEvent(event_name, extension_id, &filter);
   }
 }
 
-bool EventRouter::HasEventListener(const std::string& event_name) {
+bool EventRouter::HasEventListener(const std::string& event_name) const {
   return listeners_.HasListenerForEvent(event_name);
 }
 
-bool EventRouter::ExtensionHasEventListener(const std::string& extension_id,
-                                            const std::string& event_name) {
+bool EventRouter::ExtensionHasEventListener(
+    const std::string& extension_id,
+    const std::string& event_name) const {
   return listeners_.HasListenerForExtension(extension_id, event_name);
 }
 
 std::set<std::string> EventRouter::GetRegisteredEvents(
     const std::string& extension_id) const {
   std::set<std::string> events;
   const ListValue* events_value = NULL;
 
   if (!extension_prefs_ ||
       !extension_prefs_->ReadPrefAsList(
@@ skipping 236 matching lines @@
 
 bool EventRouter::MaybeLoadLazyBackgroundPageToDispatchEvent(
     BrowserContext* context,
     const Extension* extension,
     const linked_ptr<Event>& event,
     const base::DictionaryValue* listener_filter) {
   if (!CanDispatchEventToBrowserContext(context, extension, event))
     return false;
 
   LazyBackgroundTaskQueue* queue = LazyBackgroundTaskQueue::Get(context);
-  if (queue->ShouldEnqueueTask(context, extension)) {
-    linked_ptr<Event> dispatched_event(event);
+  if (!queue->ShouldEnqueueTask(context, extension))
+    return false;
 
-    // If there's a dispatch callback, call it now (rather than dispatch time)
-    // to avoid lifetime issues. Use a separate copy of the event args, so they
-    // last until the event is dispatched.
-    if (!event->will_dispatch_callback.is_null()) {
-      dispatched_event.reset(event->DeepCopy());
-      if (!dispatched_event->will_dispatch_callback.Run(
-              context, extension, dispatched_event.get(), listener_filter)) {
-        // The event has been canceled.
-        return true;
-      }
-      // Ensure we don't call it again at dispatch time.
-      dispatched_event->will_dispatch_callback.Reset();
+  linked_ptr<Event> dispatched_event(event);
+
+  // If there's a dispatch callback, call it now (rather than dispatch time)
+  // to avoid lifetime issues. Use a separate copy of the event args, so they
+  // last until the event is dispatched.
+  if (!event->will_dispatch_callback.is_null()) {
+    dispatched_event.reset(event->DeepCopy());
+    if (!dispatched_event->will_dispatch_callback.Run(
+            context, extension, dispatched_event.get(), listener_filter)) {
+      // The event has been canceled.
+      return true;
     }
-
-    queue->AddPendingTask(context, extension->id(),
-                          base::Bind(&EventRouter::DispatchPendingEvent,
-                                     base::Unretained(this), dispatched_event));
-    return true;
+    // Ensure we don't call it again at dispatch time.
+    dispatched_event->will_dispatch_callback.Reset();
   }
 
-  return false;
+  queue->AddPendingTask(context, extension->id(),
+                        base::Bind(&EventRouter::DispatchPendingEvent,
+                                   base::Unretained(this), dispatched_event));
+  return true;
 }
 
 // static
 void EventRouter::DoDispatchEventToSenderBookkeepingOnUI(
     void* browser_context_id,
     const std::string& extension_id,
     int event_id,
     events::HistogramValue histogram_value,
     const std::string& event_name) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ skipping 136 matching lines @@
     filtered_events->SetWithoutPathExpansion(event_name,
                                              base::WrapUnique(filter_list));
   }
 
   filter_list->Append(filter->CreateDeepCopy());
 }
 
 void EventRouter::Observe(int type,
                           const content::NotificationSource& source,
                           const content::NotificationDetails& details) {
-  switch (type) {
-    case extensions::NOTIFICATION_EXTENSION_ENABLED: {
-      // If the extension has a lazy background page, make sure it gets loaded
-      // to register the events the extension is interested in.
-      const Extension* extension =
-          content::Details<const Extension>(details).ptr();
-      if (BackgroundInfo::HasLazyBackgroundPage(extension)) {
-        LazyBackgroundTaskQueue* queue =
-            LazyBackgroundTaskQueue::Get(browser_context_);
-        queue->AddPendingTask(browser_context_, extension->id(),
-                              base::Bind(&DoNothing));
-      }
-      break;
-    }
-    default:
-      NOTREACHED();
+  DCHECK_EQ(extensions::NOTIFICATION_EXTENSION_ENABLED, type);
+  // If the extension has a lazy background page, make sure it gets loaded
+  // to register the events the extension is interested in.
+  const Extension* extension = content::Details<const Extension>(details).ptr();
+  if (BackgroundInfo::HasLazyBackgroundPage(extension)) {
+    LazyBackgroundTaskQueue* queue =
+        LazyBackgroundTaskQueue::Get(browser_context_);
+    queue->AddPendingTask(browser_context_, extension->id(),
+                          base::Bind(&DoNothing));
   }
 }
 
 void EventRouter::OnExtensionLoaded(content::BrowserContext* browser_context,
                                     const Extension* extension) {
   // Add all registered lazy listeners to our cache.
   std::set<std::string> registered_events =
       GetRegisteredEvents(extension->id());
   listeners_.LoadUnfilteredLazyListeners(extension->id(), registered_events);
   const DictionaryValue* filtered_events = GetFilteredEvents(extension->id());
@@ skipping 63 matching lines @@
                                      const std::string& extension_id,
                                      const GURL& listener_url,
                                      content::BrowserContext* browser_context)
     : event_name(event_name),
       extension_id(extension_id),
       listener_url(listener_url),
       browser_context(browser_context) {
 }
 
 }  // namespace extensions