Create ServiceWorkerProviderHost before starting worker

content/browser/payments/payment_app_content_unittest_base.cc

Index: content/browser/payments/payment_app_content_unittest_base.cc
diff --git a/content/browser/payments/payment_app_content_unittest_base.cc b/content/browser/payments/payment_app_content_unittest_base.cc
index 1ce3175e55c712d10a8d155d3607a68ae5aaf137..e852d38dcee9c09ee0448ac9de6360a0674c4826 100644
--- a/content/browser/payments/payment_app_content_unittest_base.cc
+++ b/content/browser/payments/payment_app_content_unittest_base.cc
@@ -28,11 +28,13 @@ namespace content {
 namespace {
 
 void RegisterServiceWorkerCallback(bool* called,
+                                   int64_t* out_registration_id,
                                    ServiceWorkerStatusCode status,
                                    const std::string& status_message,
                                    int64_t registration_id) {
   EXPECT_EQ(SERVICE_WORKER_OK, status) << ServiceWorkerStatusToString(status);
   *called = true;
+  *out_registration_id = registration_id;
 }
 
 void UnregisterServiceWorkerCallback(bool* called,
@@ -41,6 +43,11 @@ void UnregisterServiceWorkerCallback(bool* called,
   *called = true;
 }
 
+void StopWorkerCallback(bool* called, ServiceWorkerStatusCode status) {
+  EXPECT_EQ(SERVICE_WORKER_OK, status) << ServiceWorkerStatusToString(status);
+  *called = true;
+}
+
 }  // namespace
 
 class PaymentAppContentUnitTestBase::PaymentAppForWorkerTestHelper
@@ -51,21 +58,23 @@ class PaymentAppContentUnitTestBase::PaymentAppForWorkerTestHelper
         last_sw_registration_id_(kInvalidServiceWorkerRegistrationId) {}
   ~PaymentAppForWorkerTestHelper() override {}
 
-  void OnStartWorker(int embedded_worker_id,
-                     int64_t service_worker_version_id,
-                     const GURL& scope,
-                     const GURL& script_url,
-                     bool pause_after_download,
-                     mojom::ServiceWorkerEventDispatcherRequest request,
-                     mojom::EmbeddedWorkerInstanceHostAssociatedPtrInfo
-                         instance_host) override {
+  void OnStartWorker(
+      int embedded_worker_id,
+      int64_t service_worker_version_id,
+      const GURL& scope,
+      const GURL& script_url,
+      bool pause_after_download,
+      mojom::ServiceWorkerEventDispatcherRequest request,
+      mojom::EmbeddedWorkerInstanceHostAssociatedPtrInfo instance_host,
+      mojom::ServiceWorkerProviderClientInfoPtr provider_client_info) override {
     ServiceWorkerVersion* version =
         context()->GetLiveVersion(service_worker_version_id);
     last_sw_registration_id_ = version->registration_id();
     last_sw_scope_ = scope;
     EmbeddedWorkerTestHelper::OnStartWorker(
         embedded_worker_id, service_worker_version_id, scope, script_url,
-        pause_after_download, std::move(request), std::move(instance_host));
+        pause_after_download, std::move(request), std::move(instance_host),
+        std::move(provider_client_info));
   }
 
   void OnPaymentRequestEvent(
@@ -111,9 +120,23 @@ PaymentManager* PaymentAppContentUnitTestBase::CreatePaymentManager(
     const GURL& sw_script_url) {
   // Register service worker for payment manager.
   bool called = false;
+  int64_t registration_id;
   worker_helper_->context()->RegisterServiceWorker(
       scope_url, sw_script_url, nullptr,
-      base::Bind(&RegisterServiceWorkerCallback, &called));
+      base::Bind(&RegisterServiceWorkerCallback, &called, &registration_id));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(called);
+
+  // Ensure the worker used for installation has stopped.
+  called = false;
+  ServiceWorkerRegistration* registration =
+      worker_helper_->context()->GetLiveRegistration(registration_id);
+  EXPECT_TRUE(registration);

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

ASSERT, since the other EXPECTs don't make sense if false (we'd crash)

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

We seems not to be able to use ASSERT in functions whose return type is not
void. 
Do you have any typical ways to handle the situation?
Returning nullptr and handle that on the caller is one possible way, but I feel
it's a bit tough because all of callers should have a null check. 

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

Ah didn't know that. I guess we just use EXPECT here then.

+  EXPECT_TRUE(registration->active_version());
+  EXPECT_FALSE(registration->waiting_version());
+  EXPECT_FALSE(registration->installing_version());
+  registration->active_version()->StopWorker(
+      base::Bind(&StopWorkerCallback, &called));
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(called);