Service Worker: Change the criteria for bumping the last update check time

content/browser/service_worker/service_worker_register_job.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/service_worker/service_worker_register_job.h"
 
 #include <stdint.h>
 
 #include <vector>
 
@@ skipping 334 matching lines @@
                                            version_id, context_));
   new_version()->set_force_bypass_cache_for_scripts(force_bypass_cache_);
   new_version()->set_skip_script_comparison(skip_script_comparison_);
   new_version()->StartWorker(
       base::Bind(&ServiceWorkerRegisterJob::OnStartWorkerFinished,
                  weak_factory_.GetWeakPtr()));
 }
 
 void ServiceWorkerRegisterJob::OnStartWorkerFinished(
     ServiceWorkerStatusCode status) {
+  // Bump the last update check time only when the register/update job fetched
+  // the version having bypassed the network cache. We assume that the
+  // BYPASS_CACHE flag evicts an existing cache entry, so even if the install
+  // ultimately failed for whatever reason, we know the version in the HTTP
+  // cache is not stale, so it's OK to bump the update check time.
+  if (new_version()->embedded_worker()->network_accessed_for_script() ||
+      new_version()->force_bypass_cache_for_scripts()) {
+    registration()->set_last_update_check(base::Time::Now());
+
+    if (job_type_ == UPDATE_JOB)

[falken, 2016/01/06 05:22:08]

Ick, I made a mistake. The registration could be on disk for a REGISTER_JOB that
found an existing registration.

I'm not sure there's an accurate way to tell if SWRegistration is stored. I
think maybe checking if there's a waiting/active version works. I really don't
want to add an is_stored flag to that class, the three curently flags
is_deleted, is_uninstalling, is_uninstalled are already a mess.

[jungkees, 2016/01/06 05:45:29]

I tried it with this.

+      context_->storage()->UpdateLastUpdateCheckTime(registration());
+  }
+
   if (status == SERVICE_WORKER_OK) {
     InstallAndContinue();
     return;
   }
 
   // The updated worker is identical to the incumbent.
   if (status == SERVICE_WORKER_ERROR_EXISTS) {
-    // Only bump the last check time when we've bypassed the browser cache.
-    base::TimeDelta time_since_last_check =
-        base::Time::Now() - registration()->last_update_check();
-    if (time_since_last_check > base::TimeDelta::FromHours(
-                                    kServiceWorkerScriptMaxCacheAgeInHours) ||
-        new_version()->force_bypass_cache_for_scripts()) {
-      registration()->set_last_update_check(base::Time::Now());
-      context_->storage()->UpdateLastUpdateCheckTime(registration());
-    }
-
     ResolvePromise(SERVICE_WORKER_OK, std::string(), registration());
     Complete(status, "The updated worker is identical to the incumbent.");
     return;
   }
 
   // "If serviceWorker fails to start up..." then reject the promise with an
   // error and abort.
   if (status == SERVICE_WORKER_ERROR_TIMEOUT) {
     Complete(status, "Timed out while trying to start the Service Worker.");
     return;
@@ skipping 44 matching lines @@
     ServiceWorkerStatusCode status) {
   ServiceWorkerMetrics::RecordInstallEventStatus(status);
 
   if (status != SERVICE_WORKER_OK) {
     // "8. If installFailed is true, then:..."
     Complete(status);
     return;
   }
 
   SetPhase(STORE);
-  registration()->set_last_update_check(base::Time::Now());
   context_->storage()->StoreRegistration(
       registration(),
       new_version(),
       base::Bind(&ServiceWorkerRegisterJob::OnStoreRegistrationComplete,
                  weak_factory_.GetWeakPtr()));
 }
 
 void ServiceWorkerRegisterJob::OnStoreRegistrationComplete(
     ServiceWorkerStatusCode status) {
   if (status != SERVICE_WORKER_OK) {
@@ skipping 100 matching lines @@
     if (host->IsHostToRunningServiceWorker())
       continue;
     if (!ServiceWorkerUtils::ScopeMatches(registration->pattern(),
                                           host->document_url()))
       continue;
     host->AddMatchingRegistration(registration);
   }
 }
 
 }  // namespace content