ServiceWorker: Call SyncMatchingRegistration when document_url is changed

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 67b10a78ecc5f67db3558614ad66363396779441..2dc21069e463ac6c762ed22ba21a556683c6c557 100644
--- a/content/browser/service_worker/service_worker_provider_host.cc
+++ b/content/browser/service_worker/service_worker_provider_host.cc
@@ -182,6 +182,7 @@ void ServiceWorkerProviderHost::OnSkippedWaiting(
 void ServiceWorkerProviderHost::SetDocumentUrl(const GURL& url) {
   DCHECK(!url.has_ref());
   document_url_ = url;
+  SyncMatchingRegistrations();
 }
 
 void ServiceWorkerProviderHost::SetTopmostFrameUrl(const GURL& url) {
@@ -306,19 +307,6 @@ void ServiceWorkerProviderHost::RemoveMatchingRegistration(
   matching_registrations_.erase(key);
 }
 
-void ServiceWorkerProviderHost::AddAllMatchingRegistrations() {
-  DCHECK(context_);
-  const std::map<int64_t, ServiceWorkerRegistration*>& registrations =
-      context_->GetLiveRegistrations();
-  for (const auto& key_registration : registrations) {
-    ServiceWorkerRegistration* registration = key_registration.second;
-    if (!registration->is_uninstalled() &&
-        ServiceWorkerUtils::ScopeMatches(registration->pattern(),
-                                         document_url_))
-      AddMatchingRegistration(registration);
-  }
-}
-
 ServiceWorkerRegistration*
 ServiceWorkerProviderHost::MatchRegistration() const {
   ServiceWorkerRegistrationMap::const_reverse_iterator it =
@@ -601,6 +589,27 @@ void ServiceWorkerProviderHost::SendAssociateRegistrationMessage() {
       render_thread_id_, provider_id(), handle->GetObjectInfo(), attrs));
 }
 
+void ServiceWorkerProviderHost::SyncMatchingRegistrations() {
+  DCHECK(context_);
+
+  for (const auto& it : matching_registrations_) {
+    auto* registration = it.second.get();

[nhiroki, 2016/08/19 05:26:51]

line 605 does not use auto*. Can you make them consistent?

(I'd prefer to clarify the type here like line 605 because |it.second.get()|
does not really imply its type)

[shimazu, 2016/08/19 09:43:15]

Done.

+    DecreaseProcessReference(registration->pattern());
+    registration->RemoveListener(this);
+  }
+  matching_registrations_.clear();

[nhiroki, 2016/08/19 05:26:51]

How about making...

void ServiceWorkerProviderHost::RemoveAllMatchingRegistrations() {
  for (const auto& it : matching_registrations_) {
    auto* registration = it.second.get();
    DecreaseProcessReference(registration->pattern());
    registration->RemoveListener(this);
  }
  matching_registrations_.clear();
}

and calling it here and in ~ServiceWorkerProviderHost()?

[shimazu, 2016/08/19 09:43:15]

Done.

+
+  const std::map<int64_t, ServiceWorkerRegistration*>& registrations =

[nhiroki, 2016/08/19 05:26:51]

const auto& ?

[shimazu, 2016/08/19 09:43:15]

Done.

+      context_->GetLiveRegistrations();
+  for (const auto& key_registration : registrations) {
+    ServiceWorkerRegistration* registration = key_registration.second;
+    if (!registration->is_uninstalled() &&
+        ServiceWorkerUtils::ScopeMatches(registration->pattern(),
+                                         document_url_))
+      AddMatchingRegistration(registration);
+  }
+}
+
 void ServiceWorkerProviderHost::IncreaseProcessReference(
     const GURL& pattern) {
   if (context_ && context_->process_manager()) {