ServiceWorker: Ensure that ServiceWorkerDispatcher always adopts passed handle references (1)

content/child/service_worker/service_worker_dispatcher_unittest.cc

 // Copyright 2015 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 "content/child/service_worker/service_worker_dispatcher.h"
 #include "content/child/service_worker/service_worker_provider_context.h"
 #include "content/child/service_worker/web_service_worker_impl.h"
 #include "content/child/service_worker/web_service_worker_registration_impl.h"
 #include "content/child/thread_safe_sender.h"
 #include "content/common/service_worker/service_worker_messages.h"
@@ skipping 47 matching lines @@
   }
 
   bool ContainsServiceWorker(int handle_id) {
     return ContainsKey(dispatcher_->service_workers_, handle_id);
   }
 
   bool ContainsRegistration(int registration_handle_id) {
     return ContainsKey(dispatcher_->registrations_, registration_handle_id);
   }
 
+  void OnAssociateRegistrationWithServiceWorker(
+      int thread_id,
+      int provider_id,
+      const ServiceWorkerRegistrationObjectInfo& info,
+      const ServiceWorkerVersionAttributes& attrs) {
+    dispatcher_->OnAssociateRegistrationWithServiceWorker(
+        thread_id, provider_id, info, attrs);
+  }
+
   void OnAssociateRegistration(int thread_id,
                                int provider_id,
                                const ServiceWorkerRegistrationObjectInfo& info,
                                const ServiceWorkerVersionAttributes& attrs) {
     dispatcher_->OnAssociateRegistration(thread_id, provider_id, info, attrs);
   }
 
+  void OnDisassociateRegistration(int thread_id, int provider_id) {
+    dispatcher_->OnDisassociateRegistration(thread_id, provider_id);
+  }
+
   void OnSetControllerServiceWorker(int thread_id,
                                     int provider_id,
                                     const ServiceWorkerObjectInfo& info,
                                     bool should_notify_controllerchange) {
     dispatcher_->OnSetControllerServiceWorker(thread_id, provider_id, info,
                                               should_notify_controllerchange);
   }
 
   ServiceWorkerDispatcher* dispatcher() { return dispatcher_.get(); }
   ThreadSafeSender* thread_safe_sender() { return sender_.get(); }
@@ skipping 34 matching lines @@
       const blink::WebMessagePortChannelArray& channels) override {
     NOTREACHED();
   }
 
  private:
   const int provider_id_;
   ServiceWorkerDispatcher* dispatcher_;
 };
 
 // TODO(nhiroki): Add tests for message handlers especially to receive reference
-// counts like OnAssociateRegistration().
+// counts.
+
+TEST_F(ServiceWorkerDispatcherTest, OnAssociateRegistrationWithServiceWorker) {
+  const int kProviderId = 10;
+
+  // Assume that these objects are passed from the browser process and own
+  // references to browser-side registration/worker representations.
+  ServiceWorkerRegistrationObjectInfo info;
+  ServiceWorkerVersionAttributes attrs;
+  CreateObjectInfoAndVersionAttributes(&info, &attrs);
+
+  // The passed references should be adopted but immediately destroyed because
+  // there is no provider context to own the references.
+  OnAssociateRegistrationWithServiceWorker(kDocumentMainThreadId, kProviderId,
+                                           info, attrs);
+  ASSERT_EQ(4UL, ipc_sink()->message_count());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(0)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(1)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(2)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementRegistrationRefCount::ID,
+            ipc_sink()->GetMessageAt(3)->type());
+  ipc_sink()->ClearMessages();
+
+  // Set up ServiceWorkerProviderContext for ServiceWorkerGlobalScope.
+  scoped_refptr<ServiceWorkerProviderContext> provider_context(
+      new ServiceWorkerProviderContext(kProviderId,
+                                       SERVICE_WORKER_PROVIDER_FOR_CONTROLLER,
+                                       thread_safe_sender()));
+
+  // The passed references should be adoped and owned by the provider context.

[falken, 2015/11/20 03:35:54]

nit: adopted (two places)

[nhiroki, 2015/11/20 05:14:59]

Done.

+  OnAssociateRegistrationWithServiceWorker(kDocumentMainThreadId, kProviderId,
+                                           info, attrs);
+  EXPECT_EQ(0UL, ipc_sink()->message_count());
+
+  // Destruction of the provider context should release references to the
+  // associated registration and its versions.
+  provider_context = nullptr;
+  ASSERT_EQ(4UL, ipc_sink()->message_count());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(0)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(1)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(2)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementRegistrationRefCount::ID,
+            ipc_sink()->GetMessageAt(3)->type());
+}
+
+TEST_F(ServiceWorkerDispatcherTest, OnAssociateRegistration) {
+  const int kProviderId = 10;
+
+  // Assume that these objects are passed from the browser process and own
+  // references to browser-side registration/worker representations.
+  ServiceWorkerRegistrationObjectInfo info;
+  ServiceWorkerVersionAttributes attrs;
+  CreateObjectInfoAndVersionAttributes(&info, &attrs);
+
+  // The passed references should be adopted but immediately destroyed because
+  // there is no provider context to own the references.
+  OnAssociateRegistrationWithServiceWorker(kDocumentMainThreadId, kProviderId,
+                                           info, attrs);

[falken, 2015/11/20 03:35:54]

How does this test differ from the one above it? The names don't give much clue;
they are both calling OnAssociateRegistrationWithServiceWorker.

[nhiroki, 2015/11/20 05:14:59]

Good catch! That should be OnAssociateRegistration. Fixed.

By the way, OnAssociateRegistration is now equal to
OnAssociateRegistrationWithServiceWorker and an operation is dispatched into
different implementations (ControllerDelegate vs. ControlleeDelegate) in
ServiceWorkerProviderContext depending on its provider type (please see my
comment on line 220), so we could simply merge them. I'll try it in a separate
CL.

[falken, 2015/11/20 05:26:25]

Agree with merging them.

+  ASSERT_EQ(4UL, ipc_sink()->message_count());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(0)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(1)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(2)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementRegistrationRefCount::ID,
+            ipc_sink()->GetMessageAt(3)->type());
+  ipc_sink()->ClearMessages();
+
+  // Set up ServiceWorkerProviderContext for a document context.
+  scoped_refptr<ServiceWorkerProviderContext> provider_context(
+      new ServiceWorkerProviderContext(kProviderId,
+                                       SERVICE_WORKER_PROVIDER_FOR_WINDOW,

[nhiroki, 2015/11/20 05:14:59]

fyi: This provider type is 
different from one in the other test (FOR_WINDOW vs. FOR_CONTROLLER).

+                                       thread_safe_sender()));
+
+  // The passed references should be adoped and only the registration reference
+  // should be owned by the provider context.
+  OnAssociateRegistrationWithServiceWorker(kDocumentMainThreadId, kProviderId,
+                                           info, attrs);
+  ASSERT_EQ(3UL, ipc_sink()->message_count());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(0)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(1)->type());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
+            ipc_sink()->GetMessageAt(2)->type());
+  ipc_sink()->ClearMessages();
+
+  // Disassociating the provider context from the registration should release
+  // the reference.
+  OnDisassociateRegistration(kDocumentMainThreadId, kProviderId);
+  ASSERT_EQ(1UL, ipc_sink()->message_count());
+  EXPECT_EQ(ServiceWorkerHostMsg_DecrementRegistrationRefCount::ID,
+            ipc_sink()->GetMessageAt(0)->type());
+}
 
 TEST_F(ServiceWorkerDispatcherTest, OnSetControllerServiceWorker) {
   const int kProviderId = 10;
   bool should_notify_controllerchange = true;
 
   // Assume that these objects are passed from the browser process and own
   // references to browser-side registration/worker representations.
   ServiceWorkerRegistrationObjectInfo info;
   ServiceWorkerVersionAttributes attrs;
   CreateObjectInfoAndVersionAttributes(&info, &attrs);
@@ skipping 211 matching lines @@
             ipc_sink()->GetMessageAt(0)->type());
   EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
             ipc_sink()->GetMessageAt(1)->type());
   EXPECT_EQ(ServiceWorkerHostMsg_DecrementServiceWorkerRefCount::ID,
             ipc_sink()->GetMessageAt(2)->type());
   EXPECT_EQ(ServiceWorkerHostMsg_DecrementRegistrationRefCount::ID,
             ipc_sink()->GetMessageAt(3)->type());
 }
 
 }  // namespace content