Revert of Evict Service Worker when reading it from disk cache fails....

Revert of Evict Service Worker when reading it from disk cache fails. (patchset #2 id:20001 of https://codereview.chromium.org/1098083003/)

Reason for revert:
Broke Win 8 Aura content_browsertests

[ RUN      ] ServiceWorkerVersionBrowserTest.ReadResourceFailure
...
[2420:2792:0426/232049:1965296:FATAL:service_worker_registration.cc(44)] Check failed: ::content::BrowserThread::CurrentlyOn(BrowserThread::IO). Must be called on Chrome_IOThread; actually called on Unknown Thread.
Backtrace:
	base::debug::StackTrace::StackTrace [0x07FB1081+33]
	logging::LogMessage::~LogMessage [0x07FFD9FF+63]
	content::ServiceWorkerRegistration::~ServiceWorkerRegistration [0x045639F5+293]
	content::ServiceWorkerRegistration::`scalar deleting destructor' [0x006A087A+26]
	base::RefCounted\u003Ccontent::ServiceWorkerRegistration>::Release [0x0372C3D2+114]
	scoped_refptr\u003Ccontent::ServiceWorkerRegistration>::Release [0x03D8F94E+14]

http://build.chromium.org/p/chromium.win/builders/Win8%20Aura/builds/29227/steps/content_browsertests/logs/ServiceWorkerVersionBrowserTest.ReadResourceFailure

Original issue's description:
> Evict Service Worker when reading it from disk cache fails.
>
> If reading the SW resources from the disk cache fails, we should evict
> the worker or else we will keep getting the same failure.
>
> This patch unsets the worker from the live registration, and then
> deletes the registration if the SW was the stored version.
>
> BUG=448003
>
> Committed: https://crrev.com/64e3ffb1c6b85c7d21e63afcbffe85c5a1112eb7
> Cr-Commit-Position: refs/heads/master@{#327000}

TBR=nhiroki@chromium.org,kinuko@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=448003

Review URL: https://codereview.chromium.org/1056913010

Cr-Commit-Position: refs/heads/master@{#327012}

content/browser/service_worker/service_worker_browsertest.cc

@@ -572,32 +572,6 @@ class ServiceWorkerVersionBrowserTest : public ServiceWorkerBrowserTest {
     version_->OnPingTimeout();
   }
 
-  void AddControlleeOnIOThread() {
-    ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
-    scoped_ptr<ServiceWorkerProviderHost> host(new ServiceWorkerProviderHost(
-        33 /* dummy render process id */,
-        MSG_ROUTING_NONE /* render_frame_id */, 1 /* dummy provider_id */,
-        SERVICE_WORKER_PROVIDER_FOR_WINDOW, wrapper()->context()->AsWeakPtr(),
-        NULL));
-    host->SetDocumentUrl(
-        embedded_test_server()->GetURL("/service_worker/host"));
-    host->AssociateRegistration(registration_.get(),
-                                false /* notify_controllerchange */);
-    wrapper()->context()->AddProviderHost(host.Pass());
-  }
-
-  void AddWaitingWorkerOnIOThread(const std::string& worker_url) {
-    ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
-    scoped_refptr<ServiceWorkerVersion> waiting_version(
-        new ServiceWorkerVersion(
-            registration_.get(), embedded_test_server()->GetURL(worker_url),
-            wrapper()->context()->storage()->NewVersionId(),
-            wrapper()->context()->AsWeakPtr()));
-    waiting_version->SetStatus(ServiceWorkerVersion::INSTALLED);
-    registration_->SetWaitingVersion(waiting_version.get());
-    registration_->ActivateWaitingVersionWhenReady();
-  }
-
   void StartWorker(ServiceWorkerStatusCode expected_status) {
     ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
     ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
@@ -640,7 +614,6 @@ class ServiceWorkerVersionBrowserTest : public ServiceWorkerBrowserTest {
                           ServiceWorkerStatusCode* result) {
     ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
     version_->SetStatus(ServiceWorkerVersion::ACTIVATING);
-    registration_->SetActiveVersion(version_.get());
     version_->DispatchActivateEvent(
         CreateReceiver(BrowserThread::UI, done, result));
   }
@@ -757,61 +730,6 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, StartNotFound) {
   StartWorker(SERVICE_WORKER_ERROR_NETWORK);
 }
 
-IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, ReadResourceFailure) {
-  RunOnIOThread(base::Bind(&self::SetUpRegistrationOnIOThread, this,
-                           "/service_worker/worker.js"));
-  version_->SetStatus(ServiceWorkerVersion::ACTIVATED);
-
-  // Add a non-existent resource to the version.
-  std::vector<ServiceWorkerDatabase::ResourceRecord> records;
-  records.push_back(
-      ServiceWorkerDatabase::ResourceRecord(30, version_->script_url(), 100));
-  version_->script_cache_map()->SetResources(records);
-
-  // We'll fail to read from disk and the worker should be doomed.
-  StartWorker(SERVICE_WORKER_ERROR_DISK_CACHE);
-  EXPECT_EQ(ServiceWorkerVersion::REDUNDANT, version_->status());
-
-  // The registration should be deleted since the version was the stored one.
-  EXPECT_TRUE(registration_->is_deleted());
-}
-
-IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
-                       ReadResourceFailure_WaitingWorker) {
-  RunOnIOThread(base::Bind(&self::SetUpRegistrationOnIOThread, this,
-                           "/service_worker/worker.js"));
-  base::RunLoop acrivate_run_loop;
-  ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
-  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-                          base::Bind(&self::ActivateOnIOThread, this,
-                                     acrivate_run_loop.QuitClosure(), &status));
-  acrivate_run_loop.Run();
-  ASSERT_TRUE(registration_->active_version());
-
-  // Give the version a controllee.
-  RunOnIOThread(base::Bind(&self::AddControlleeOnIOThread, this));
-
-  // Add a non-existent resource to the version.
-  std::vector<ServiceWorkerDatabase::ResourceRecord> records;
-  records.push_back(
-      ServiceWorkerDatabase::ResourceRecord(30, version_->script_url(), 100));
-  version_->script_cache_map()->SetResources(records);
-
-  // Make a waiting version.
-  RunOnIOThread(base::Bind(&self::AddWaitingWorkerOnIOThread, this,
-                           "/service_worker/worker.js"));
-
-  // Start the worker. We'll fail to read from disk and the worker should be
-  // doomed.
-  StopWorker(SERVICE_WORKER_OK);  // in case it's already running
-  StartWorker(SERVICE_WORKER_ERROR_DISK_CACHE);
-  EXPECT_EQ(ServiceWorkerVersion::REDUNDANT, version_->status());
-
-  // The registration is still alive since the waiting version was the stored
-  // one.
-  EXPECT_FALSE(registration_->is_deleted());
-}
-
 IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, Install) {
   InstallTestHelper("/service_worker/worker.js", SERVICE_WORKER_OK);
 }

content/browser/service_worker/service_worker_read_from_cache_job.cc

@@ -11,7 +11,6 @@
 #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_metrics.h"
-#include "content/public/browser/browser_thread.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_request_headers.h"
@@ -196,15 +195,7 @@ void ServiceWorkerReadFromCacheJob::SetupRangeResponse(int resource_size) {
 void ServiceWorkerReadFromCacheJob::Done(const net::URLRequestStatus& status) {
   if (!status.is_success()) {
     version_->SetStartWorkerStatusCode(SERVICE_WORKER_ERROR_DISK_CACHE);
-    // TODO(falken): Retry before evicting.
-    if (context_) {
-      ServiceWorkerRegistration* registration =
-          context_->GetLiveRegistration(version_->registration_id());
-      BrowserThread::PostTask(
-          BrowserThread::IO, FROM_HERE,
-          base::Bind(&ServiceWorkerRegistration::DeleteVersion,
-                     make_scoped_refptr(registration), version_));
-    }
+    // TODO(falken): Retry and evict the SW if it fails again.
   }
   NotifyDone(status);
 }

content/browser/service_worker/service_worker_registration.cc

@@ -302,45 +302,6 @@ void ServiceWorkerRegistration::ActivateWaitingVersion() {
                  this, activating_version));
 }
 
-void ServiceWorkerRegistration::DeleteVersion(
-    const scoped_refptr<ServiceWorkerVersion>& version) {
-  DCHECK_EQ(id(), version->registration_id());
-
-  // "Set registration's active worker to null." (The spec's step order may
-  // differ. It's OK because the other steps queue a task.)
-  UnsetVersion(version.get());
-
-  // "Run the Update State algorithm passing registration's active worker and
-  // 'redundant' as the arguments."
-  version->SetStatus(ServiceWorkerVersion::REDUNDANT);
-
-  // "For each service worker client client whose active worker is
-  // registration's active worker..." set the active worker to null.
-  for (scoped_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
-           context_->GetProviderHostIterator();
-       !it->IsAtEnd(); it->Advance()) {
-    ServiceWorkerProviderHost* host = it->GetProviderHost();
-    if (host->controlling_version() == version)
-      host->NotifyControllerActivationFailed();
-  }
-
-  version->Doom();
-
-  if (!active_version() && !waiting_version()) {
-    // Delete the records from the db.
-    context_->storage()->DeleteRegistration(
-        id(), pattern().GetOrigin(),
-        base::Bind(&ServiceWorkerRegistration::OnDeleteFinished, this));
-    // But not from memory if there is a version in the pipeline.
-    if (installing_version()) {
-      is_deleted_ = false;
-    } else {
-      is_uninstalled_ = true;
-      FOR_EACH_OBSERVER(Listener, listeners_, OnRegistrationFailed(this));
-    }
-  }
-}
-
 void ServiceWorkerRegistration::OnActivateEventFinished(
     ServiceWorkerVersion* activating_version,
     ServiceWorkerStatusCode status) {
@@ -350,7 +311,38 @@ void ServiceWorkerRegistration::OnActivateEventFinished(
 
   // "If activateFailed is true, then:..."
   if (status != SERVICE_WORKER_OK) {
-    DeleteVersion(make_scoped_refptr(activating_version));
+    // "Set registration's active worker to null." (The spec's step order may
+    // differ. It's OK because the other steps queue a task.)
+    UnsetVersion(activating_version);
+
+    // "Run the Update State algorithm passing registration's active worker and
+    // 'redundant' as the arguments."
+    activating_version->SetStatus(ServiceWorkerVersion::REDUNDANT);
+
+    // "For each service worker client client whose active worker is
+    // registration's active worker..." set the active worker to null.
+    for (scoped_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
+             context_->GetProviderHostIterator();
+         !it->IsAtEnd(); it->Advance()) {
+      ServiceWorkerProviderHost* host = it->GetProviderHost();
+      if (host->controlling_version() == activating_version)
+        host->NotifyControllerActivationFailed();
+    }
+
+    activating_version->Doom();
+    if (!waiting_version()) {
+      // Delete the records from the db.
+      context_->storage()->DeleteRegistration(
+          id(), pattern().GetOrigin(),
+          base::Bind(&ServiceWorkerRegistration::OnDeleteFinished, this));
+      // But not from memory if there is a version in the pipeline.
+      if (installing_version()) {
+        is_deleted_ = false;
+      } else {
+        is_uninstalled_ = true;
+        FOR_EACH_OBSERVER(Listener, listeners_, OnRegistrationFailed(this));
+      }
+    }
     return;
   }
 

content/browser/service_worker/service_worker_registration.h

@@ -137,10 +137,6 @@ class CONTENT_EXPORT ServiceWorkerRegistration
   void ClearUserData(const std::string& key,
                      const StatusCallback& callback);
 
-  // Unsets the version and deletes its resources. Also deletes this
-  // registration from storage if there is no longer a stored version.
-  void DeleteVersion(const scoped_refptr<ServiceWorkerVersion>& version);
-
  private:
   friend class base::RefCounted<ServiceWorkerRegistration>;