Use FakeServiceWorkerContext in IsolatedPrerenderTabHelper unittests

This CL removes the ForTest() functions in service_worker_context.h,
specifically:
- WaitForRegistrationsInitializedForTest()
- AddRegistrationToRegisteredOriginsForTest(const url::Origin& origin)

The motivation of this change is to avoid unnecessarily exposing
methods from service_worker_context_wrapper.cc, and use the
FakeServiceWorkerContext instead of using the real context when it is
not needed. This leads to better consistency with the storage as well.

Bug: 807440
Change-Id: I01791b82062d8e3c3bef63b029e6f77d1fe02469
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2262398Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Cr-Commit-Position: refs/heads/master@{#784201}

chrome/browser/prerender/isolated/isolated_prerender_tab_helper.cc

@@ -190,6 +190,15 @@ const void* IsolatedPrerenderTabHelper::PrefetchingLikelyEventKey() {
   return &kPrefetchingLikelyEventKey;
 }
 
+static content::ServiceWorkerContext* g_service_worker_context_for_test =
+    nullptr;
+
+// static
+void IsolatedPrerenderTabHelper::SetServiceWorkerContextForTest(
+    content::ServiceWorkerContext* context) {
+  g_service_worker_context_for_test = context;
+}
+
 IsolatedPrerenderTabHelper::IsolatedPrerenderTabHelper(
     content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents) {
@@ -917,6 +926,15 @@ void IsolatedPrerenderTabHelper::OnPredictionUpdated(
   }
 }
 
+// static
+content::ServiceWorkerContext*
+IsolatedPrerenderTabHelper::GetServiceWorkerContext(Profile* profile) {
+  if (g_service_worker_context_for_test)
+    return g_service_worker_context_for_test;
+  return content::BrowserContext::GetDefaultStoragePartition(profile)
+      ->GetServiceWorkerContext();
+}
+
 // static
 void IsolatedPrerenderTabHelper::CheckEligibilityOfURL(
     Profile* profile,
@@ -971,7 +989,7 @@ void IsolatedPrerenderTabHelper::CheckEligibilityOfURL(
   }
 
   content::ServiceWorkerContext* service_worker_context_ =
-      default_storage_partition->GetServiceWorkerContext();
+      GetServiceWorkerContext(profile);
 
   bool site_has_service_worker =
       service_worker_context_->MaybeHasRegistrationForOrigin(

chrome/browser/prerender/isolated/isolated_prerender_tab_helper.h

@@ -18,6 +18,7 @@
 #include "base/time/time.h"
 #include "chrome/browser/navigation_predictor/navigation_predictor_keyed_service.h"
 #include "chrome/browser/prerender/isolated/prefetched_mainframe_response_container.h"
+#include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
@@ -224,6 +225,13 @@ class IsolatedPrerenderTabHelper
     base::Optional<base::TimeDelta> probe_latency_;
   };
 
+  // Checks if a |service_worker_context_for_test_| is available, and if not,
+  // returns the real service worker context from the default storage partition.
+  // This is used for passing the |service_worker_context| to
+  // CheckEligibilityOfURL().
+  static content::ServiceWorkerContext* GetServiceWorkerContext(
+      Profile* profile);
+
   // Used to determine if |url| is eligible for isolated prefetching. Also gives
   // a reason in |status| if one is applicable.
   using OnEligibilityResultCallback = base::OnceCallback<void(
@@ -276,6 +284,12 @@ class IsolatedPrerenderTabHelper
 
   network::mojom::NetworkContext* GetIsolatedContextForTesting() const;
 
+  // Sets the service_worker_context_for_test_ with a FakeServiceWorkerContext
+  // for the the purpose of testing.
+  // Used in the SetUp() method in isolated_prerender_tab_helper_unittest.cc.
+  static void SetServiceWorkerContextForTest(
+      content::ServiceWorkerContext* context);
+
  protected:
   // Exposed for testing.
   explicit IsolatedPrerenderTabHelper(content::WebContents* web_contents);

chrome/browser/prerender/isolated/isolated_prerender_tab_helper_unittest.cc

@@ -30,6 +30,7 @@
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/test/fake_service_worker_context.h"
 #include "content/public/test/mock_navigation_handle.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/isolation_info.h"
@@ -97,14 +98,10 @@ class IsolatedPrerenderTabHelperTest : public ChromeRenderViewHostTestHarness {
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
 
-    content::ServiceWorkerContext* service_worker_context_ =
-        content::BrowserContext::GetDefaultStoragePartition(profile())
-            ->GetServiceWorkerContext();
-    service_worker_context_->WaitForRegistrationsInitializedForTest();
-
     tab_helper_ =
         std::make_unique<TestIsolatedPrerenderTabHelper>(web_contents());
     tab_helper_->SetURLLoaderFactory(test_shared_loader_factory_);
+    tab_helper_->SetServiceWorkerContextForTest(&service_worker_context_);
 
     SetDataSaverEnabled(true);
   }
@@ -325,6 +322,8 @@ class IsolatedPrerenderTabHelperTest : public ChromeRenderViewHostTestHarness {
   network::TestURLLoaderFactory test_url_loader_factory_;
   scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
 
+  content::FakeServiceWorkerContext service_worker_context_;
+
  private:
   std::unique_ptr<TestIsolatedPrerenderTabHelper> tab_helper_;
 };
@@ -1322,10 +1321,7 @@ TEST_F(IsolatedPrerenderTabHelperTest, ServiceWorkerRegistered) {
   GURL doc_url("https://www.google.com/search?q=cats");
   GURL prediction_url("https://www.cat-food.com/");
 
-  content::ServiceWorkerContext* service_worker_context_ =
-      content::BrowserContext::GetDefaultStoragePartition(profile())
-          ->GetServiceWorkerContext();
-  service_worker_context_->AddRegistrationToRegisteredOriginsForTest(
+  service_worker_context_.AddRegistrationToRegisteredOrigins(
       url::Origin::Create(prediction_url));
 
   MakeNavigationPrediction(web_contents(), doc_url, {prediction_url});
@@ -1354,10 +1350,7 @@ TEST_F(IsolatedPrerenderTabHelperTest, ServiceWorkerNotRegistered) {
   GURL prediction_url("https://www.cat-food.com/");
   GURL service_worker_registration("https://www.service-worker.com/");
 
-  content::ServiceWorkerContext* service_worker_context_ =
-      content::BrowserContext::GetDefaultStoragePartition(profile())
-          ->GetServiceWorkerContext();
-  service_worker_context_->AddRegistrationToRegisteredOriginsForTest(
+  service_worker_context_.AddRegistrationToRegisteredOrigins(
       url::Origin::Create(service_worker_registration));
 
   MakeNavigationPrediction(web_contents(), doc_url, {prediction_url});
@@ -1538,10 +1531,7 @@ TEST_F(IsolatedPrerenderTabHelperRedirectTest, NoRedirect_ServiceWorker) {
 
   GURL site_with_worker("https://service-worker.com");
 
-  content::ServiceWorkerContext* service_worker_context_ =
-      content::BrowserContext::GetDefaultStoragePartition(profile())
-          ->GetServiceWorkerContext();
-  service_worker_context_->AddRegistrationToRegisteredOriginsForTest(
+  service_worker_context_.AddRegistrationToRegisteredOrigins(
       url::Origin::Create(site_with_worker));
 
   RunNoRedirectTest(site_with_worker);

components/performance_manager/service_worker_context_adapter.cc

@@ -122,15 +122,6 @@ bool ServiceWorkerContextAdapter::MaybeHasRegistrationForOrigin(
   return false;
 }
 
-void ServiceWorkerContextAdapter::WaitForRegistrationsInitializedForTest() {
-  NOTIMPLEMENTED();
-}
-
-void ServiceWorkerContextAdapter::AddRegistrationToRegisteredOriginsForTest(
-    const url::Origin& origin) {
-  NOTIMPLEMENTED();
-}
-
 void ServiceWorkerContextAdapter::GetAllOriginsInfo(
     GetUsageInfoCallback callback) {
   NOTIMPLEMENTED();

components/performance_manager/service_worker_context_adapter.h

@@ -62,9 +62,6 @@ class ServiceWorkerContextAdapter
       const GURL& origin,
       CountExternalRequestsCallback callback) override;
   bool MaybeHasRegistrationForOrigin(const url::Origin& origin) override;
-  void WaitForRegistrationsInitializedForTest() override;
-  void AddRegistrationToRegisteredOriginsForTest(
-      const url::Origin& origin) override;
   void GetAllOriginsInfo(GetUsageInfoCallback callback) override;
   void DeleteForOrigin(const GURL& origin_url,
                        ResultCallback callback) override;

content/browser/service_worker/service_worker_context_wrapper.cc

@@ -579,20 +579,6 @@ bool ServiceWorkerContextWrapper::MaybeHasRegistrationForOrigin(
   return false;
 }
 
-void ServiceWorkerContextWrapper::WaitForRegistrationsInitializedForTest() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (registrations_initialized_)
-    return;
-  base::RunLoop loop;
-  on_registrations_initialized_ = loop.QuitClosure();
-  loop.Run();
-}
-
-void ServiceWorkerContextWrapper::AddRegistrationToRegisteredOriginsForTest(
-    const url::Origin& origin) {
-  registered_origins_.insert(origin);
-}
-
 void ServiceWorkerContextWrapper::GetAllOriginsInfo(
     GetUsageInfoCallback callback) {
   RunOrPostTaskOnCoreThread(
@@ -2089,6 +2075,15 @@ void ServiceWorkerContextWrapper::DidSetUpLoaderFactoryForUpdateCheck(
   std::move(callback).Run(std::move(loader_factory));
 }
 
+void ServiceWorkerContextWrapper::WaitForRegistrationsInitializedForTest() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (registrations_initialized_)
+    return;
+  base::RunLoop loop;
+  on_registrations_initialized_ = loop.QuitClosure();
+  loop.Run();
+}
+
 void ServiceWorkerContextWrapper::DidGetRegisteredOrigins(
     std::vector<url::Origin> origins) {
   DCHECK_CURRENTLY_ON(GetCoreThreadId());

content/browser/service_worker/service_worker_context_wrapper.h

@@ -168,9 +168,6 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
       const GURL& url,
       CountExternalRequestsCallback callback) override;
   bool MaybeHasRegistrationForOrigin(const url::Origin& origin) override;
-  void WaitForRegistrationsInitializedForTest() override;
-  void AddRegistrationToRegisteredOriginsForTest(
-      const url::Origin& origin) override;
   void GetAllOriginsInfo(GetUsageInfoCallback callback) override;
   void DeleteForOrigin(const GURL& origin, ResultCallback callback) override;
   void PerformStorageCleanup(base::OnceClosure callback) override;
@@ -360,6 +357,12 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
   // DeleteAndStartOver fails.
   ServiceWorkerContextCore* context();
 
+  // This method waits for service worker registrations to be initialized on the
+  // UI thread, and depends on |on_registrations_initialized_| and
+  // |registrations_initialized_| which are called in
+  // InitializeRegisteredOriginsOnUI().
+  void WaitForRegistrationsInitializedForTest();
+
   // This must be called on the core thread, and the |callback| also runs on
   // the core thread which can be called with nullptr on failure.
   void GetLoaderFactoryForUpdateCheck(

content/public/browser/service_worker_context.h

@@ -165,13 +165,6 @@ class CONTENT_EXPORT ServiceWorkerContext {
   // Must be called on the UI thread.
   virtual bool MaybeHasRegistrationForOrigin(const url::Origin& origin) = 0;
 
-  // TODO(nidhijaju): Remove the ForTest() functions here and the tests that
-  // need it (e.g. in IsolatedPrerender) should use FakeServiceWorkerContext
-  // instead.
-  virtual void WaitForRegistrationsInitializedForTest() = 0;
-  virtual void AddRegistrationToRegisteredOriginsForTest(
-      const url::Origin& origin) = 0;
-
   // May be called from any thread, and the callback is called on that thread.
   virtual void GetAllOriginsInfo(GetUsageInfoCallback callback) = 0;
 

content/public/test/fake_service_worker_context.cc

@@ -57,15 +57,7 @@ void FakeServiceWorkerContext::CountExternalRequestsForTest(
 }
 bool FakeServiceWorkerContext::MaybeHasRegistrationForOrigin(
     const url::Origin& origin) {
-  NOTREACHED();
-  return false;
-}
-void FakeServiceWorkerContext::WaitForRegistrationsInitializedForTest() {
-  NOTREACHED();
-}
-void FakeServiceWorkerContext::AddRegistrationToRegisteredOriginsForTest(
-    const url::Origin& origin) {
-  NOTREACHED();
+  return registered_origins_.find(origin) != registered_origins_.end();
 }
 void FakeServiceWorkerContext::GetAllOriginsInfo(
     GetUsageInfoCallback callback) {
@@ -152,4 +144,9 @@ void FakeServiceWorkerContext::NotifyObserversOnNoControllees(
     observer.OnNoControllees(version_id, scope);
 }
 
+void FakeServiceWorkerContext::AddRegistrationToRegisteredOrigins(
+    const url::Origin& origin) {
+  registered_origins_.insert(origin);
+}
+
 }  // namespace content

content/public/test/fake_service_worker_context.h

@@ -49,9 +49,6 @@ class FakeServiceWorkerContext : public ServiceWorkerContext {
       const GURL& url,
       CountExternalRequestsCallback callback) override;
   bool MaybeHasRegistrationForOrigin(const url::Origin& origin) override;
-  void WaitForRegistrationsInitializedForTest() override;
-  void AddRegistrationToRegisteredOriginsForTest(
-      const url::Origin& origin) override;
   void GetAllOriginsInfo(GetUsageInfoCallback callback) override;
   void DeleteForOrigin(const GURL& origin, ResultCallback callback) override;
   void PerformStorageCleanup(base::OnceClosure callback) override;
@@ -83,6 +80,9 @@ class FakeServiceWorkerContext : public ServiceWorkerContext {
   void NotifyObserversOnVersionRedundant(int64_t version_id, const GURL& scope);
   void NotifyObserversOnNoControllees(int64_t version_id, const GURL& scope);
 
+  // Inserts |origin| into |registered_origins_| if it doesn't already exist.
+  void AddRegistrationToRegisteredOrigins(const url::Origin& origin);
+
   bool start_service_worker_for_navigation_hint_called() {
     return start_service_worker_for_navigation_hint_called_;
   }
@@ -115,6 +115,8 @@ class FakeServiceWorkerContext : public ServiceWorkerContext {
   base::ObserverList<ServiceWorkerContextObserver, true>::Unchecked observers_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeServiceWorkerContext);
+
+  std::set<url::Origin> registered_origins_;
 };
 
 }  // namespace content