Modernize some extensions code.

extensions/browser/event_listener_map_unittest.cc

Index: extensions/browser/event_listener_map_unittest.cc
diff --git a/extensions/browser/event_listener_map_unittest.cc b/extensions/browser/event_listener_map_unittest.cc
index 67de0d85cbec769a3d0c7b06beb9afd89818fd5c..9608e7bc5ab1ad195b8f21cdc70c704c6e8272fa 100644
--- a/extensions/browser/event_listener_map_unittest.cc
+++ b/extensions/browser/event_listener_map_unittest.cc
@@ -28,36 +28,35 @@ const char kEvent1Name[] = "event1";
 const char kEvent2Name[] = "event2";
 const char kURL[] = "https://google.com/some/url";
 
-typedef base::Callback<std::unique_ptr<EventListener>(
-    const std::string&,           // event_name
-    content::RenderProcessHost*,  // process
-    base::DictionaryValue*        // filter (takes ownership)
-    )>
-    EventListenerConstructor;
+using EventListenerConstructor = base::Callback<std::unique_ptr<EventListener>(
+    const std::string& /* event_name */,

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

is this stylistically preferred?

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

The old code triggers a lint warning saying ")>" should go on the previous
line... but it can't with // style comments.

We have comments like this all over the place too. e.g. in IPC message files.

+    content::RenderProcessHost* /* process */,
+    std::unique_ptr<base::DictionaryValue> /* filter */)>;
 
 class EmptyDelegate : public EventListenerMap::Delegate {
-  void OnListenerAdded(const EventListener* listener) override{};
-  void OnListenerRemoved(const EventListener* listener) override{};
+  void OnListenerAdded(const EventListener* listener) override {}
+  void OnListenerRemoved(const EventListener* listener) override {}
 };
 
 class EventListenerMapTest : public testing::Test {
  public:
   EventListenerMapTest()
-      : delegate_(new EmptyDelegate),
-        listeners_(new EventListenerMap(delegate_.get())),
-        browser_context_(new content::TestBrowserContext),
-        process_(new content::MockRenderProcessHost(browser_context_.get())) {}
+      : delegate_(base::MakeUnique<EmptyDelegate>()),
+        listeners_(base::MakeUnique<EventListenerMap>(delegate_.get())),
+        browser_context_(base::MakeUnique<content::TestBrowserContext>()),
+        process_(base::MakeUnique<content::MockRenderProcessHost>(
+            browser_context_.get())) {}
 
   std::unique_ptr<DictionaryValue> CreateHostSuffixFilter(
       const std::string& suffix) {
-    std::unique_ptr<DictionaryValue> filter(new DictionaryValue);
-    std::unique_ptr<ListValue> filter_list(new ListValue);
-    std::unique_ptr<DictionaryValue> filter_dict(new DictionaryValue);
-
-    filter_dict->Set("hostSuffix", new Value(suffix));
+    auto filter_dict = base::MakeUnique<DictionaryValue>();
+    filter_dict->Set("hostSuffix", base::MakeUnique<Value>(suffix));
 
+    auto filter_list = base::MakeUnique<ListValue>();
     filter_list->Append(std::move(filter_dict));
-    filter->Set("url", filter_list.release());
+
+    auto filter = base::MakeUnique<DictionaryValue>();
+    filter->Set("url", std::move(filter_list));
     return filter;
   }
 
@@ -69,10 +68,9 @@ class EventListenerMapTest : public testing::Test {
                                      const GURL& url) {
     EventFilteringInfo info;
     info.SetURL(url);
-    std::unique_ptr<Event> result(
-        new Event(events::FOR_TEST, event_name, base::MakeUnique<ListValue>(),
-                  NULL, GURL(), EventRouter::USER_GESTURE_UNKNOWN, info));
-    return result;
+    return base::MakeUnique<Event>(
+        events::FOR_TEST, event_name, base::MakeUnique<ListValue>(), nullptr,
+        GURL(), EventRouter::USER_GESTURE_UNKNOWN, info);
   }
 
  protected:
@@ -94,24 +92,24 @@ std::unique_ptr<EventListener> CreateEventListenerForExtension(
     const std::string& extension_id,
     const std::string& event_name,
     content::RenderProcessHost* process,
-    base::DictionaryValue* filter) {
+    std::unique_ptr<base::DictionaryValue> filter) {
   return EventListener::ForExtension(event_name, extension_id, process,
-                                     base::WrapUnique(filter));
+                                     std::move(filter));
 }
 
 std::unique_ptr<EventListener> CreateEventListenerForURL(
     const GURL& listener_url,
     const std::string& event_name,
     content::RenderProcessHost* process,
-    base::DictionaryValue* filter) {
+    std::unique_ptr<base::DictionaryValue> filter) {
   return EventListener::ForURL(event_name, listener_url, process,
-                               base::WrapUnique(filter));
+                               std::move(filter));
 }
 
 void EventListenerMapTest::TestUnfilteredEventsGoToAllListeners(
     const EventListenerConstructor& constructor) {
-  listeners_->AddListener(
-      constructor.Run(kEvent1Name, NULL, new DictionaryValue()));
+  listeners_->AddListener(constructor.Run(
+      kEvent1Name, nullptr, base::MakeUnique<base::DictionaryValue>()));
   std::unique_ptr<Event> event(CreateNamedEvent(kEvent1Name));
   ASSERT_EQ(1u, listeners_->GetEventListeners(*event).size());
 }
@@ -128,10 +126,9 @@ TEST_F(EventListenerMapTest, UnfilteredEventsGoToAllListenersForURLs) {
 
 TEST_F(EventListenerMapTest, FilteredEventsGoToAllMatchingListeners) {
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL,
-      std::unique_ptr<DictionaryValue>(new DictionaryValue)));
+      kEvent1Name, kExt1Id, nullptr, base::MakeUnique<DictionaryValue>()));
 
   std::unique_ptr<Event> event(CreateNamedEvent(kEvent1Name));
   event->filter_info.SetURL(GURL("http://www.google.com"));
@@ -141,9 +138,9 @@ TEST_F(EventListenerMapTest, FilteredEventsGoToAllMatchingListeners) {
 
 TEST_F(EventListenerMapTest, FilteredEventsOnlyGoToMatchingListeners) {
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("yahoo.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("yahoo.com")));
 
   std::unique_ptr<Event> event(CreateNamedEvent(kEvent1Name));
   event->filter_info.SetURL(GURL("http://www.google.com"));
@@ -153,7 +150,7 @@ TEST_F(EventListenerMapTest, FilteredEventsOnlyGoToMatchingListeners) {
 
 TEST_F(EventListenerMapTest, LazyAndUnlazyListenersGetReturned) {
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
 
   listeners_->AddListener(
       EventListener::ForExtension(kEvent1Name,
@@ -170,12 +167,10 @@ TEST_F(EventListenerMapTest, LazyAndUnlazyListenersGetReturned) {
 void EventListenerMapTest::TestRemovingByProcess(
     const EventListenerConstructor& constructor) {
   listeners_->AddListener(constructor.Run(
-      kEvent1Name, NULL, CreateHostSuffixFilter("google.com").release()));
+      kEvent1Name, nullptr, CreateHostSuffixFilter("google.com")));
 
-  listeners_->AddListener(
-      constructor.Run(kEvent1Name,
-                      process_.get(),
-                      CreateHostSuffixFilter("google.com").release()));
+  listeners_->AddListener(constructor.Run(
+      kEvent1Name, process_.get(), CreateHostSuffixFilter("google.com")));
 
   listeners_->RemoveListenersForProcess(process_.get());
 
@@ -195,16 +190,13 @@ TEST_F(EventListenerMapTest, TestRemovingByProcessForURL) {
 void EventListenerMapTest::TestRemovingByListener(
     const EventListenerConstructor& constructor) {
   listeners_->AddListener(constructor.Run(
-      kEvent1Name, NULL, CreateHostSuffixFilter("google.com").release()));
+      kEvent1Name, nullptr, CreateHostSuffixFilter("google.com")));
 
-  listeners_->AddListener(
-      constructor.Run(kEvent1Name,
-                      process_.get(),
-                      CreateHostSuffixFilter("google.com").release()));
+  listeners_->AddListener(constructor.Run(
+      kEvent1Name, process_.get(), CreateHostSuffixFilter("google.com")));
 
-  std::unique_ptr<EventListener> listener(
-      constructor.Run(kEvent1Name, process_.get(),
-                      CreateHostSuffixFilter("google.com").release()));
+  std::unique_ptr<EventListener> listener(constructor.Run(
+      kEvent1Name, process_.get(), CreateHostSuffixFilter("google.com")));
   listeners_->RemoveListener(listener.get());
 
   std::unique_ptr<Event> event(CreateNamedEvent(kEvent1Name));
@@ -222,12 +214,12 @@ TEST_F(EventListenerMapTest, TestRemovingByListenerForURL) {
 
 TEST_F(EventListenerMapTest, TestLazyDoubleAddIsUndoneByRemove) {
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
 
   std::unique_ptr<EventListener> listener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
   listeners_->RemoveListener(listener.get());
 
   std::unique_ptr<Event> event(CreateNamedEvent(kEvent1Name));
@@ -246,27 +238,29 @@ TEST_F(EventListenerMapTest, HostSuffixFilterEquality) {
 
 TEST_F(EventListenerMapTest, RemoveListenersForExtension) {
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent2Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent2Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
 
   listeners_->RemoveListenersForExtension(kExt1Id);
 
-  std::unique_ptr<Event> event(CreateNamedEvent(kEvent1Name));
-  event->filter_info.SetURL(GURL("http://www.google.com"));
-  std::set<const EventListener*> targets(listeners_->GetEventListeners(*event));
+  std::unique_ptr<Event> event1(CreateNamedEvent(kEvent1Name));
+  event1->filter_info.SetURL(GURL("http://www.google.com"));
+  std::set<const EventListener*> targets(
+      listeners_->GetEventListeners(*event1));
   ASSERT_EQ(0u, targets.size());
 
-  event->event_name = kEvent2Name;
-  targets = listeners_->GetEventListeners(*event);
+  std::unique_ptr<Event> event2(CreateNamedEvent(kEvent2Name));
+  event2->filter_info.SetURL(GURL("http://www.google.com"));
+  targets = listeners_->GetEventListeners(*event2);
   ASSERT_EQ(0u, targets.size());
 }
 
 TEST_F(EventListenerMapTest, AddExistingFilteredListener) {
   bool first_new = listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
   bool second_new = listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
 
   ASSERT_TRUE(first_new);
   ASSERT_FALSE(second_new);
@@ -274,13 +268,13 @@ TEST_F(EventListenerMapTest, AddExistingFilteredListener) {
 
 void EventListenerMapTest::TestAddExistingUnfilteredListener(
     const EventListenerConstructor& constructor) {
-  bool first_add = listeners_->AddListener(
-      constructor.Run(kEvent1Name, NULL, new DictionaryValue()));
-  bool second_add = listeners_->AddListener(
-      constructor.Run(kEvent1Name, NULL, new DictionaryValue()));
+  bool first_add = listeners_->AddListener(constructor.Run(
+      kEvent1Name, nullptr, base::MakeUnique<base::DictionaryValue>()));
+  bool second_add = listeners_->AddListener(constructor.Run(
+      kEvent1Name, nullptr, base::MakeUnique<base::DictionaryValue>()));
 
-  std::unique_ptr<EventListener> listener(
-      constructor.Run(kEvent1Name, NULL, new DictionaryValue()));
+  std::unique_ptr<EventListener> listener(constructor.Run(
+      kEvent1Name, nullptr, base::MakeUnique<base::DictionaryValue>()));
   bool first_remove = listeners_->RemoveListener(listener.get());
   bool second_remove = listeners_->RemoveListener(listener.get());
 
@@ -315,8 +309,8 @@ void EventListenerMapTest::TestHasListenerForEvent(
     const EventListenerConstructor& constructor) {
   ASSERT_FALSE(listeners_->HasListenerForEvent(kEvent1Name));
 
-  listeners_->AddListener(
-      constructor.Run(kEvent1Name, process_.get(), new DictionaryValue()));
+  listeners_->AddListener(constructor.Run(
+      kEvent1Name, process_.get(), base::MakeUnique<base::DictionaryValue>()));
 
   ASSERT_FALSE(listeners_->HasListenerForEvent(kEvent2Name));
   ASSERT_TRUE(listeners_->HasListenerForEvent(kEvent1Name));
@@ -342,7 +336,7 @@ TEST_F(EventListenerMapTest, HasListenerForExtension) {
                                   std::unique_ptr<DictionaryValue>()));
   // Lazy listener.
   listeners_->AddListener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, std::unique_ptr<DictionaryValue>()));
+      kEvent1Name, kExt1Id, nullptr, std::unique_ptr<DictionaryValue>()));
 
   ASSERT_FALSE(listeners_->HasListenerForExtension(kExt1Id, kEvent2Name));
   ASSERT_TRUE(listeners_->HasListenerForExtension(kExt1Id, kEvent1Name));
@@ -354,17 +348,12 @@ TEST_F(EventListenerMapTest, HasListenerForExtension) {
 }
 
 TEST_F(EventListenerMapTest, AddLazyListenersFromPreferences) {
-  std::unique_ptr<DictionaryValue> filter1(
-      CreateHostSuffixFilter("google.com"));
-  std::unique_ptr<DictionaryValue> filter2(CreateHostSuffixFilter("yahoo.com"));
+  auto filter_list = base::MakeUnique<ListValue>();
+  filter_list->Append(CreateHostSuffixFilter("google.com"));
+  filter_list->Append(CreateHostSuffixFilter("yahoo.com"));
 
   DictionaryValue filtered_listeners;
-  ListValue* filter_list = new ListValue();
-  filtered_listeners.Set(kEvent1Name, filter_list);
-
-  filter_list->Append(std::move(filter1));
-  filter_list->Append(std::move(filter2));
-
+  filtered_listeners.Set(kEvent1Name, std::move(filter_list));
   listeners_->LoadFilteredLazyListeners(kExt1Id, filtered_listeners);
 
   std::unique_ptr<Event> event(
@@ -372,17 +361,14 @@ TEST_F(EventListenerMapTest, AddLazyListenersFromPreferences) {
   std::set<const EventListener*> targets(listeners_->GetEventListeners(*event));
   ASSERT_EQ(1u, targets.size());
   std::unique_ptr<EventListener> listener(EventListener::ForExtension(
-      kEvent1Name, kExt1Id, NULL, CreateHostSuffixFilter("google.com")));
+      kEvent1Name, kExt1Id, nullptr, CreateHostSuffixFilter("google.com")));
   ASSERT_TRUE((*targets.begin())->Equals(listener.get()));
 }
 
 TEST_F(EventListenerMapTest, CorruptedExtensionPrefsShouldntCrash) {
-  std::unique_ptr<DictionaryValue> filter1(
-      CreateHostSuffixFilter("google.com"));
-
   DictionaryValue filtered_listeners;
   // kEvent1Name should be associated with a list, not a dictionary.
-  filtered_listeners.Set(kEvent1Name, filter1.release());
+  filtered_listeners.Set(kEvent1Name, CreateHostSuffixFilter("google.com"));
 
   listeners_->LoadFilteredLazyListeners(kExt1Id, filtered_listeners);