ServiceWorker: Schedule DeleteAndStartOver when failing to store resource ids

content/browser/service_worker/service_worker_script_cache_map.cc

 // Copyright 2014 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_script_cache_map.h"
 
 #include "base/logging.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_disk_cache.h"
 #include "content/browser/service_worker/service_worker_storage.h"
@@ skipping 30 matching lines @@
 void ServiceWorkerScriptCacheMap::NotifyStartedCaching(
     const GURL& url, int64 resource_id) {
   DCHECK_EQ(kInvalidServiceWorkerResponseId, LookupResourceId(url));
   DCHECK(owner_->status() == ServiceWorkerVersion::NEW ||
          owner_->status() == ServiceWorkerVersion::INSTALLING)
       << owner_->status();
   if (!context_)
     return;  // Our storage has been wiped via DeleteAndStartOver.
   resource_map_[url] =
       ServiceWorkerDatabase::ResourceRecord(resource_id, url, -1);
-  context_->storage()->StoreUncommittedResponseId(resource_id);
+  context_->storage()->StoreUncommittedResourceId(resource_id);
 }
 
 void ServiceWorkerScriptCacheMap::NotifyFinishedCaching(
     const GURL& url,
     int64 size_bytes,
     const net::URLRequestStatus& status,
     const std::string& status_message) {
   DCHECK_NE(kInvalidServiceWorkerResponseId, LookupResourceId(url));
   DCHECK(owner_->status() == ServiceWorkerVersion::NEW ||
          owner_->status() == ServiceWorkerVersion::INSTALLING ||
          owner_->status() == ServiceWorkerVersion::REDUNDANT);
   if (!context_)
     return;  // Our storage has been wiped via DeleteAndStartOver.
   if (!status.is_success()) {
-    context_->storage()->DoomUncommittedResponse(LookupResourceId(url));

[nhiroki, 2015/10/07 06:10:27]

Renamed this to ClearUncommittedResourceId

"DoomUncommitted" sounds odd to me because "uncommited" means that the resource
hasn't been stored in the diskcache yet.

[nhiroki, 2015/10/07 08:24:43]

Sorry for confusing you. This comment was completely wrong. I really meant to
say that this is a failure case and the resource for |url| wasn't written in the
diskcache, so I thought we don't have to doom it here.

But, now I'm thinking that the resource could be partially written in the disk
cache even if the failure case, so it might be better always to doom the
resource here...

This is not what I really want to fix in this CL, so I'll revert this change...

+    context_->storage()->ClearUncommittedResourceId(LookupResourceId(url));
     resource_map_.erase(url);
     if (owner_->script_url() == url) {
       main_script_status_ = status;
       main_script_status_message_ = status_message;
     }
   } else {
     resource_map_[url].size_bytes = size_bytes;
   }
 }
 
@@ skipping 47 matching lines @@
 }
 
 void ServiceWorkerScriptCacheMap::OnMetadataWritten(
     scoped_ptr<ServiceWorkerResponseMetadataWriter> writer,
     const net::CompletionCallback& callback,
     int result) {
   callback.Run(result);
 }
 
 }  // namespace content