ServiceWorker: Make SWProviderHost listen to SWRegistration

This is a refactoring patch. Before this patch, ServiceWorkerRegisterJob is
responsible for updating ServiceWorkerRegistration object and document's
version attributes (ie. '.installing' etc) via ServiceWorkerProviderHost.

This change makes the provider host implement Registration::Listener.
Listers are notified when the registration is updated, so the register
job no longer has to manage the version attributes.


BUG=396400
TEST=content_unittests --gtest_filter=ServiceWorker*
TEST=run_webkit_tests.py --debug http/tests/serviceworker

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

Cr-Commit-Position: refs/heads/master@{#288323}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288323 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/service_worker/service_worker_controllee_request_handler.cc

@@ -114,10 +114,8 @@ void ServiceWorkerControlleeRequestHandler::PrepareForMainResource(
   DCHECK(context_);
   // The corresponding provider_host may already have associated a registration
   // in redirect case, unassociate it now.
+  provider_host_->UnassociateRegistration();
   provider_host_->SetControllerVersion(NULL);
-  provider_host_->SetActiveVersion(NULL);
-  provider_host_->SetWaitingVersion(NULL);
-  provider_host_->SetInstallingVersion(NULL);
 
   GURL stripped_url = net::SimplifyUrlForRequest(url);
   provider_host_->SetDocumentUrl(stripped_url);
@@ -152,7 +150,7 @@ ServiceWorkerControlleeRequestHandler::DidLookupRegistrationForMainResource(
 
   // Wait until it's activated before firing fetch events.
   if (active_version &&
-      active_version->status() ==  ServiceWorkerVersion::ACTIVATING) {
+      active_version->status() == ServiceWorkerVersion::ACTIVATING) {
     registration->active_version()->RegisterStatusChangeCallback(
         base::Bind(&self::OnVersionStatusChanged,
                    weak_factory_.GetWeakPtr(),
@@ -167,10 +165,8 @@ ServiceWorkerControlleeRequestHandler::DidLookupRegistrationForMainResource(
     return;
   }
 
+  provider_host_->AssociateRegistration(registration);
   provider_host_->SetControllerVersion(registration->active_version());
-  provider_host_->SetActiveVersion(registration->active_version());
-  provider_host_->SetWaitingVersion(registration->waiting_version());
-  provider_host_->SetInstallingVersion(registration->installing_version());
 
   job_->ForwardToServiceWorker();
 }
@@ -183,10 +179,9 @@ void ServiceWorkerControlleeRequestHandler::OnVersionStatusChanged(
     job_->FallbackToNetwork();
     return;
   }
+
+  provider_host_->AssociateRegistration(registration);
   provider_host_->SetControllerVersion(registration->active_version());
-  provider_host_->SetActiveVersion(registration->active_version());
-  provider_host_->SetWaitingVersion(registration->waiting_version());
-  provider_host_->SetInstallingVersion(registration->installing_version());
 
   job_->ForwardToServiceWorker();
 }

content/browser/service_worker/service_worker_job_unittest.cc

@@ -896,6 +896,10 @@ class UpdateJobTestHelper
 
   UpdateJobTestHelper(int mock_render_process_id)
       : EmbeddedWorkerTestHelper(mock_render_process_id) {}
+  virtual ~UpdateJobTestHelper() {
+    if (registration_)
+      registration_->RemoveListener(this);
+  }
 
   ServiceWorkerStorage* storage() { return context()->storage(); }
   ServiceWorkerJobCoordinator* job_coordinator() {
@@ -917,6 +921,7 @@ class UpdateJobTestHelper
     EXPECT_TRUE(registration->active_version());
     EXPECT_FALSE(registration->installing_version());
     EXPECT_FALSE(registration->waiting_version());
+    registration_ = registration;
     return registration;
   }
 
@@ -963,6 +968,11 @@ class UpdateJobTestHelper
     attribute_change_log_.push_back(entry);
   }
 
+  virtual void OnRegistrationFailed(
+      ServiceWorkerRegistration* registration) OVERRIDE {
+    NOTREACHED();
+  }
+
   // ServiceWorkerVersion::Listener overrides
   virtual void OnVersionStateChanged(ServiceWorkerVersion* version) OVERRIDE {
     StateChangeLogEntry entry;
@@ -971,6 +981,8 @@ class UpdateJobTestHelper
     state_change_log_.push_back(entry);
   }
 
+  scoped_refptr<ServiceWorkerRegistration> registration_;
+
   std::vector<AttributeChangeLogEntry> attribute_change_log_;
   std::vector<StateChangeLogEntry> state_change_log_;
 };

content/browser/service_worker/service_worker_provider_host.cc

@@ -41,6 +41,27 @@ ServiceWorkerProviderHost::~ServiceWorkerProviderHost() {
     waiting_version_->RemovePotentialControllee(this);
   if (installing_version_)
     installing_version_->RemovePotentialControllee(this);
+  if (associated_registration_)
+    associated_registration_->RemoveListener(this);
+}
+
+void ServiceWorkerProviderHost::OnVersionAttributesChanged(
+    ServiceWorkerRegistration* registration,
+    ChangedVersionAttributesMask changed_mask,
+    const ServiceWorkerRegistrationInfo& info) {
+  if (changed_mask.installing_changed())
+    SetInstallingVersion(registration->installing_version());
+  if (changed_mask.waiting_changed())
+    SetWaitingVersion(registration->waiting_version());
+  if (changed_mask.active_changed())
+    SetActiveVersion(registration->active_version());
+}
+
+void ServiceWorkerProviderHost::OnRegistrationFailed(
+    ServiceWorkerRegistration* registration) {
+  DCHECK(associated_registration_);
+  DCHECK_EQ(registration->id(), associated_registration_->id());
+  UnassociateRegistration();
 }
 
 void ServiceWorkerProviderHost::SetDocumentUrl(const GURL& url) {
@@ -159,6 +180,29 @@ bool ServiceWorkerProviderHost::SetHostedVersionId(int64 version_id) {
   return true;
 }
 
+void ServiceWorkerProviderHost::AssociateRegistration(
+    ServiceWorkerRegistration* registration) {
+  DCHECK(CanAssociateRegistration(registration));
+  associated_registration_ = registration;
+  registration->AddListener(this);
+
+  SetActiveVersion(registration->active_version());
+  SetInstallingVersion(registration->installing_version());
+  SetWaitingVersion(registration->waiting_version());
+}
+
+void ServiceWorkerProviderHost::UnassociateRegistration() {
+  if (!associated_registration_)
+    return;
+  associated_registration_->RemoveListener(this);
+  associated_registration_ = NULL;
+
+  SetActiveVersion(NULL);
+  SetInstallingVersion(NULL);
+  SetWaitingVersion(NULL);
+  SetControllerVersion(NULL);
+}
+
 scoped_ptr<ServiceWorkerRequestHandler>
 ServiceWorkerProviderHost::CreateRequestHandler(
     ResourceType resource_type,
@@ -201,6 +245,17 @@ bool ServiceWorkerProviderHost::CanAssociateVersion(
               version->registration_id();
 }
 
+bool ServiceWorkerProviderHost::CanAssociateRegistration(
+    ServiceWorkerRegistration* registration) {
+  if (!context_)
+    return false;
+  if (running_hosted_version_)
+    return false;
+  if (!registration || associated_registration_)
+    return false;
+  return true;
+}
+
 void ServiceWorkerProviderHost::PostMessage(
     const base::string16& message,
     const std::vector<int>& sent_message_port_ids) {

content/browser/service_worker/service_worker_provider_host.h

@@ -10,6 +10,7 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "content/browser/service_worker/service_worker_registration.h"
 #include "content/common/content_export.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/public/common/resource_type.h"
@@ -38,13 +39,14 @@ class ServiceWorkerVersion;
 // Note this class can also host a running service worker, in which
 // case it will observe resource loads made directly by the service worker.
 class CONTENT_EXPORT ServiceWorkerProviderHost
-    : public base::SupportsWeakPtr<ServiceWorkerProviderHost> {
+    : public ServiceWorkerRegistration::Listener,
+      public base::SupportsWeakPtr<ServiceWorkerProviderHost> {
  public:
   ServiceWorkerProviderHost(int process_id,
                             int provider_id,
                             base::WeakPtr<ServiceWorkerContextCore> context,
                             ServiceWorkerDispatcherHost* dispatcher_host);
-  ~ServiceWorkerProviderHost();
+  virtual ~ServiceWorkerProviderHost();
 
   int process_id() const { return process_id_; }
   int provider_id() const { return provider_id_; }
@@ -76,16 +78,15 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   void SetDocumentUrl(const GURL& url);
   const GURL& document_url() const { return document_url_; }
 
-  // Associates |version| to the corresponding field or if |version| is NULL
-  // clears the field.
+  // Associates |version| to the controller field or if |version| is NULL clears
+  // the field.
   void SetControllerVersion(ServiceWorkerVersion* version);
-  void SetActiveVersion(ServiceWorkerVersion* version);
-  void SetWaitingVersion(ServiceWorkerVersion* version);
-  void SetInstallingVersion(ServiceWorkerVersion* version);
 
-  // If |version| is the installing, waiting, or active version of this
-  // provider, the method will reset that field to NULL.
-  void UnsetVersion(ServiceWorkerVersion* version);
+  // Associates to |registration| to listen for its version change events.
+  void AssociateRegistration(ServiceWorkerRegistration* registration);
+
+  // Clears the associated registration and stop listening to it.
+  void UnassociateRegistration();
 
   // Returns false if the version is not in the expected STARTING in our
   // process state. That would be indicative of a bad IPC message.
@@ -100,6 +101,9 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   // Returns true if |version| can be associated with this provider.
   bool CanAssociateVersion(ServiceWorkerVersion* version);
 
+  // Returns true if |registration| can be associated with this provider.
+  bool CanAssociateRegistration(ServiceWorkerRegistration* registration);
+
   // Returns true if the context referred to by this host (i.e. |context_|) is
   // still alive.
   bool IsContextAlive();
@@ -109,6 +113,26 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
                    const std::vector<int>& sent_message_port_ids);
 
  private:
+  friend class ServiceWorkerProviderHostTest;
+
+  // ServiceWorkerRegistration::Listener overrides.
+  virtual void OnVersionAttributesChanged(
+      ServiceWorkerRegistration* registration,
+      ChangedVersionAttributesMask changed_mask,
+      const ServiceWorkerRegistrationInfo& info) OVERRIDE;
+  virtual void OnRegistrationFailed(
+      ServiceWorkerRegistration* registration) OVERRIDE;
+
+  // Associates |version| to the corresponding field or if |version| is NULL
+  // clears the field.
+  void SetActiveVersion(ServiceWorkerVersion* version);
+  void SetWaitingVersion(ServiceWorkerVersion* version);
+  void SetInstallingVersion(ServiceWorkerVersion* version);
+
+  // If |version| is the installing, waiting, or active version of this
+  // provider, the method will reset that field to NULL.
+  void UnsetVersion(ServiceWorkerVersion* version);
+
   // Creates a ServiceWorkerHandle to retain |version| and returns a
   // ServiceWorkerInfo with the handle ID to pass to the provider. The
   // provider is responsible for releasing the handle.
@@ -118,6 +142,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   const int provider_id_;
   GURL document_url_;
 
+  scoped_refptr<ServiceWorkerRegistration> associated_registration_;
+
   scoped_refptr<ServiceWorkerVersion> controlling_version_;
   scoped_refptr<ServiceWorkerVersion> active_version_;
   scoped_refptr<ServiceWorkerVersion> waiting_version_;

content/browser/service_worker/service_worker_register_job.cc

@@ -247,6 +247,7 @@ void ServiceWorkerRegisterJob::RegisterAndContinue(
   set_registration(new ServiceWorkerRegistration(
       pattern_, script_url_, context_->storage()->NewRegistrationId(),
       context_));
+  AssociateProviderHostsToRegistration(registration());
   UpdateAndContinue();
 }
 
@@ -296,7 +297,6 @@ void ServiceWorkerRegisterJob::InstallAndContinue() {
 
   // "3. Set registration.installingWorker to worker."
   registration()->SetInstallingVersion(new_version());
-  AssociateInstallingVersionToDocuments(context_, new_version());
 
   // "4. Run the [[UpdateState]] algorithm passing registration.installingWorker
   // and "installing" as the arguments."
@@ -346,9 +346,7 @@ void ServiceWorkerRegisterJob::OnStoreRegistrationComplete(
 
   // "10. Set registration.waitingWorker to registration.installingWorker."
   // "11. Set registration.installingWorker to null."
-  DisassociateVersionFromDocuments(context_, new_version());
   registration()->SetWaitingVersion(new_version());
-  AssociateWaitingVersionToDocuments(context_, new_version());
 
   // "12. Run the [[UpdateState]] algorithm passing registration.waitingWorker
   // and "installed" as the arguments."
@@ -374,11 +372,12 @@ void ServiceWorkerRegisterJob::CompleteInternal(
   if (status != SERVICE_WORKER_OK) {
     if (registration()) {
       if (new_version()) {
-        DisassociateVersionFromDocuments(context_, new_version());
         registration()->UnsetVersion(new_version());
         new_version()->Doom();
       }
-      if (!registration()->active_version()) {
+      if (!registration()->waiting_version() &&
+          !registration()->active_version()) {
+        registration()->NotifyRegistrationFailed();
         context_->storage()->DeleteRegistration(
             registration()->id(),
             registration()->script_url().GetOrigin(),
@@ -453,77 +452,19 @@ void ServiceWorkerRegisterJob::OnCompareScriptResourcesComplete(
   new_version()->embedded_worker()->RemoveListener(this);
 }
 
-// static
-void ServiceWorkerRegisterJob::AssociateInstallingVersionToDocuments(
-    base::WeakPtr<ServiceWorkerContextCore> context,
-    ServiceWorkerVersion* version) {
-  DCHECK(context);
-  DCHECK(version);
-
-  for (scoped_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
-           context->GetProviderHostIterator();
-       !it->IsAtEnd(); it->Advance()) {
-    ServiceWorkerProviderHost* host = it->GetProviderHost();
-    if (ServiceWorkerUtils::ScopeMatches(version->scope(),
-                                         host->document_url())) {
-      if (!host->CanAssociateVersion(version))
-        continue;
-      host->SetInstallingVersion(version);
-    }
-  }
-}
-
-// static
-void ServiceWorkerRegisterJob::AssociateWaitingVersionToDocuments(
-    base::WeakPtr<ServiceWorkerContextCore> context,
-    ServiceWorkerVersion* version) {
-  DCHECK(context);
-  DCHECK(version);
-
-  for (scoped_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
-           context->GetProviderHostIterator();
-       !it->IsAtEnd(); it->Advance()) {
-    ServiceWorkerProviderHost* host = it->GetProviderHost();
-    if (ServiceWorkerUtils::ScopeMatches(version->scope(),
-                                         host->document_url())) {
-      if (!host->CanAssociateVersion(version))
-        continue;
-      host->SetWaitingVersion(version);
-    }
-  }
-}
-
-// static
-void ServiceWorkerRegisterJob::AssociateActiveVersionToDocuments(
-    base::WeakPtr<ServiceWorkerContextCore> context,
-    ServiceWorkerVersion* version) {
-  DCHECK(context);
-  DCHECK(version);
-
+void ServiceWorkerRegisterJob::AssociateProviderHostsToRegistration(
+    ServiceWorkerRegistration* registration) {
+  DCHECK(registration);
   for (scoped_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
-           context->GetProviderHostIterator();
+           context_->GetProviderHostIterator();
        !it->IsAtEnd(); it->Advance()) {
     ServiceWorkerProviderHost* host = it->GetProviderHost();
-    if (ServiceWorkerUtils::ScopeMatches(version->scope(),
+    if (ServiceWorkerUtils::ScopeMatches(registration->pattern(),
                                          host->document_url())) {
-      if (!host->CanAssociateVersion(version))
-        continue;
-      host->SetActiveVersion(version);
+      if (host->CanAssociateRegistration(registration))
+        host->AssociateRegistration(registration);
     }
   }
 }
 
-// static
-void ServiceWorkerRegisterJob::DisassociateVersionFromDocuments(
-    base::WeakPtr<ServiceWorkerContextCore> context,
-    ServiceWorkerVersion* version) {
-  DCHECK(context);
-  for (scoped_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
-           context->GetProviderHostIterator();
-       !it->IsAtEnd(); it->Advance()) {
-    ServiceWorkerProviderHost* host = it->GetProviderHost();
-    host->UnsetVersion(version);
-  }
-}
-
 }  // namespace content

content/browser/service_worker/service_worker_register_job.h

@@ -67,22 +67,6 @@ class ServiceWorkerRegisterJob
   virtual bool Equals(ServiceWorkerRegisterJobBase* job) OVERRIDE;
   virtual RegistrationJobType GetType() OVERRIDE;
 
-  // TODO(michaeln): Use the registration listerer's OnVersionAttributesChanged
-  // method to replace these methods, have the host listen for changes
-  // to their registration.
-  CONTENT_EXPORT static void AssociateInstallingVersionToDocuments(
-      base::WeakPtr<ServiceWorkerContextCore> context,
-      ServiceWorkerVersion* version);
-  static void AssociateWaitingVersionToDocuments(
-      base::WeakPtr<ServiceWorkerContextCore> context,
-      ServiceWorkerVersion* version);
-  static void AssociateActiveVersionToDocuments(
-      base::WeakPtr<ServiceWorkerContextCore> context,
-      ServiceWorkerVersion* version);
-  CONTENT_EXPORT static void DisassociateVersionFromDocuments(
-      base::WeakPtr<ServiceWorkerContextCore> context,
-      ServiceWorkerVersion* version);
-
  private:
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerProviderHostWaitingVersionTest,
                            AssociateInstallingVersionToDocuments);
@@ -148,6 +132,9 @@ class ServiceWorkerRegisterJob
       ServiceWorkerStatusCode status,
       bool are_equal);
 
+  void AssociateProviderHostsToRegistration(
+      ServiceWorkerRegistration* registration);
+
   // The ServiceWorkerContextCore object should always outlive this.
   base::WeakPtr<ServiceWorkerContextCore> context_;
 

content/browser/service_worker/service_worker_registration.cc

@@ -40,6 +40,7 @@ ServiceWorkerRegistration::ServiceWorkerRegistration(
 
 ServiceWorkerRegistration::~ServiceWorkerRegistration() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(!listeners_.might_have_observers());
   if (context_)
     context_->RemoveLiveRegistration(registration_id_);
   ResetShouldActivateWhenReady();
@@ -53,6 +54,10 @@ void ServiceWorkerRegistration::RemoveListener(Listener* listener) {
   listeners_.RemoveObserver(listener);
 }
 
+void ServiceWorkerRegistration::NotifyRegistrationFailed() {
+  FOR_EACH_OBSERVER(Listener, listeners_, OnRegistrationFailed(this));
+}
+
 ServiceWorkerRegistrationInfo ServiceWorkerRegistration::GetInfo() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   return ServiceWorkerRegistrationInfo(
@@ -175,11 +180,7 @@ void ServiceWorkerRegistration::ActivateWaitingVersion() {
 
   // "5. Set serviceWorkerRegistration.activeWorker to activatingWorker."
   // "6. Set serviceWorkerRegistration.waitingWorker to null."
-  ServiceWorkerRegisterJob::DisassociateVersionFromDocuments(
-      context_, activating_version);
   SetActiveVersion(activating_version);
-  ServiceWorkerRegisterJob::AssociateActiveVersionToDocuments(
-      context_, activating_version);
 
   // "7. Run the [[UpdateState]] algorithm passing registration.activeWorker and
   // "activating" as arguments."
@@ -202,8 +203,6 @@ void ServiceWorkerRegistration::OnActivateEventFinished(
   // unexpectedly terminated) we may want to retry sending the event again.
   if (status != SERVICE_WORKER_OK) {
     // "11. If activateFailed is true, then:..."
-    ServiceWorkerRegisterJob::DisassociateVersionFromDocuments(
-        context_, activating_version);
     UnsetVersion(activating_version);
     activating_version->Doom();
     if (!waiting_version()) {

content/browser/service_worker/service_worker_registration.h

@@ -36,7 +36,9 @@ class CONTENT_EXPORT ServiceWorkerRegistration
     virtual void OnVersionAttributesChanged(
         ServiceWorkerRegistration* registration,
         ChangedVersionAttributesMask changed_mask,
-        const ServiceWorkerRegistrationInfo& info)  = 0;
+        const ServiceWorkerRegistrationInfo& info) = 0;
+    virtual void OnRegistrationFailed(
+        ServiceWorkerRegistration* registration) = 0;
   };
 
   ServiceWorkerRegistration(const GURL& pattern,
@@ -62,6 +64,7 @@ class CONTENT_EXPORT ServiceWorkerRegistration
 
   void AddListener(Listener* listener);
   void RemoveListener(Listener* listener);
+  void NotifyRegistrationFailed();
 
   ServiceWorkerRegistrationInfo GetInfo();
 

content/browser/service_worker/service_worker_registration_unittest.cc

@@ -40,7 +40,10 @@ class ServiceWorkerRegistrationTest : public testing::Test {
   class RegistrationListener : public ServiceWorkerRegistration::Listener {
    public:
     RegistrationListener() {}
-    ~RegistrationListener() {}
+    ~RegistrationListener() {
+      if (observed_registration_)
+        observed_registration_->RemoveListener(this);
+    }
 
     virtual void OnVersionAttributesChanged(
         ServiceWorkerRegistration* registration,
@@ -51,6 +54,11 @@ class ServiceWorkerRegistrationTest : public testing::Test {
       observed_info_ = info;
     }
 
+    virtual void OnRegistrationFailed(
+        ServiceWorkerRegistration* registration) OVERRIDE {
+      NOTREACHED();
+    }
+
     void Reset() {
       observed_registration_ = NULL;
       observed_changed_mask_ = ChangedVersionAttributesMask();

content/browser/service_worker/service_worker_url_request_job_unittest.cc

@@ -105,7 +105,8 @@ class ServiceWorkerURLRequestJobTest : public testing::Test {
     scoped_ptr<ServiceWorkerProviderHost> provider_host(
         new ServiceWorkerProviderHost(
             kProcessID, kProviderID, helper_->context()->AsWeakPtr(), NULL));
-    provider_host->SetActiveVersion(version_.get());
+    provider_host->AssociateRegistration(registration_);
+    registration_->SetActiveVersion(version_.get());
 
     ChromeBlobStorageContext* chrome_blob_storage_context =
         ChromeBlobStorageContext::GetFor(browser_context_.get());