Modernize some extensions code.

extensions/browser/event_listener_map.cc

Index: extensions/browser/event_listener_map.cc
diff --git a/extensions/browser/event_listener_map.cc b/extensions/browser/event_listener_map.cc
index 12dd5b4499da1f744e97585ba8f6ce54251d50d4..8905bb620142ddfb1714200644147aabf8e4c8bb 100644
--- a/extensions/browser/event_listener_map.cc
+++ b/extensions/browser/event_listener_map.cc
@@ -63,10 +63,10 @@ bool EventListener::Equals(const EventListener* other) const {
 std::unique_ptr<EventListener> EventListener::Copy() const {
   std::unique_ptr<DictionaryValue> filter_copy;
   if (filter_)
-    filter_copy.reset(filter_->DeepCopy());
-  return std::unique_ptr<EventListener>(
-      new EventListener(event_name_, extension_id_, listener_url_, process_,
-                        std::move(filter_copy)));
+    filter_copy = filter_->CreateDeepCopy();
+  return base::WrapUnique(new EventListener(event_name_, extension_id_,

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

why not MakeUnique?

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

Private ctor.

+                                            listener_url_, process_,
+                                            std::move(filter_copy)));
 }
 
 bool EventListener::IsLazy() const {
@@ -121,18 +121,17 @@ bool EventListenerMap::AddListener(std::unique_ptr<EventListener> listener) {
 
 std::unique_ptr<EventMatcher> EventListenerMap::ParseEventMatcher(
     DictionaryValue* filter_dict) {
-  return std::unique_ptr<EventMatcher>(new EventMatcher(
-      base::WrapUnique(filter_dict->DeepCopy()), MSG_ROUTING_NONE));
+  return base::MakeUnique<EventMatcher>(filter_dict->CreateDeepCopy(),
+                                        MSG_ROUTING_NONE);
 }
 
 bool EventListenerMap::RemoveListener(const EventListener* listener) {
   ListenerList& listeners = listeners_[listener->event_name()];
-  for (ListenerList::iterator it = listeners.begin(); it != listeners.end();
-       it++) {
-    if ((*it)->Equals(listener)) {
-      CleanupListener(it->get());
+  for (auto& it : listeners) {

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

s/it/listener

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

There's already a |listener| param. :-P

+    if (it->Equals(listener)) {
+      CleanupListener(it.get());
       // Popping from the back should be cheaper than erase(it).
-      std::swap(*it, listeners.back());
+      std::swap(it, listeners.back());

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

Does this work?

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

The trybots seem to think so. Since the ListenerList is really a std::vector,
|it| is just a reference to an item in the vector.

       listeners.pop_back();
       delegate_->OnListenerRemoved(listener);
       return true;
@@ -141,47 +140,45 @@ bool EventListenerMap::RemoveListener(const EventListener* listener) {
   return false;
 }
 
-bool EventListenerMap::HasListenerForEvent(const std::string& event_name) {
-  ListenerMap::iterator it = listeners_.find(event_name);
+bool EventListenerMap::HasListenerForEvent(
+    const std::string& event_name) const {
+  ListenerMap::const_iterator it = listeners_.find(event_name);
   return it != listeners_.end() && !it->second.empty();
 }
 
 bool EventListenerMap::HasListenerForExtension(
     const std::string& extension_id,
-    const std::string& event_name) {
-  ListenerMap::iterator it = listeners_.find(event_name);
+    const std::string& event_name) const {
+  auto it = listeners_.find(event_name);
   if (it == listeners_.end())
     return false;
 
-  for (ListenerList::iterator it2 = it->second.begin();
-       it2 != it->second.end(); it2++) {
-    if ((*it2)->extension_id() == extension_id)
+  for (const auto& listener_to_search : it->second) {
+    if (listener_to_search->extension_id() == extension_id)
       return true;
   }
   return false;
 }
 
-bool EventListenerMap::HasListener(const EventListener* listener) {
-  ListenerMap::iterator it = listeners_.find(listener->event_name());
+bool EventListenerMap::HasListener(const EventListener* listener) const {
+  auto it = listeners_.find(listener->event_name());
   if (it == listeners_.end())
     return false;
-  for (ListenerList::iterator it2 = it->second.begin();
-       it2 != it->second.end(); it2++) {
-    if ((*it2)->Equals(listener)) {
+
+  for (const auto& listener_to_search : it->second) {
+    if (listener_to_search->Equals(listener))
       return true;
-    }
   }
   return false;
 }
 
-bool EventListenerMap::HasProcessListener(content::RenderProcessHost* process,
-                                          const std::string& extension_id) {
-  for (ListenerMap::iterator it = listeners_.begin(); it != listeners_.end();
-       it++) {
-    for (ListenerList::iterator it2 = it->second.begin();
-         it2 != it->second.end(); it2++) {
-      if ((*it2)->process() == process &&
-          (*it2)->extension_id() == extension_id)
+bool EventListenerMap::HasProcessListener(
+    content::RenderProcessHost* process,
+    const std::string& extension_id) const {
+  for (const auto& it : listeners_) {

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

maybe s/it/listener_list?  Tricky since it's a key-value; up to you.

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

Gonna just leave it.

+    for (const auto& listener : it.second) {
+      if (listener->process() == process &&
+          listener->extension_id() == extension_id)
         return true;
     }
   }
@@ -190,17 +187,17 @@ bool EventListenerMap::HasProcessListener(content::RenderProcessHost* process,
 
 void EventListenerMap::RemoveListenersForExtension(
     const std::string& extension_id) {
-  for (ListenerMap::iterator it = listeners_.begin(); it != listeners_.end();
-       it++) {
-    for (ListenerList::iterator it2 = it->second.begin();
-         it2 != it->second.end();) {
+  for (auto& it : listeners_) {
+    auto& listener_map = it.second;
+    for (ListenerList::iterator it2 = listener_map.begin();
+         it2 != listener_map.end();) {
       if ((*it2)->extension_id() == extension_id) {
         std::unique_ptr<EventListener> listener_removed = std::move(*it2);
         CleanupListener(listener_removed.get());
-        it2 = it->second.erase(it2);
+        it2 = listener_map.erase(it2);
         delegate_->OnListenerRemoved(listener_removed.get());
       } else {
-        it2++;
+        ++it2;
       }
     }
   }
@@ -209,10 +206,9 @@ void EventListenerMap::RemoveListenersForExtension(
 void EventListenerMap::LoadUnfilteredLazyListeners(
     const std::string& extension_id,
     const std::set<std::string>& event_names) {
-  for (std::set<std::string>::const_iterator it = event_names.begin();
-       it != event_names.end(); ++it) {
+  for (const auto& name : event_names) {
     AddListener(EventListener::ForExtension(
-        *it, extension_id, NULL, std::unique_ptr<DictionaryValue>()));
+        name, extension_id, nullptr, std::unique_ptr<DictionaryValue>()));
   }
 }
 
@@ -242,18 +238,14 @@ std::set<const EventListener*> EventListenerMap::GetEventListeners(
     std::set<MatcherID> ids =
         event_filter_.MatchEvent(event.event_name, event.filter_info,
             MSG_ROUTING_NONE);
-    for (std::set<MatcherID>::iterator id = ids.begin(); id != ids.end();
-         id++) {
-      EventListener* listener = listeners_by_matcher_id_[*id];
+    for (const MatcherID& id : ids) {
+      EventListener* listener = listeners_by_matcher_id_[id];
       CHECK(listener);
       interested_listeners.insert(listener);
     }
   } else {
-    ListenerList& listeners = listeners_[event.event_name];
-    for (ListenerList::const_iterator it = listeners.begin();
-         it != listeners.end(); it++) {
-      interested_listeners.insert(it->get());
-    }
+    for (const auto& listener : listeners_[event.event_name])
+      interested_listeners.insert(listener.get());
   }
 
   return interested_listeners;
@@ -262,17 +254,17 @@ std::set<const EventListener*> EventListenerMap::GetEventListeners(
 void EventListenerMap::RemoveListenersForProcess(
     const content::RenderProcessHost* process) {
   CHECK(process);
-  for (ListenerMap::iterator it = listeners_.begin(); it != listeners_.end();
-       it++) {
-    for (ListenerList::iterator it2 = it->second.begin();
-         it2 != it->second.end();) {
+  for (auto& it : listeners_) {
+    auto& listener_map = it.second;
+    for (ListenerList::iterator it2 = listener_map.begin();
+         it2 != listener_map.end();) {
       if ((*it2)->process() == process) {
         std::unique_ptr<EventListener> listener_removed = std::move(*it2);
         CleanupListener(listener_removed.get());
-        it2 = it->second.erase(it2);
+        it2 = listener_map.erase(it2);
         delegate_->OnListenerRemoved(listener_removed.get());
       } else {
-        it2++;
+        ++it2;
       }
     }
   }
@@ -287,7 +279,7 @@ void EventListenerMap::CleanupListener(EventListener* listener) {
 }
 
 bool EventListenerMap::IsFilteredEvent(const Event& event) const {
-  return filtered_events_.count(event.event_name) > 0u;
+  return base::ContainsKey(filtered_events_, event.event_name);
 }
 
 }  // namespace extensions