Implement memory-persistent registration

content/browser/service_worker/service_worker_storage.cc

+// Copyright 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.
+
+#include "content/browser/service_worker/service_worker_storage.h"
+
+#include <string>
+
+#include "base/strings/string_util.h"
+#include "content/browser/service_worker/service_worker_register_job.h"
+#include "content/browser/service_worker/service_worker_registration.h"
+#include "content/public/browser/browser_thread.h"
+#include "webkit/browser/quota/quota_manager.h"
+
+namespace {
+// This is temporary until we figure out how registration ids will be
+// calculated.
+int64 NextRegistrationId() {
+  static int64 worker_id = 0;
+  return worker_id++;
+}
+}  // namespace
+
+namespace content {
+
+const base::FilePath::CharType kServiceWorkerDirectory[] =
+    FILE_PATH_LITERAL("ServiceWorker");
+
+ServiceWorkerStorage::ServiceWorkerStorage(
+    const base::FilePath& path,
+    quota::QuotaManagerProxy* quota_manager_proxy)
+    : quota_manager_proxy_(quota_manager_proxy), weak_factory_(this) {
+  if (!path.empty())
+    path_ = path.Append(kServiceWorkerDirectory);
+}
+
+ServiceWorkerStorage::~ServiceWorkerStorage() {}
+
+void ServiceWorkerStorage::Shutdown() {
+  for (PatternToRegistrationMap::const_iterator iter =
+           registration_by_pattern_.begin();
+       iter != registration_by_pattern_.end();
+       ++iter) {
+    iter->second->Shutdown();
+  }
+  registration_by_pattern_.clear();
+}
+
+void ServiceWorkerStorage::FindRegistrationForPattern(
+    const GURL& pattern,
+    const RegistrationCallback& callback) {
+  PatternToRegistrationMap::const_iterator match =
+      registration_by_pattern_.find(pattern);
+  if (match == registration_by_pattern_.end()) {
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(callback,
+                   REGISTRATION_NOT_FOUND,
+                   scoped_refptr<ServiceWorkerRegistration>()));
+    return;
+  }
+  BrowserThread::PostTask(BrowserThread::IO,
+                          FROM_HERE,
+                          base::Bind(callback, REGISTRATION_OK, match->second));
+}
+
+void ServiceWorkerStorage::FindRegistrationForDocument(
+    const GURL& document_url,
+    const RegistrationCallback& callback) {
+  // TODO(alecflett): This needs to be synchronous in the fast path,
+  // but asynchronous in the slow path (when the patterns have to be
+  // loaded from disk). For now it is always pessimistically async.
+  for (PatternToRegistrationMap::const_iterator it =
+           registration_by_pattern_.begin();
+       it != registration_by_pattern_.end();
+       ++it) {
+    if (PatternMatches(it->first, document_url)) {
+      BrowserThread::PostTask(
+          BrowserThread::IO,
+          FROM_HERE,
+          base::Bind(callback,
+                     REGISTRATION_OK,
+                     scoped_refptr<ServiceWorkerRegistration>(it->second)));
+      return;
+    }
+  }
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(callback,
+                 REGISTRATION_NOT_FOUND,
+                 scoped_refptr<ServiceWorkerRegistration>()));
+}
+
+void ServiceWorkerStorage::Register(const GURL& pattern,
+                                    const GURL& script_url,
+                                    const RegistrationCallback& callback) {
+  scoped_ptr<ServiceWorkerRegisterJob> job(new ServiceWorkerRegisterJob(
+      weak_factory_.GetWeakPtr(),
+      base::Bind(&ServiceWorkerStorage::RegisterComplete,
+                 weak_factory_.GetWeakPtr(),
+                 callback)));
+  job->StartRegister(pattern, script_url);
+  registration_jobs_.push_back(job.release());

[kinuko, 2013/12/04 13:04:29]

We use multiple async calls to FindRegistration and Unregistration, or Find +
Unregister + Register.
How do you plan to make sure multiple registration and unregistration requests
don't interleave each other?  I imagine we'll probably need some serialization,
and for storage-related atomic operations we may want to just perform all in a
single task.  For the latter reason I feel storage methods for register &
unregister could be a single async method (it could internally call common
methods)... wdyt?

[alecflett, 2013/12/06 05:43:33]

ultimately registration_jobs_ will evolve to handle this - I'm thinking it will
become a map of pattern -> list of ServiceWorkerRegisterJob instances that are
in process. This will also allow things like the ability for an unregister() to
cancel an in-process register().
Well the approach I'm taking is to let them share a common class but let the
Start* be the point where we diverge. In its current form this scaffolding may
seem excessive, but I think down the road it will make these "Register" and
"Unregister" methods simpler, (because I think StartRegister and StartUnregister
are going to diverge further) and the Start* is what sets up the chain of async
operations. If it turns out later to be a lot of duplicated code between
Register and Unregister, I'm happy to merge them.

[kinuko, 2013/12/06 15:24:49]

Ah, my comment was probably not clear enough, I'm not saying that we should
merge Register and Unregister, but wondering if we should have one (or fewer)
atomic async method for Register and another for Unregister.  Of course they
could call common private methods internally when necessary (like currently we
do in this patch), but they may not need to be in a series of multiple methods? 
For example in the current model we're supposed to make two async calls to
'Find' and 'UnregisterInternal' to unregister, but when we think about how we'll
be interacting with the actual db this model sounds a bit weird, since we need
to do always 'Find' the registration to 'Unregister' things (e.g. like we do
registration_by_pattern_.find in both FindRegistrationForPattern and
UnregisterInternal), this feels a bit wasteful.  And because they're supposed to
be called asynchronously another 'RegisterInternal' or 'UnregisterInternal'
could easily interleave between the 'Find' and 'Unregister'.

To be more specific, I think, say, having a separate 'Find' method itself
totally makes sense as it'd make testing much easier, and I understand that the
atomicity problem can be addressed at the bigger 'job' level as you mention, but
I'm still feeling that some of them are probably not need to be separate *async*
methods, and having simpler method model would also make the storage model
simple and robust.

+}
+
+void ServiceWorkerStorage::Unregister(const GURL& pattern,
+                                      const UnregistrationCallback& callback) {
+  scoped_ptr<ServiceWorkerRegisterJob> job(new ServiceWorkerRegisterJob(
+      weak_factory_.GetWeakPtr(),
+      base::Bind(&ServiceWorkerStorage::UnregisterComplete,
+                 weak_factory_.GetWeakPtr(),
+                 callback)));
+  job->StartUnregister(pattern);
+  registration_jobs_.push_back(job.release());
+}
+
+scoped_refptr<ServiceWorkerRegistration> ServiceWorkerStorage::RegisterInternal(
+    const GURL& pattern,
+    const GURL& script_url) {
+
+  PatternToRegistrationMap::const_iterator current(
+      registration_by_pattern_.find(pattern));
+  DCHECK(current == registration_by_pattern_.end() ||
+         current->second->script_url() == script_url);
+
+  if (current == registration_by_pattern_.end()) {
+    scoped_refptr<ServiceWorkerRegistration> registration(
+        new ServiceWorkerRegistration(
+            pattern, script_url, NextRegistrationId()));
+    // TODO(alecflett): version upgrade path.
+    registration_by_pattern_[pattern] = registration;
+    return registration;
+  }
+
+  return current->second;
+}
+
+void ServiceWorkerStorage::UnregisterInternal(const GURL& pattern) {
+  PatternToRegistrationMap::const_iterator match =
+      registration_by_pattern_.find(pattern);
+  if (match != registration_by_pattern_.end()) {
+    match->second->Shutdown();
+    registration_by_pattern_.erase(match);
+  }
+}
+
+bool ServiceWorkerStorage::PatternMatches(const GURL& pattern,
+                                          const GURL& url) {
+  // This is a really basic, naive
+  // TODO(alecflett): Formalize what pattern matches mean.
+  // Temporarily borrowed directly from appcache::Namespace::IsMatch().
+  // We have to escape '?' characters since MatchPattern also treats those
+  // as wildcards which we don't want here, we only do '*'s.
+  std::string pattern_spec(pattern.spec());
+  if (pattern.has_query())
+    ReplaceSubstringsAfterOffset(&pattern_spec, 0, "?", "\\?");
+  return MatchPattern(url.spec(), pattern_spec);
+}
+
+void ServiceWorkerStorage::EraseJob(ServiceWorkerRegisterJob* job) {
+  ScopedVector<ServiceWorkerRegisterJob>::iterator job_position =
+      registration_jobs_.begin();
+  bool found = false;
+  for (; job_position != registration_jobs_.end(); ++job_position) {
+    if (*job_position == job) {
+      found = true;
+      registration_jobs_.erase(job_position);
+      return;
+    }
+  }
+  NOTREACHED() << "Deleting non-existent job. ";
+}
+
+void ServiceWorkerStorage::UnregisterComplete(
+    const UnregistrationCallback& callback,
+    ServiceWorkerRegisterJob* job,
+    ServiceWorkerRegistrationStatus status,
+    ServiceWorkerRegistration* previous_registration) {
+  callback.Run(status);
+  EraseJob(job);
+}
+
+void ServiceWorkerStorage::RegisterComplete(
+    const RegistrationCallback& callback,
+    ServiceWorkerRegisterJob* job,
+    ServiceWorkerRegistrationStatus status,
+    ServiceWorkerRegistration* registration) {
+  callback.Run(status, registration);
+  EraseJob(job);
+}
+
+}  // namespace content