Implement memory-persistent registration

content/browser/service_worker/service_worker_register_job.h

Index: content/browser/service_worker/service_worker_register_job.h
diff --git a/content/browser/service_worker/service_worker_register_job.h b/content/browser/service_worker/service_worker_register_job.h
new file mode 100644
index 0000000000000000000000000000000000000000..34940b9772b27231fa2953eb5572bfef7565e0ef
--- /dev/null
+++ b/content/browser/service_worker/service_worker_register_job.h
@@ -0,0 +1,49 @@
+// Copyright (c) 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.
+
+#ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_REGISTER_JOB_H_
+#define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_REGISTER_JOB_H_
+
+#include "base/memory/weak_ptr.h"
+#include "content/browser/service_worker/service_worker_storage.h"
+
+namespace content {
+
+class ServiceWorkerRegisterJob {

[kinuko, 2013/11/25 08:17:10]

It feels this job's basically providing implementation details of Storage class,
am I correct?  Does this need to have its own .h/.cc file?  While our version
just operates on in-memory map I'd consider this impl detail is not very
important while storage class's interface will be more important. If we share
this assumption I feel this class's impl could be slightly more simplified while
we're sticking to in-memory thing. We'll need to add some complexities when we
really need it.

[alecflett, 2013/11/26 00:19:14]

Not exactly... There may be one or more ServiceWorkerRegisterJob operations in
flight, (thinks mulitple registration/unregistrations happening at the same
time) and a Job consists of a chain of asynchronous events. The goal of this
class is to abstract out that chain of events, so you can imagine multiple Jobs
running from a single Storage instance, and thus being able (ultimately) to
cancel, abort, or complete a job.
Well I made this in response to folks wanting a slightly less-temporary
solution. As I broke out the impl. into the storage class, I realized I had this
chain of async events (find, unregister, register, install, upgrade, activate,
etc) and needed a place to contain that process. michaeln suggested the notion
of a "job" and I think it fits the pattern well.

[kinuko, 2013/11/26 08:02:00]

Ok, the class structure (storage class starts this and owns jobs, and the
existence of job is not exposed outside the storage class) made me think like
this one's implementing internal details of storage.

I can imagine some benefits having a separate job class for registration, but
they're not too obvious to me (yet) in the current code structure. Do you have
some thoughts on: what interfaces we'll want to have for start/abort/wait a job,
who will call those interfaces, and who should own/manage the job's lifetime?

+ public:
+  typedef base::Callback<
+      void(ServiceWorkerRegisterJob*, ServiceWorkerRegistration*)>
+      RegistrationCompleteCallback;
+
+  ServiceWorkerRegisterJob(const base::WeakPtr<ServiceWorkerStorage>& storage,
+                           const RegistrationCompleteCallback& callback);
+  ~ServiceWorkerRegisterJob();
+  void StartRegister(const GURL& pattern, const GURL& script_url);
+  void StartUnregister(const GURL& pattern);
+
+ private:
+  void RegisterPatternAndContinue(
+      const GURL& pattern,
+      const GURL& script_url,
+      const ServiceWorkerStorage::ResponseCallback callback);
+
+  void UnregisterPatternAndContinue(
+      const GURL& pattern,
+      const GURL& script_url,
+      const base::Closure& callback,
+      const scoped_refptr<ServiceWorkerRegistration>& previous_registration);
+
+  void UnregisterComplete();
+  void RegisterComplete(
+      const scoped_refptr<ServiceWorkerRegistration>& registration);
+
+  const base::WeakPtr<ServiceWorkerStorage> storage_;
+  const RegistrationCompleteCallback callback_;
+  base::WeakPtrFactory<ServiceWorkerRegisterJob> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ServiceWorkerRegisterJob);
+};
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_REGISTER_JOB_H_