Service Worker: Update stale workers

content/browser/service_worker/service_worker_version.cc

Index: content/browser/service_worker/service_worker_version.cc
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index 55b6a0e5dc2822ab51e846b8525fa0765b1a0f2f..55f13c9603a689058a5ae55e49f8309355fa1df9 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -415,7 +415,7 @@ class ServiceWorkerVersion::Metrics {
 // if the worker is not stalling.
 class ServiceWorkerVersion::PingController {
  public:
-  PingController(ServiceWorkerVersion* version) : version_(version) {}
+  explicit PingController(ServiceWorkerVersion* version) : version_(version) {}
   ~PingController() {}
 
   void Activate() { ping_state_ = PINGING; }
@@ -604,26 +604,32 @@ void ServiceWorkerVersion::ScheduleUpdate() {
     update_timer_.Reset();
     return;
   }
-  update_timer_.Start(
-      FROM_HERE, base::TimeDelta::FromSeconds(kUpdateDelaySeconds),
-      base::Bind(&ServiceWorkerVersion::StartUpdate,
-                 weak_factory_.GetWeakPtr()));
-}
 
-void ServiceWorkerVersion::DeferScheduledUpdate() {
-  if (update_timer_.IsRunning())
-    update_timer_.Reset();
+  if (is_update_scheduled_)
+    return;
+  is_update_scheduled_ = true;
+
+  // Protect |this| until the timer fires, since we may be stopping
+  // and soon no one might hold a reference to us.
+  context_->ProtectVersion(make_scoped_refptr(this));

[nhiroki, 2015/06/24 05:14:41]

Can you check |context| before accessing?

[falken, 2015/06/24 07:58:10]

Done.

+  update_timer_.Start(FROM_HERE,
+                      base::TimeDelta::FromSeconds(kUpdateDelaySeconds),
+                      base::Bind(&ServiceWorkerVersion::StartUpdate,
+                                 weak_factory_.GetWeakPtr()));
 }
 
 void ServiceWorkerVersion::StartUpdate() {
-  update_timer_.Stop();
   if (!context_)
     return;
-  ServiceWorkerRegistration* registration =
-      context_->GetLiveRegistration(registration_id_);
-  if (!registration || !registration->GetNewestVersion())
-    return;
-  context_->UpdateServiceWorker(registration, false /* force_bypass_cache */);
+  context_->storage()->FindRegistrationForId(
+      registration_id_, scope_.GetOrigin(),
+      base::Bind(&ServiceWorkerVersion::FoundRegistrationForUpdate,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerVersion::DeferScheduledUpdate() {
+  if (update_timer_.IsRunning())
+    update_timer_.Reset();
 }
 
 void ServiceWorkerVersion::DispatchMessageEvent(
@@ -1725,6 +1731,8 @@ void ServiceWorkerVersion::DidEnsureLiveRegistrationForStartWorker(
     return;
   }
 
+  MarkIfStale();
+
   switch (running_status()) {
     case RUNNING:
       RunSoon(base::Bind(callback, SERVICE_WORKER_OK));
@@ -1837,6 +1845,13 @@ void ServiceWorkerVersion::StartTimeoutTimer() {
 
 void ServiceWorkerVersion::StopTimeoutTimer() {
   timeout_timer_.Stop();
+
+  // Trigger update if worker is stale.
+  if (!stale_time_.is_null()) {
+    ClearTick(&stale_time_);
+    if (!update_timer_.IsRunning())
+      ScheduleUpdate();
+  }
 }
 
 void ServiceWorkerVersion::OnTimeoutTimer() {
@@ -1844,6 +1859,17 @@ void ServiceWorkerVersion::OnTimeoutTimer() {
          running_status() == STOPPING)
       << running_status();
 
+  MarkIfStale();
+
+  // Trigger update if worker is stale and we waited long enough for it to go
+  // idle.
+  if (GetTickDuration(stale_time_) >
+      base::TimeDelta::FromMinutes(kRequestTimeoutMinutes)) {
+    ClearTick(&stale_time_);
+    if (!update_timer_.IsRunning())
+      ScheduleUpdate();
+  }
+
   // Starting a worker hasn't finished within a certain period.
   if (GetTickDuration(start_time_) >
       base::TimeDelta::FromMinutes(kStartWorkerTimeoutMinutes)) {
@@ -2056,4 +2082,34 @@ ServiceWorkerStatusCode ServiceWorkerVersion::DeduceStartWorkerFailureReason(
   return default_code;
 }
 
+void ServiceWorkerVersion::MarkIfStale() {
+  if (!context_)
+    return;
+  if (update_timer_.IsRunning() || !stale_time_.is_null())
+    return;
+  ServiceWorkerRegistration* registration =
+      context_->GetLiveRegistration(registration_id_);
+  if (!registration || registration->active_version() != this)
+    return;
+  base::TimeDelta time_since_last_check =
+      base::Time::Now() - registration->last_update_check();
+  if (time_since_last_check >
+      base::TimeDelta::FromHours(kServiceWorkerScriptMaxCacheAgeInHours))
+    RestartTick(&stale_time_);
+}
+
+void ServiceWorkerVersion::FoundRegistrationForUpdate(
+    ServiceWorkerStatusCode status,
+    const scoped_refptr<ServiceWorkerRegistration>& registration) {
+  if (!context_)
+    return;
+  const scoped_refptr<ServiceWorkerVersion> protect = this;
+  context_->UnprotectVersion(make_scoped_refptr(this));

[nhiroki, 2015/06/24 05:14:41]

How about passing "version_id" instead of "make_scoped_refptr(this)"?

[falken, 2015/06/24 07:58:10]

Sure, done.

+  is_update_scheduled_ = false;
+  if (status != SERVICE_WORKER_OK || registration->active_version() != this)
+    return;
+  context_->UpdateServiceWorker(registration.get(),
+                                false /* force_bypass_cache */);
+}
+
 }  // namespace content