ServiceWorker: Make "ready" fetches registration from browser process(2/3).

content/browser/service_worker/service_worker_provider_host.cc

Index: content/browser/service_worker/service_worker_provider_host.cc
diff --git a/content/browser/service_worker/service_worker_provider_host.cc b/content/browser/service_worker/service_worker_provider_host.cc
index 2926c75032d6893c2f084cb24acb59d0cd3bd7b3..42b024a82ce1a8c835994b89d485a395bc6c9305 100644
--- a/content/browser/service_worker/service_worker_provider_host.cc
+++ b/content/browser/service_worker/service_worker_provider_host.cc
@@ -114,24 +114,40 @@ ServiceWorkerProviderHost::~ServiceWorkerProviderHost() {
   document_url_ = GURL();
   if (controlling_version_.get())
     controlling_version_->RemoveControllee(this);
-  if (associated_registration_.get()) {
-    DecreaseProcessReference(associated_registration_->pattern());
-    associated_registration_->RemoveListener(this);
+
+  for (auto& key_registration : matching_registrations_) {
+    DecreaseProcessReference(key_registration.second->pattern());
+    key_registration.second->RemoveListener(this);
   }
 
   for (const GURL& pattern : associated_patterns_)
     DecreaseProcessReference(pattern);
 }
 
+void ServiceWorkerProviderHost::OnVersionAttributesChanged(
+    ServiceWorkerRegistration* registration,
+    ChangedVersionAttributesMask changed_mask,
+    const ServiceWorkerRegistrationInfo& info) {
+  ReturnRegistrationForReadyIfNeeded();
+}
+
 void ServiceWorkerProviderHost::OnRegistrationFailed(
     ServiceWorkerRegistration* registration) {
-  DCHECK_EQ(associated_registration_.get(), registration);
-  DisassociateRegistration();
+  if (associated_registration_ == registration)
+    DisassociateRegistration();

[falken, 2015/02/16 09:18:29]

Shouldn't you RemoveMatchingRegistration in this case also?

[xiang, 2015/02/26 06:52:14]

You're right, done.

+  else
+    RemoveMatchingRegistration(registration);
+}
+
+void ServiceWorkerProviderHost::OnRegistrationFinishedUninstalling(
+    ServiceWorkerRegistration* registration) {
+  RemoveMatchingRegistration(registration);
 }
 
 void ServiceWorkerProviderHost::OnSkippedWaiting(
     ServiceWorkerRegistration* registration) {
-  DCHECK_EQ(associated_registration_.get(), registration);
+  if (associated_registration_ != registration)
+    return;
   // A client is "using" a registration if it is controlled by the active
   // worker of the registration. skipWaiting doesn't cause a client to start
   // using the registration.
@@ -202,9 +218,8 @@ bool ServiceWorkerProviderHost::SetHostedVersionId(int64 version_id) {
 void ServiceWorkerProviderHost::AssociateRegistration(
     ServiceWorkerRegistration* registration) {
   DCHECK(CanAssociateRegistration(registration));
-  IncreaseProcessReference(registration->pattern());
   associated_registration_ = registration;
-  associated_registration_->AddListener(this);
+  AddMatchingRegistration(registration);
   SendAssociateRegistrationMessage();
   SetControllerVersionAttribute(registration->active_version());
 }
@@ -213,8 +228,6 @@ void ServiceWorkerProviderHost::DisassociateRegistration() {
   queued_events_.clear();
   if (!associated_registration_.get())
     return;
-  DecreaseProcessReference(associated_registration_->pattern());
-  associated_registration_->RemoveListener(this);
   associated_registration_ = NULL;
   SetControllerVersionAttribute(NULL);
 
@@ -227,6 +240,42 @@ void ServiceWorkerProviderHost::DisassociateRegistration() {
       render_thread_id_, provider_id()));
 }
 
+void ServiceWorkerProviderHost::AddMatchingRegistration(
+    ServiceWorkerRegistration* registration) {
+  DCHECK(ServiceWorkerUtils::ScopeMatches(
+        registration->pattern(), document_url_));
+  size_t key = registration->pattern().spec().size();
+  if (ContainsKey(matching_registrations_, key))
+    return;
+  IncreaseProcessReference(registration->pattern());
+  registration->AddListener(this);
+  matching_registrations_[key] = registration;
+  ReturnRegistrationForReadyIfNeeded();
+}
+
+void ServiceWorkerProviderHost::RemoveMatchingRegistration(
+    ServiceWorkerRegistration* registration) {
+  size_t key = registration->pattern().spec().size();
+  DCHECK(ContainsKey(matching_registrations_, key));
+  DecreaseProcessReference(registration->pattern());
+  registration->RemoveListener(this);
+  matching_registrations_.erase(key);
+}
+
+ServiceWorkerRegistration*
+ServiceWorkerProviderHost::MatchRegistration() const {
+  ServiceWorkerRegistrationMap::const_reverse_iterator it =
+      matching_registrations_.rbegin();
+  for (; it != matching_registrations_.rend(); ++it) {
+    if (it->second->is_uninstalled())
+      continue;
+    if (it->second->is_uninstalling())
+      return nullptr;
+    break;

[falken, 2015/02/16 09:18:29]

nit: maybe just return it->second.get() here? then the final return is just
nullptr.

[xiang, 2015/02/26 06:52:14]

Done.

+  }
+  return it == matching_registrations_.rend() ? nullptr : it->second.get();
+}
+
 scoped_ptr<ServiceWorkerRequestHandler>
 ServiceWorkerProviderHost::CreateRequestHandler(
     FetchRequestMode request_mode,
@@ -337,6 +386,13 @@ void ServiceWorkerProviderHost::ClaimedByRegistration(
   is_claiming_ = false;
 }
 
+void ServiceWorkerProviderHost::GetRegistrationForReady(
+    const GetRegistrationForReadyCallback& callback) {
+  DCHECK(get_ready_callback_.is_null());

[falken, 2015/02/16 09:18:29]

As in my other comment, this DCHECK can be violated if the renderer is
compromised. Instead maybe return false here, so the caller can can
BadMessageReceived.

[xiang, 2015/02/26 06:52:14]

Done.

+  get_ready_callback_ = callback;
+  ReturnRegistrationForReadyIfNeeded();
+}
+
 void ServiceWorkerProviderHost::PrepareForCrossSiteTransfer() {
   DCHECK_NE(ChildProcessHost::kInvalidUniqueID, render_process_id_);
   DCHECK_NE(MSG_ROUTING_NONE, render_frame_id_);
@@ -345,8 +401,10 @@ void ServiceWorkerProviderHost::PrepareForCrossSiteTransfer() {
   for (const GURL& pattern : associated_patterns_)
     DecreaseProcessReference(pattern);
 
+  for (auto& key_registration : matching_registrations_)
+    DecreaseProcessReference(key_registration.second->pattern());
+
   if (associated_registration_.get()) {
-    DecreaseProcessReference(associated_registration_->pattern());
     if (dispatcher_host_) {
       Send(new ServiceWorkerMsg_DisassociateRegistration(
           render_thread_id_, provider_id()));
@@ -378,8 +436,10 @@ void ServiceWorkerProviderHost::CompleteCrossSiteTransfer(
   for (const GURL& pattern : associated_patterns_)
     IncreaseProcessReference(pattern);
 
+  for (auto& key_registration : matching_registrations_)
+    IncreaseProcessReference(key_registration.second->pattern());
+
   if (associated_registration_.get()) {
-    IncreaseProcessReference(associated_registration_->pattern());
     SendAssociateRegistrationMessage();
     if (dispatcher_host_ && associated_registration_->active_version()) {
       Send(new ServiceWorkerMsg_SetControllerServiceWorker(
@@ -505,6 +565,19 @@ void ServiceWorkerProviderHost::DecreaseProcessReference(
   }
 }
 
+void ServiceWorkerProviderHost::ReturnRegistrationForReadyIfNeeded() {
+  if (get_ready_callback_.is_null())
+    return;
+  ServiceWorkerRegistration* registration = MatchRegistration();
+  if (!registration)
+    return;
+  if (registration->active_version()) {
+    get_ready_callback_.Run(registration);
+    get_ready_callback_.Reset();

[falken, 2015/02/16 09:18:29]

Are you sure we should reset it here? I think the GetRegistrationForReady should
only be called once per the lifetime of the document, no?

[xiang, 2015/02/26 06:52:14]

Yes, but I thought we already made Blink can only call this once for each
document.
Not resetting it maybe more safe.

+    return;
+  }
+}
+
 bool ServiceWorkerProviderHost::IsReadyToSendMessages() const {
   return render_thread_id_ != kInvalidEmbeddedWorkerThreadId;
 }