Fix extension service worker event filter in the browser process.

This CL makes lazy variant of service worker listener drop its
service worker version id (akin to dropping worker thread id)
by setting version id to kInvalidServiceWorkerVersionId.
This CL also loads service worker listener's filters correctly
from prefs. Note that these listeners (already) start with
dropped worker version id.

This CL expands unit tests that can now be run for service workers,
namely EventRouterFilterTest and AddLazyListenersFromPreferences.

Bug: 1094697, 1095568
Change-Id: I9ea04aa5439e001d1cb92ad774fb94067929a466
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2245513
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779202}

extensions/browser/event_listener_map.cc

@@ -96,10 +96,12 @@ bool EventListener::IsLazy() const {
 
 void EventListener::MakeLazy() {
   // A lazy listener neither has a process attached to it nor it has a worker
-  // thread id (if the listener was for a service worker), so reset these values
+  // thread (if the listener was for a service worker), so reset these values
   // below to reflect that.
-  if (is_for_service_worker_)
+  if (is_for_service_worker_) {
     worker_thread_id_ = kMainThreadId;
+    service_worker_version_id_ = blink::mojom::kInvalidServiceWorkerVersionId;
+  }
   process_ = nullptr;
 }
 
@@ -300,7 +302,7 @@ void EventListenerMap::LoadFilteredLazyListeners(
             // https://crbug.com/773103.
             Extension::GetBaseURLFromExtensionId(extension_id),
             blink::mojom::kInvalidServiceWorkerVersionId, kMainThreadId,
-            nullptr));
+            filter->CreateDeepCopy()));
       } else {
         AddListener(EventListener::ForExtension(it.key(), extension_id, nullptr,
                                                 filter->CreateDeepCopy()));

extensions/browser/event_listener_map.h

@@ -124,7 +124,7 @@ class EventListener {
 
   const bool is_for_service_worker_ = false;
 
-  const int64_t service_worker_version_id_ =
+  int64_t service_worker_version_id_ =
       blink::mojom::kInvalidServiceWorkerVersionId;
 
   // If this listener is for a service worker (i.e.

extensions/browser/event_listener_map_unittest.cc

@@ -499,8 +499,8 @@ TEST_F(EventListenerMapTest, HasListenerForExtension) {
   }
 }
 
-TEST_F(EventListenerMapTest, AddLazyListenersFromPreferences) {
-  const bool is_for_service_worker = false;
+TEST_P(EventListenerMapWithContextTest, AddLazyListenersFromPreferences) {
+  const bool is_for_service_worker = GetParam();
   struct TestCase {
     const std::string filter_host_suffix;
     const std::string url_of_event;

extensions/browser/event_router_unittest.cc

@@ -187,7 +187,8 @@ class EventRouterTest : public ExtensionsTest {
   DISALLOW_COPY_AND_ASSIGN(EventRouterTest);
 };
 
-class EventRouterFilterTest : public ExtensionsTest {
+class EventRouterFilterTest : public ExtensionsTest,
+                              public testing::WithParamInterface<bool> {
  public:
   EventRouterFilterTest() {}
 
@@ -211,7 +212,9 @@ class EventRouterFilterTest : public ExtensionsTest {
 
   const DictionaryValue* GetFilteredEvents(const std::string& extension_id) {
     return event_router()->GetFilteredEvents(
-        extension_id, EventRouter::RegisteredEventType::kLazy);
+        extension_id, is_for_service_worker()
+                          ? EventRouter::RegisteredEventType::kServiceWorker
+                          : EventRouter::RegisteredEventType::kLazy);
   }
 
   bool ContainsFilter(const std::string& extension_id,
@@ -235,6 +238,8 @@ class EventRouterFilterTest : public ExtensionsTest {
     return false;
   }
 
+  bool is_for_service_worker() const { return GetParam(); }
+
  private:
   const ListValue* GetFilterList(const std::string& extension_id,
                                  const std::string& event_name) {
@@ -373,19 +378,29 @@ TEST_F(EventRouterTest, TestReportEvent) {
 }
 
 // Tests adding and removing events with filters.
-TEST_F(EventRouterFilterTest, Basic) {
+TEST_P(EventRouterFilterTest, Basic) {
   // For the purpose of this test, "." is important in |event_name| as it
   // exercises the code path that uses |event_name| as a key in DictionaryValue.
   const std::string kEventName = "webNavigation.onBeforeNavigate";
 
   const std::string kExtensionId = "mbflcebpggnecokmikipoihdbecnjfoj";
   const std::string kHostSuffixes[] = {"foo.com", "bar.com", "baz.com"};
+
+  base::Optional<ServiceWorkerIdentifier> worker_identifier = base::nullopt;
+  if (is_for_service_worker()) {
+    ServiceWorkerIdentifier identifier;
+    identifier.scope = Extension::GetBaseURLFromExtensionId(kExtensionId);
+    identifier.version_id = 99;  // Dummy version_id.
+    identifier.thread_id = 199;  // Dummy thread_id.
+    worker_identifier =
+        base::make_optional<ServiceWorkerIdentifier>(std::move(identifier));
+  }
   std::vector<std::unique_ptr<DictionaryValue>> filters;
   for (size_t i = 0; i < base::size(kHostSuffixes); ++i) {
     std::unique_ptr<base::DictionaryValue> filter =
         CreateHostSuffixFilter(kHostSuffixes[i]);
     event_router()->AddFilteredEventListener(kEventName, render_process_host(),
-                                             kExtensionId, base::nullopt,
+                                             kExtensionId, worker_identifier,
                                              *filter, true);
     filters.push_back(std::move(filter));
   }
@@ -408,7 +423,7 @@ TEST_F(EventRouterFilterTest, Basic) {
 
   // Remove the second filter.
   event_router()->RemoveFilteredEventListener(kEventName, render_process_host(),
-                                              kExtensionId, base::nullopt,
+                                              kExtensionId, worker_identifier,
                                               *filters[1], true);
   ASSERT_TRUE(ContainsFilter(kExtensionId, kEventName, *filters[0]));
   ASSERT_FALSE(ContainsFilter(kExtensionId, kEventName, *filters[1]));
@@ -416,7 +431,7 @@ TEST_F(EventRouterFilterTest, Basic) {
 
   // Remove the first filter.
   event_router()->RemoveFilteredEventListener(kEventName, render_process_host(),
-                                              kExtensionId, base::nullopt,
+                                              kExtensionId, worker_identifier,
                                               *filters[0], true);
   ASSERT_FALSE(ContainsFilter(kExtensionId, kEventName, *filters[0]));
   ASSERT_FALSE(ContainsFilter(kExtensionId, kEventName, *filters[1]));
@@ -424,11 +439,16 @@ TEST_F(EventRouterFilterTest, Basic) {
 
   // Remove the third filter.
   event_router()->RemoveFilteredEventListener(kEventName, render_process_host(),
-                                              kExtensionId, base::nullopt,
+                                              kExtensionId, worker_identifier,
                                               *filters[2], true);
   ASSERT_FALSE(ContainsFilter(kExtensionId, kEventName, *filters[0]));
   ASSERT_FALSE(ContainsFilter(kExtensionId, kEventName, *filters[1]));
   ASSERT_FALSE(ContainsFilter(kExtensionId, kEventName, *filters[2]));
 }
 
+INSTANTIATE_TEST_SUITE_P(Lazy, EventRouterFilterTest, testing::Values(false));
+INSTANTIATE_TEST_SUITE_P(ServiceWorker,
+                         EventRouterFilterTest,
+                         testing::Values(true));
+
 }  // namespace extensions