Make ServiceWorkerRegisterJob accumulate registration state.

Make ServiceWorkerRegisterJob accumulate registration state.

Refactoring. ServiceWorkerRegisterJob used to squirrel the
ServiceWorkerRegistration it is handling in a field. It's simpler to
accumulate that state in callback arguments because it is clear when
the registration object exists.

BUG=359433
TEST=ServiceWorkerJobTest.*

[dominicc (has gone to gerrit), 2014-04-03 05:31:21]

PTAL

Part one of a couple. Just refactoring first.

[kinuko, 2014-04-03 05:33:37]

Yup, lgtm!

[dominicc (has gone to gerrit), 2014-04-03 06:27:37]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2014-04-03 06:28:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominicc@chromium.org/222003010/1

[dominicc (has gone to gerrit), 2014-04-03 06:44:31]

The CQ bit was unchecked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2014-04-03 07:06:12]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2014-04-03 07:07:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominicc@chromium.org/222003010/20001

[commit-bot: I haz the power, 2014-04-03 12:09:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-03 12:09:56]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[dominicc (has gone to gerrit), 2014-04-03 23:26:56]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2014-04-03 23:27:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominicc@chromium.org/222003010/20001

[commit-bot: I haz the power, 2014-04-03 23:28:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-03 23:28:03]

Failed to apply patch for
content/browser/service_worker/service_worker_register_job.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/browser/service_worker/service_worker_register_job.cc
  Hunk #1 succeeded at 71 (offset -3 lines).
  Hunk #2 FAILED at 90.
  Hunk #3 succeeded at 132 with fuzz 2 (offset -3 lines).
  1 out of 3 hunks FAILED -- saving rejects to file
content/browser/service_worker/service_worker_register_job.cc.rej

Patch:       content/browser/service_worker/service_worker_register_job.cc
Index: content/browser/service_worker/service_worker_register_job.cc
diff --git a/content/browser/service_worker/service_worker_register_job.cc
b/content/browser/service_worker/service_worker_register_job.cc
index
1ed32a267eb6297dc4db84c21aed382a6c296994..34f09c95c61430b5bfcda781865f83ae0f0c08c5
100644
--- a/content/browser/service_worker/service_worker_register_job.cc
+++ b/content/browser/service_worker/service_worker_register_job.cc
@@ -74,7 +74,7 @@ void
ServiceWorkerRegisterJob::HandleExistingRegistrationAndContinue(
 
   if (status != SERVICE_WORKER_OK) {
     // Abort this registration job.
-    Complete(status);
+    Complete(NULL, status);
     return;
   }
 
@@ -90,41 +90,40 @@ void
ServiceWorkerRegisterJob::HandleExistingRegistrationAndContinue(
   }
 
   // Reuse the existing registration.
-  registration_ = registration;
-  StartWorkerAndContinue(SERVICE_WORKER_OK);
+  StartWorkerAndContinue(registration, SERVICE_WORKER_OK);
 }
 
 void ServiceWorkerRegisterJob::RegisterAndContinue(
     ServiceWorkerStatusCode status) {
-  DCHECK(!registration_);
   if (status != SERVICE_WORKER_OK) {
     // Abort this registration job.
-    Complete(status);
+    Complete(NULL, status);
     return;
   }
 
-  registration_ = new ServiceWorkerRegistration(
-      pattern_, script_url_, storage_->NewRegistrationId());
+  scoped_refptr<ServiceWorkerRegistration> registration =
+      new ServiceWorkerRegistration(
+          pattern_, script_url_, storage_->NewRegistrationId());
   storage_->StoreRegistration(
-      registration_.get(),
+      registration.get(),
       base::Bind(&ServiceWorkerRegisterJob::StartWorkerAndContinue,
-                 weak_factory_.GetWeakPtr()));
+                 weak_factory_.GetWeakPtr(),
+                 registration));
 }
 
 void ServiceWorkerRegisterJob::StartWorkerAndContinue(
+    ServiceWorkerRegistration* registration,
     ServiceWorkerStatusCode status) {
   // TODO(falken): Handle the case where status is an error code.
-  DCHECK(registration_);
-  if (registration_->active_version()) {
+  if (registration->active_version()) {
     // We have an active version, so we can complete immediately, even
     // if the service worker isn't running.
-    Complete(SERVICE_WORKER_OK);
+    Complete(registration, SERVICE_WORKER_OK);
     return;
   }
 
   pending_version_ = new ServiceWorkerVersion(
-      registration_, worker_registry_,
-      storage_->NewVersionId());
+      registration, worker_registry_, storage_->NewVersionId());
   for (std::vector<int>::const_iterator it = pending_process_ids_.begin();
        it != pending_process_ids_.end();
        ++it)
@@ -135,19 +134,21 @@ void ServiceWorkerRegisterJob::StartWorkerAndContinue(
   // dispatched. The job will continue to run even though the main
   // callback has executed.
   pending_version_->StartWorker(base::Bind(&ServiceWorkerRegisterJob::Complete,
-                                           weak_factory_.GetWeakPtr()));
+                                           weak_factory_.GetWeakPtr(),
+                                           make_scoped_refptr(registration)));
 
   // TODO(falken): Don't set the active version until just before
   // the activate event is dispatched.
   pending_version_->SetStatus(ServiceWorkerVersion::ACTIVE);
-  registration_->set_active_version(pending_version_);
+  registration->set_active_version(pending_version_);
 }
 
-void ServiceWorkerRegisterJob::Complete(ServiceWorkerStatusCode status) {
+void ServiceWorkerRegisterJob::Complete(ServiceWorkerRegistration*
registration,
+                                        ServiceWorkerStatusCode status) {
   for (std::vector<RegistrationCallback>::iterator it = callbacks_.begin();
        it != callbacks_.end();
        ++it) {
-    it->Run(status, registration_);
+    it->Run(status, registration);
   }
   coordinator_->FinishJob(pattern_, this);
 }

[dominicc (has gone to gerrit), 2014-04-04 02:18:40]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2014-04-04 02:18:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominicc@chromium.org/222003010/40001

[dominicc (has gone to gerrit), 2014-04-04 04:44:50]

The CQ bit was unchecked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2014-04-04 04:46:27]

Abandoned this; bikeshed the design w/ falken and we decided to go for getter
and setter w/ state machine so we can localize checks. It's not as rigorous as
passing the parameter, but it's more flexible if we add more state later and
light-weight when adding steps.