After 24 hours, bust the browser cache when checking for ServiceWorker updates

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 <vector>
 
 #include "base/message_loop/message_loop.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
@@ skipping 214 matching lines @@
   if (status != SERVICE_WORKER_OK) {
     Complete(status);
     return;
   }
 
   if (existing_registration != registration()) {
     Complete(SERVICE_WORKER_ERROR_NOT_FOUND);
     return;
   }
 
+  // TODO(michaeln): If the last update check was less than 24 hours
+  // ago, depending on the freshness of the cached worker script we
+  // may be able to complete the update job right here.
+
   UpdateAndContinue();
 }
 
 // Creates a new ServiceWorkerRegistration.
 void ServiceWorkerRegisterJob::RegisterAndContinue(
     ServiceWorkerStatusCode status) {
   SetPhase(REGISTER);
   if (status != SERVICE_WORKER_OK) {
     // Abort this registration job.
     Complete(status);
@@ skipping 71 matching lines @@
     ServiceWorkerStatusCode status) {
   // TODO(kinuko,falken): For some error cases (e.g. ServiceWorker is
   // unexpectedly terminated) we may want to retry sending the event again.
   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 73 matching lines @@
   for (std::vector<RegistrationCallback>::iterator it = callbacks_.begin();
        it != callbacks_.end();
        ++it) {
     it->Run(status, registration, version);
   }
   callbacks_.clear();
 }
 
 void ServiceWorkerRegisterJob::OnPausedAfterDownload() {
   // This happens prior to OnStartWorkerFinished time.
-  scoped_refptr<ServiceWorkerVersion> current_version =
-      registration()->active_version();
-  DCHECK(current_version);
-  int64 current_script_id =
-      current_version->script_cache_map()->Lookup(script_url_);
+  scoped_refptr<ServiceWorkerVersion> most_recent_version =
+      registration()->waiting_version() ?
+          registration()->waiting_version() :
+          registration()->active_version();
+  DCHECK(most_recent_version);
+  int64 most_recent_script_id =
+      most_recent_version->script_cache_map()->Lookup(script_url_);
   int64 new_script_id =
       new_version()->script_cache_map()->Lookup(script_url_);
 
   // TODO(michaeln): It would be better to compare as the new resource
   // is being downloaded and to avoid writing it to disk until we know
   // its needed.
   context_->storage()->CompareScriptResources(
-      current_script_id, new_script_id,
+      most_recent_script_id, new_script_id,
       base::Bind(&ServiceWorkerRegisterJob::OnCompareScriptResourcesComplete,
                  weak_factory_.GetWeakPtr(),
-                 current_version));
+                 most_recent_version));
 }
 
 bool ServiceWorkerRegisterJob::OnMessageReceived(const IPC::Message& message) {
   return false;
 }
 
 void ServiceWorkerRegisterJob::OnCompareScriptResourcesComplete(
-    ServiceWorkerVersion* current_version,
+    ServiceWorkerVersion* most_recent_version,
     ServiceWorkerStatusCode status,
     bool are_equal) {
   if (are_equal) {
-    ResolvePromise(SERVICE_WORKER_OK, registration(), current_version);
+    // Only bump the last check time when we've bypassed the browser cache.

[falken, 2014/08/15 02:13:56]

Nice catch! But this seems flawed also. Doesn't it mean if you cache a script,
let 24 hours pass, and the script never changes, you'll bypass the cache every
time?

Would it be better for ContextRequestHandler to update the check time whenever
it sets LOAD_BYPASS_CACHE?

[michaeln, 2014/08/15 02:53:24]

I don't think so?
- cache script
- wait a day
- update check will bypass the cache
- script is the same, so we set a new last_update_check time
- update check 10 minutes later, won't bypass cache and won't update the last
time
- update check a day later, will bypass cache and if equal, will update the last
time and if not equal will install a new version

If you wait 24 hours between each check, you'll bypass the cache every time.
The request could fail and that class doesn't have enough context to know if the
resources match.

+    base::TimeDelta time_since_last_check =
+        base::Time::Now() - registration()->last_update_check();
+    if (time_since_last_check > base::TimeDelta::FromHours(24)) {
+      registration()->set_last_update_check(base::Time::Now());
+      context_->storage()->UpdateLastUpdateCheckTime(registration());
+    }
+
+    ResolvePromise(SERVICE_WORKER_OK, registration(), most_recent_version);
     Complete(SERVICE_WORKER_ERROR_EXISTS);
     return;
   }
 
   // Proceed with really starting the worker.
   new_version()->embedded_worker()->ResumeAfterDownload();
   new_version()->embedded_worker()->RemoveListener(this);
 }
 
 void ServiceWorkerRegisterJob::AssociateProviderHostsToRegistration(
     ServiceWorkerRegistration* registration) {
   DCHECK(registration);
   for (scoped_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
            context_->GetProviderHostIterator();
        !it->IsAtEnd(); it->Advance()) {
     ServiceWorkerProviderHost* host = it->GetProviderHost();
     if (ServiceWorkerUtils::ScopeMatches(registration->pattern(),
                                          host->document_url())) {
       if (host->CanAssociateRegistration(registration))
         host->AssociateRegistration(registration);
     }
   }
 }
 
 }  // namespace content