Delay SW check for showing app banner until SW registration is complete.

Without this, the first visit to any valid site does not register as
valid, and one more visit than should be necessary is needed before a
banner is shown.

BUG=529182

Review URL: https://codereview.chromium.org/1325973004

Cr-Commit-Position: refs/heads/master@{#357753}

10 files changed, 130 insertions, 125 deletions

diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
index 68ff525..da264d5 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
@@ -12,7 +12,6 @@ import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.pm.PackageInfo;
 import android.graphics.Bitmap;
-import android.net.Uri;
 import android.test.mock.MockPackageManager;
 import android.test.suitebuilder.annotation.MediumTest;
 import android.test.suitebuilder.annotation.SmallTest;
@@ -424,27 +423,7 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {
     @SmallTest
     @Feature({"AppBanners"})
     public void testWebAppBannerAppears() throws Exception {
-        // Create a Tab that doesn't have the AppBannerManager enabled.  This prevents race
-        // conditions between service worker activation and AppBannerManager getting triggered.
-        // This race condition is a known problem, which is why the specs include wiggle room for
-        // how many times a site must be visited.
-        AppBannerManager.setIsEnabledForTesting(false);
-        loadUrlInNewTab("about:blank");
-
-        // Visit a site that can have a banner, then wait until the service worker is activated.
-        assertTrue(CriteriaHelper.pollForUIThreadCriteria(
-                new TabLoadObserver(getActivity().getActivityTab(), WEB_APP_URL)));
-        assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
-            @Override
-            public boolean isSatisfied() {
-                String url = getActivity().getActivityTab().getUrl();
-                Uri uri = Uri.parse(url);
-                return TextUtils.equals(uri.getFragment(), "sw_activated");
-            }
-        }));
-        AppBannerManager.setIsEnabledForTesting(true);
-
-        // Revisit the site in a new tab, which will have the AppBannerManager enabled.
+        // Visit the site in a new tab.
         loadUrlInNewTab("about:blank");
         assertTrue(CriteriaHelper.pollForUIThreadCriteria(
                 new TabLoadObserver(getActivity().getActivityTab(), WEB_APP_URL)));
@@ -481,27 +460,7 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {
         final TestDataStorageFactory dataStorageFactory = new TestDataStorageFactory();
         WebappDataStorage.setFactoryForTests(dataStorageFactory);
 
-        // Create a Tab that doesn't have the AppBannerManager enabled.  This prevents race
-        // conditions between service worker activation and AppBannerManager getting triggered.
-        // This race condition is a known problem, which is why the specs include wiggle room for
-        // how many times a site must be visited.
-        AppBannerManager.setIsEnabledForTesting(false);
-        loadUrlInNewTab("about:blank");
-
-        // Visit a site that can have a banner, then wait until the service worker is activated.
-        assertTrue(CriteriaHelper.pollForUIThreadCriteria(
-                new TabLoadObserver(getActivity().getActivityTab(), WEB_APP_URL)));
-        assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
-            @Override
-            public boolean isSatisfied() {
-                String url = getActivity().getActivityTab().getUrl();
-                Uri uri = Uri.parse(url);
-                return TextUtils.equals(uri.getFragment(), "sw_activated");
-            }
-        }));
-        AppBannerManager.setIsEnabledForTesting(true);
-
-        // Revisit the site in a new tab, which will have the AppBannerManager enabled.
+        // Visit the site in a new tab.
         loadUrlInNewTab("about:blank");
         assertTrue(CriteriaHelper.pollForUIThreadCriteria(
                 new TabLoadObserver(getActivity().getActivityTab(), WEB_APP_URL)));
diff --git a/chrome/browser/banners/app_banner_data_fetcher_browsertest.cc b/chrome/browser/banners/app_banner_data_fetcher_browsertest.cc
index 027df6d..e803089 100644
--- a/chrome/browser/banners/app_banner_data_fetcher_browsertest.cc
+++ b/chrome/browser/banners/app_banner_data_fetcher_browsertest.cc
@@ -109,15 +109,6 @@ class AppBannerDataFetcherBrowserTest : public InProcessBrowserTest,
     ASSERT_FALSE(fetcher->is_active());
   }
 
-  void LoadURLAndWaitForServiceWorker(const GURL& url) {
-    content::WebContents* web_contents =
-        browser()->tab_strip_model()->GetActiveWebContents();
-    content::TestNavigationObserver observer(web_contents, 2);
-    ui_test_utils::NavigateToURL(browser(), url);
-    observer.Wait();
-    EXPECT_EQ("sw_activated", observer.last_navigation_url().ref());
-  }
-
   void RunBannerTest(const std::string& manifest_page,
                      ui::PageTransition transition,
                      unsigned int unshown_repetitions,
@@ -128,13 +119,13 @@ class AppBannerDataFetcherBrowserTest : public InProcessBrowserTest,
         browser()->tab_strip_model()->GetActiveWebContents();
 
     for (unsigned int i = 0; i < unshown_repetitions; ++i) {
-      LoadURLAndWaitForServiceWorker(test_url);
+      ui_test_utils::NavigateToURL(browser(), test_url);
       RunFetcher(web_contents->GetURL(), std::string(), transition, false);
       AppBannerDataFetcher::SetTimeDeltaForTesting(i+1);
     }
 
     // On the final loop, check whether the banner triggered or not as expected.
-    LoadURLAndWaitForServiceWorker(test_url);
+    ui_test_utils::NavigateToURL(browser(), test_url);
     RunFetcher(web_contents->GetURL(), std::string(), transition, expectation);
   }
 
@@ -246,40 +237,40 @@ IN_PROC_BROWSER_TEST_F(AppBannerDataFetcherBrowserTest,
       browser()->tab_strip_model()->GetActiveWebContents();
 
   // Add a direct nav on day 1.
-  LoadURLAndWaitForServiceWorker(test_url);
+  ui_test_utils::NavigateToURL(browser(), test_url);
   RunFetcher(web_contents->GetURL(), std::string(), ui::PAGE_TRANSITION_TYPED,
              false);
 
   // Add an indirect nav on day 1 which is ignored.
-  LoadURLAndWaitForServiceWorker(test_url);
+  ui_test_utils::NavigateToURL(browser(), test_url);
   RunFetcher(web_contents->GetURL(), std::string(), ui::PAGE_TRANSITION_LINK,
              false);
   AppBannerDataFetcher::SetTimeDeltaForTesting(1);
 
   // Add an indirect nav on day 2.
-  LoadURLAndWaitForServiceWorker(test_url);
+  ui_test_utils::NavigateToURL(browser(), test_url);
   RunFetcher(web_contents->GetURL(), std::string(),
              ui::PAGE_TRANSITION_MANUAL_SUBFRAME, false);
 
   // Add a direct nav on day 2 which overrides.
-  LoadURLAndWaitForServiceWorker(test_url);
+  ui_test_utils::NavigateToURL(browser(), test_url);
   RunFetcher(web_contents->GetURL(), std::string(),
              ui::PAGE_TRANSITION_GENERATED, false);
   AppBannerDataFetcher::SetTimeDeltaForTesting(2);
 
   // Add a direct nav on day 3.
-  LoadURLAndWaitForServiceWorker(test_url);
+  ui_test_utils::NavigateToURL(browser(), test_url);
   RunFetcher(web_contents->GetURL(), std::string(),
              ui::PAGE_TRANSITION_GENERATED, false);
   AppBannerDataFetcher::SetTimeDeltaForTesting(3);
 
   // Add an indirect nav on day 4.
-  LoadURLAndWaitForServiceWorker(test_url);
+  ui_test_utils::NavigateToURL(browser(), test_url);
   RunFetcher(web_contents->GetURL(), std::string(),
              ui::PAGE_TRANSITION_FORM_SUBMIT, false);
 
   // Add a direct nav on day 4 which should trigger the banner.
-  LoadURLAndWaitForServiceWorker(test_url);
+  ui_test_utils::NavigateToURL(browser(), test_url);
   RunFetcher(web_contents->GetURL(), std::string(),
              ui::PAGE_TRANSITION_TYPED, true);
 }
diff --git a/chrome/test/data/banners/main.js b/chrome/test/data/banners/main.js
index 8fe1af3..bb349f1 100644
--- a/chrome/test/data/banners/main.js
+++ b/chrome/test/data/banners/main.js
@@ -2,22 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-var registration;
-var interval;
-
-function waitUntilActivated() {
-  if (registration && registration.active) {
-    window.location = "#sw_activated";
-    window.clearInterval(interval);
-  } else {
-    console.log("SW not yet active.");
-  }
-}
-
 function initialize() {
-  navigator.serviceWorker.register('service_worker.js')
-    .then(function(r) {
-            registration = r;
-          });
-  interval = window.setInterval(function() {waitUntilActivated()}, 50);
-}
\ No newline at end of file
+  navigator.serviceWorker.register('service_worker.js');
+}
diff --git a/content/browser/service_worker/service_worker_context_core.cc b/content/browser/service_worker/service_worker_context_core.cc
index ec97d01..503f27d 100644
--- a/content/browser/service_worker/service_worker_context_core.cc
+++ b/content/browser/service_worker/service_worker_context_core.cc
@@ -23,6 +23,7 @@
 #include "content/browser/service_worker/service_worker_register_job.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_storage.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/browser_thread.h"
 #include "ipc/ipc_message.h"
 #include "net/http/http_response_headers.h"
@@ -668,6 +669,16 @@ void ServiceWorkerContextCore::ClearAllServiceWorkersForTest(
                  AsWeakPtr()));
 }
 
+void ServiceWorkerContextCore::CheckHasServiceWorker(
+    const GURL& url,
+    const GURL& other_url,
+    const ServiceWorkerContext::CheckHasServiceWorkerCallback callback) {
+  storage()->FindRegistrationForDocument(
+      url, base::Bind(&ServiceWorkerContextCore::
+                          DidFindRegistrationForCheckHasServiceWorker,
+                      AsWeakPtr(), other_url, callback));
+}
+
 void ServiceWorkerContextCore::OnRunningStateChanged(
     ServiceWorkerVersion* version) {
   if (!observer_list_)
@@ -759,4 +770,42 @@ ServiceWorkerProcessManager* ServiceWorkerContextCore::process_manager() {
   return wrapper_->process_manager();
 }
 
+void ServiceWorkerContextCore::DidFindRegistrationForCheckHasServiceWorker(
+    const GURL& other_url,
+    const ServiceWorkerContext::CheckHasServiceWorkerCallback callback,
+    ServiceWorkerStatusCode status,
+    const scoped_refptr<ServiceWorkerRegistration>& registration) {
+  if (status != SERVICE_WORKER_OK) {
+    callback.Run(false);
+    return;
+  }
+
+  if (!ServiceWorkerUtils::ScopeMatches(registration->pattern(), other_url)) {
+    callback.Run(false);
+    return;
+  }
+
+  if (registration->is_uninstalling() || registration->is_uninstalled()) {
+    callback.Run(false);
+    return;
+  }
+
+  if (!registration->active_version() && !registration->waiting_version()) {
+    registration->RegisterRegistrationFinishedCallback(
+        base::Bind(&ServiceWorkerContextCore::
+                       OnRegistrationFinishedForCheckHasServiceWorker,
+                   AsWeakPtr(), callback, registration));
+    return;
+  }
+
+  callback.Run(true);
+}
+
+void ServiceWorkerContextCore::OnRegistrationFinishedForCheckHasServiceWorker(
+    const ServiceWorkerContext::CheckHasServiceWorkerCallback callback,
+    const scoped_refptr<ServiceWorkerRegistration>& registration) {
+  callback.Run(registration->active_version() ||
+               registration->waiting_version());
+}
+
 }  // namespace content
diff --git a/content/browser/service_worker/service_worker_context_core.h b/content/browser/service_worker/service_worker_context_core.h
index 6f44c73..f822e47 100644
--- a/content/browser/service_worker/service_worker_context_core.h
+++ b/content/browser/service_worker/service_worker_context_core.h
@@ -20,6 +20,7 @@
 #include "content/browser/service_worker/service_worker_registration_status.h"
 #include "content/browser/service_worker/service_worker_storage.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/service_worker_context.h"
 
 class GURL;
 
@@ -262,6 +263,14 @@ class CONTENT_EXPORT ServiceWorkerContextCore
 
   void ClearAllServiceWorkersForTest(const base::Closure& callback);
 
+  // Determines if there is a ServiceWorker registration that matches |url|, and
+  // if |other_url| falls inside the scope of the same registration. See
+  // ServiceWorkerContext::CheckHasServiceWorker for more details.
+  void CheckHasServiceWorker(
+      const GURL& url,
+      const GURL& other_url,
+      const ServiceWorkerContext::CheckHasServiceWorkerCallback callback);
+
   base::WeakPtr<ServiceWorkerContextCore> AsWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
@@ -295,6 +304,15 @@ class CONTENT_EXPORT ServiceWorkerContextCore
       const GURL& origin,
       const std::vector<ServiceWorkerRegistrationInfo>& registrations);
 
+  void DidFindRegistrationForCheckHasServiceWorker(
+      const GURL& other_url,
+      const ServiceWorkerContext::CheckHasServiceWorkerCallback callback,
+      ServiceWorkerStatusCode status,
+      const scoped_refptr<ServiceWorkerRegistration>& registration);
+  void OnRegistrationFinishedForCheckHasServiceWorker(
+      const ServiceWorkerContext::CheckHasServiceWorkerCallback callback,
+      const scoped_refptr<ServiceWorkerRegistration>& registration);
+
   // It's safe to store a raw pointer instead of a scoped_refptr to |wrapper_|
   // because the Wrapper::Shutdown call that hops threads to destroy |this| uses
   // Bind() to hold a reference to |wrapper_| until |this| is fully destroyed.
diff --git a/content/browser/service_worker/service_worker_context_wrapper.cc b/content/browser/service_worker/service_worker_context_wrapper.cc
index f6785c9..aaa3758 100644
--- a/content/browser/service_worker/service_worker_context_wrapper.cc
+++ b/content/browser/service_worker/service_worker_context_wrapper.cc
@@ -332,25 +332,13 @@ void ServiceWorkerContextWrapper::DidGetAllRegistrationsForGetAllOrigins(
   callback.Run(usage_infos);
 }
 
-void ServiceWorkerContextWrapper::DidFindRegistrationForCheckHasServiceWorker(
-    const GURL& other_url,
+void ServiceWorkerContextWrapper::DidCheckHasServiceWorker(
     const CheckHasServiceWorkerCallback& callback,
-    ServiceWorkerStatusCode status,
-    const scoped_refptr<ServiceWorkerRegistration>& registration) {
+    bool has_service_worker) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  if (status != SERVICE_WORKER_OK) {
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(callback, false));
-    return;
-  }
-
-  DCHECK(registration);
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(callback, registration->active_version() &&
-                               ServiceWorkerUtils::ScopeMatches(
-                                   registration->pattern(), other_url)));
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(callback, has_service_worker));
 }
 
 void ServiceWorkerContextWrapper::StopAllServiceWorkersForOrigin(
@@ -439,11 +427,10 @@ void ServiceWorkerContextWrapper::CheckHasServiceWorker(
                             base::Bind(callback, false));
     return;
   }
-  context()->storage()->FindRegistrationForDocument(
-      net::SimplifyUrlForRequest(url),
-      base::Bind(&ServiceWorkerContextWrapper::
-                     DidFindRegistrationForCheckHasServiceWorker,
-                 this, net::SimplifyUrlForRequest(other_url), callback));
+  context()->CheckHasServiceWorker(
+      net::SimplifyUrlForRequest(url), net::SimplifyUrlForRequest(other_url),
+      base::Bind(&ServiceWorkerContextWrapper::DidCheckHasServiceWorker, this,
+                 callback));
 }
 
 void ServiceWorkerContextWrapper::ClearAllServiceWorkersForTest(
diff --git a/content/browser/service_worker/service_worker_context_wrapper.h b/content/browser/service_worker/service_worker_context_wrapper.h
index a4d1e13b..37a8d24 100644
--- a/content/browser/service_worker/service_worker_context_wrapper.h
+++ b/content/browser/service_worker/service_worker_context_wrapper.h
@@ -185,11 +185,8 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
       const GetUsageInfoCallback& callback,
       const std::vector<ServiceWorkerRegistrationInfo>& registrations);
 
-  void DidFindRegistrationForCheckHasServiceWorker(
-      const GURL& other_url,
-      const CheckHasServiceWorkerCallback& callback,
-      ServiceWorkerStatusCode status,
-      const scoped_refptr<ServiceWorkerRegistration>& registration);
+  void DidCheckHasServiceWorker(const CheckHasServiceWorkerCallback& callback,
+                                bool has_service_worker);
 
   void DidFindRegistrationForUpdate(
       ServiceWorkerStatusCode status,
diff --git a/content/browser/service_worker/service_worker_registration.cc b/content/browser/service_worker/service_worker_registration.cc
index 039afa7..7517b51 100644
--- a/content/browser/service_worker/service_worker_registration.cc
+++ b/content/browser/service_worker/service_worker_registration.cc
@@ -69,12 +69,21 @@ void ServiceWorkerRegistration::RemoveListener(Listener* listener) {
 
 void ServiceWorkerRegistration::NotifyRegistrationFailed() {
   FOR_EACH_OBSERVER(Listener, listeners_, OnRegistrationFailed(this));
+  NotifyRegistrationFinished();
 }
 
 void ServiceWorkerRegistration::NotifyUpdateFound() {
   FOR_EACH_OBSERVER(Listener, listeners_, OnUpdateFound(this));
 }
 
+void ServiceWorkerRegistration::NotifyVersionAttributesChanged(
+    ChangedVersionAttributesMask mask) {
+  FOR_EACH_OBSERVER(Listener, listeners_,
+                    OnVersionAttributesChanged(this, mask, GetInfo()));
+  if (mask.active_changed() || mask.waiting_changed())
+    NotifyRegistrationFinished();
+}
+
 ServiceWorkerRegistrationInfo ServiceWorkerRegistration::GetInfo() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   return ServiceWorkerRegistrationInfo(
@@ -104,8 +113,7 @@ void ServiceWorkerRegistration::SetActiveVersion(
     active_version_->AddListener(this);
   mask.add(ChangedVersionAttributesMask::ACTIVE_VERSION);
 
-  FOR_EACH_OBSERVER(Listener, listeners_,
-                    OnVersionAttributesChanged(this, mask, GetInfo()));
+  NotifyVersionAttributesChanged(mask);
 }
 
 void ServiceWorkerRegistration::SetWaitingVersion(
@@ -120,8 +128,7 @@ void ServiceWorkerRegistration::SetWaitingVersion(
   waiting_version_ = version;
   mask.add(ChangedVersionAttributesMask::WAITING_VERSION);
 
-  FOR_EACH_OBSERVER(Listener, listeners_,
-                    OnVersionAttributesChanged(this, mask, GetInfo()));
+  NotifyVersionAttributesChanged(mask);
 }
 
 void ServiceWorkerRegistration::SetInstallingVersion(
@@ -135,8 +142,7 @@ void ServiceWorkerRegistration::SetInstallingVersion(
   installing_version_ = version;
   mask.add(ChangedVersionAttributesMask::INSTALLING_VERSION);
 
-  FOR_EACH_OBSERVER(Listener, listeners_,
-                    OnVersionAttributesChanged(this, mask, GetInfo()));
+  NotifyVersionAttributesChanged(mask);
 }
 
 void ServiceWorkerRegistration::UnsetVersion(ServiceWorkerVersion* version) {
@@ -144,11 +150,8 @@ void ServiceWorkerRegistration::UnsetVersion(ServiceWorkerVersion* version) {
     return;
   ChangedVersionAttributesMask mask;
   UnsetVersionInternal(version, &mask);
-  if (mask.changed()) {
-    ServiceWorkerRegistrationInfo info = GetInfo();
-    FOR_EACH_OBSERVER(Listener, listeners_,
-                      OnVersionAttributesChanged(this, mask, info));
-  }
+  if (mask.changed())
+    NotifyVersionAttributesChanged(mask);
 }
 
 void ServiceWorkerRegistration::UnsetVersionInternal(
@@ -336,11 +339,26 @@ void ServiceWorkerRegistration::DeleteVersion(
       is_deleted_ = false;
     } else {
       is_uninstalled_ = true;
-      FOR_EACH_OBSERVER(Listener, listeners_, OnRegistrationFailed(this));
+      NotifyRegistrationFailed();
     }
   }
 }
 
+void ServiceWorkerRegistration::NotifyRegistrationFinished() {
+  std::vector<base::Closure> callbacks;
+  callbacks.swap(registration_finished_callbacks_);
+  for (const auto& callback : callbacks)
+    callback.Run();
+}
+
+void ServiceWorkerRegistration::RegisterRegistrationFinishedCallback(
+    const base::Closure& callback) {
+  // This should only be called if the registration is in progress.
+  DCHECK(!active_version() && !waiting_version() && !is_uninstalled() &&
+         !is_uninstalling());
+  registration_finished_callbacks_.push_back(callback);
+}
+
 void ServiceWorkerRegistration::OnActivateEventFinished(
     ServiceWorkerVersion* activating_version,
     ServiceWorkerStatusCode status) {
@@ -388,11 +406,8 @@ void ServiceWorkerRegistration::Clear() {
     active_version_ = NULL;
     mask.add(ChangedVersionAttributesMask::ACTIVE_VERSION);
   }
-  if (mask.changed()) {
-    ServiceWorkerRegistrationInfo info = GetInfo();
-    FOR_EACH_OBSERVER(Listener, listeners_,
-                      OnVersionAttributesChanged(this, mask, info));
-  }
+  if (mask.changed())
+    NotifyVersionAttributesChanged(mask);
 
   FOR_EACH_OBSERVER(
       Listener, listeners_, OnRegistrationFinishedUninstalling(this));
diff --git a/content/browser/service_worker/service_worker_registration.h b/content/browser/service_worker/service_worker_registration.h
index 4a37e55..8a8cbec 100644
--- a/content/browser/service_worker/service_worker_registration.h
+++ b/content/browser/service_worker/service_worker_registration.h
@@ -90,6 +90,7 @@ class CONTENT_EXPORT ServiceWorkerRegistration
   void RemoveListener(Listener* listener);
   void NotifyRegistrationFailed();
   void NotifyUpdateFound();
+  void NotifyVersionAttributesChanged(ChangedVersionAttributesMask mask);
 
   ServiceWorkerRegistrationInfo GetInfo();
 
@@ -143,6 +144,9 @@ class CONTENT_EXPORT ServiceWorkerRegistration
   // registration from storage if there is no longer a stored version.
   void DeleteVersion(const scoped_refptr<ServiceWorkerVersion>& version);
 
+  void RegisterRegistrationFinishedCallback(const base::Closure& callback);
+  void NotifyRegistrationFinished();
+
   bool force_update_on_page_load() const { return force_update_on_page_load_; }
   void set_force_update_on_page_load(bool force_update_on_page_load) {
     force_update_on_page_load_ = force_update_on_page_load;
@@ -190,6 +194,7 @@ class CONTENT_EXPORT ServiceWorkerRegistration
   scoped_refptr<ServiceWorkerVersion> installing_version_;
 
   base::ObserverList<Listener> listeners_;
+  std::vector<base::Closure> registration_finished_callbacks_;
   base::WeakPtr<ServiceWorkerContextCore> context_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerRegistration);
diff --git a/content/public/browser/service_worker_context.h b/content/public/browser/service_worker_context.h
index 843704d..bac9ab6 100644
--- a/content/public/browser/service_worker_context.h
+++ b/content/public/browser/service_worker_context.h
@@ -70,9 +70,9 @@ class ServiceWorkerContext {
   virtual void DeleteForOrigin(const GURL& origin_url,
                                const ResultCallback& callback) = 0;
 
-  // Returns true if an active Service Worker registration exists that matches
-  // |url|, and if |other_url| falls inside the scope of the same registration.
-  // Note this still returns true even if there is a Service Worker registration
+  // Returns true if a Service Worker registration exists that matches |url|,
+  // and if |other_url| falls inside the scope of the same registration. Note
+  // this still returns true even if there is a Service Worker registration
   // which has a longer match for |other_url|.
   // This function can be called from any thread, but the callback will always
   // be called on the UI thread.