Fix crash in ServiceWorkerRegistrationHandle

Before this patch, when register() fails to install a worker, it would
result in ServiceWorkerRegistrationHandle::OnRegisterFailed nulling out
its reference to the registration in order to allow the registration
to die. This was flawed for two reasons:
- When the handle is destructed, it would dereference the null pointer.
- SWRegistrationHandle is responsible for listening for ref count changes from
Blink-side about the corresponding JavaScript ServiceWorkerRegistration object.
It's a weird state for Handle to forget about the Chromium-side registration
while Blink-side is alive. The Chromium-side Registration lifetime should be
tied to Blink's.

BUG=384119, 396400

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

Cr-Commit-Position: refs/heads/master@{#289597}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289597 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/service_worker/service_worker_registration_handle.cc

@@ -33,6 +33,7 @@ ServiceWorkerRegistrationHandle::ServiceWorkerRegistrationHandle(
 }
 
 ServiceWorkerRegistrationHandle::~ServiceWorkerRegistrationHandle() {
+  DCHECK(registration_);
   registration_->RemoveListener(this);
 }
 
@@ -60,8 +61,6 @@ void ServiceWorkerRegistrationHandle::OnRegistrationFailed(
     ServiceWorkerRegistration* registration) {
   DCHECK_EQ(registration->id(), registration_->id());
   ClearVersionAttributes();
-  registration_->RemoveListener(this);
-  registration_ = NULL;
 }
 
 void ServiceWorkerRegistrationHandle::SetVersionAttributes(

content/browser/service_worker/service_worker_registration_handle.h

@@ -26,7 +26,7 @@ class ServiceWorkerVersion;
 class ServiceWorkerRegistrationHandle
     : public ServiceWorkerRegistration::Listener {
  public:
-  ServiceWorkerRegistrationHandle(
+  CONTENT_EXPORT ServiceWorkerRegistrationHandle(
       base::WeakPtr<ServiceWorkerContextCore> context,
       ServiceWorkerDispatcherHost* dispatcher_host,
       int provider_id,

content/browser/service_worker/service_worker_registration_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/run_loop.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
+#include "content/browser/service_worker/service_worker_registration_handle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
@@ -146,4 +147,24 @@ TEST_F(ServiceWorkerRegistrationTest, SetAndUnsetVersions) {
   EXPECT_TRUE(listener.observed_info_.controlling_version.is_null);
 }
 
+TEST_F(ServiceWorkerRegistrationTest, FailedRegistrationNoCrash) {
+  const GURL kScope("http://www.example.not/");
+  const GURL kScript("http://www.example.not/service_worker.js");
+  int64 kRegistrationId = 1L;
+  int kProviderId = 1;
+  scoped_refptr<ServiceWorkerRegistration> registration =
+      new ServiceWorkerRegistration(
+          kScope,
+          kScript,
+          kRegistrationId,
+          context_ptr_);
+  scoped_ptr<ServiceWorkerRegistrationHandle> handle(
+      new ServiceWorkerRegistrationHandle(context_ptr_,
+                                          NULL,
+                                          kProviderId,
+                                          registration.get()));
+  registration->NotifyRegistrationFailed();
+  // Don't crash when handle gets destructed.
+}
+
 }  // namespace content