Create ServiceWorkerProviderHost before starting worker

content/browser/background_sync/background_sync_manager_unittest.cc

Index: content/browser/background_sync/background_sync_manager_unittest.cc
diff --git a/content/browser/background_sync/background_sync_manager_unittest.cc b/content/browser/background_sync/background_sync_manager_unittest.cc
index 81ed0b6d73712946bca0df474542c94d432e931b..7b10ed8692831351a46de76abd0a5951bbc5871a 100644
--- a/content/browser/background_sync/background_sync_manager_unittest.cc
+++ b/content/browser/background_sync/background_sync_manager_unittest.cc
@@ -27,6 +27,8 @@
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
+#include "content/browser/service_worker/service_worker_dispatcher_host.h"
+#include "content/browser/service_worker/service_worker_registration_handle.h"
 #include "content/browser/service_worker/service_worker_storage.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/public/browser/background_sync_parameters.h"
@@ -445,7 +447,28 @@ TEST_F(BackgroundSyncManagerTest, RegistrationIntact) {
 }
 
 TEST_F(BackgroundSyncManagerTest, RegisterWithoutLiveSWRegistration) {
+  // Get a provider host which is used to install the service worker.
+  ASSERT_TRUE(sw_registration_1_->active_version());
+  ASSERT_FALSE(sw_registration_1_->waiting_version());
+  ASSERT_FALSE(sw_registration_1_->installing_version());
+  ServiceWorkerProviderHost* provider_host =
+      sw_registration_1_->active_version()->provider_host();
+  ASSERT_TRUE(provider_host);
+  int process_id = provider_host->process_id();
+  int provider_id = provider_host->provider_id();
+
+  // Remove the registration handle registered on the dispatcher host.
+  ServiceWorkerDispatcherHost* dispatcher_host =
+      helper_->context()->GetDispatcherHost(process_id);
+  ServiceWorkerRegistrationHandle* handle =
+      dispatcher_host->FindRegistrationHandle(provider_id,
+                                              sw_registration_1_->id());
+  dispatcher_host->OnDecrementRegistrationRefCount(handle->handle_id());
+
+  // Ensure |sw_registration_1_| is the last reference to the registration.
+  ASSERT_TRUE(sw_registration_1_->HasOneRef());

[falken, 2017/06/06 14:06:33]

Why ASSERT? Use ASSERT for fatal errors when it wouldn't make sense to continue
(we'd crash or something) if false. It looks like EXPECT is more suitable here.

[shimazu, 2017/06/12 06:08:12]

That's because if |sw_registration_1| isn't the only reference to the
registration, the following line to release the registration doesn't make sense.
That means the following Register() and |callback_status_| should be different
of the expectation. 
However I feel I don't exactly understand when ASSERT should be used... Is
EXPECT suitable in this case?

[falken, 2017/06/12 07:01:28]

Ah I see what you mean. I agree ASSERT is OK here.

   sw_registration_1_ = nullptr;
+
   EXPECT_FALSE(Register(sync_options_1_));
   EXPECT_EQ(BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER, callback_status_);
 }