[ServiceWorkerCache] Serialize ServiceWorkerCacheStorage operations

There are correctness issues in the ServiceWorkerCacheStorage when
multiple operations occur on the same key in parallel. The various
steps of the operations can interleave. This CL serializes all storage
operations by placing all operations in a queue and calling the next
operation from the callback of the previous operation.

While this provides correctness, it is sub-optimal. We really only
need to serialize operations that affect the same key. A smarter
scheduler can be created in the future to implement that (see
https://crbug.com/451174).

What this CL does:

* Creates a new ServiceWorkerCacheScheduler class used by both
ServiceWorkerCache and ServiceWorkerCacheStorage
* Refactor ServiceWorkerCache to use the new scheduler
* Make ServiceWorkerCacheStorage run sequentially, using
  the scheduler

BUG=450697

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

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

content/browser/service_worker/service_worker_cache.cc

@@ -12,6 +12,7 @@
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/string_util.h"
 #include "content/browser/service_worker/service_worker_cache.pb.h"
+#include "content/browser/service_worker/service_worker_cache_scheduler.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/referrer.h"
 #include "net/base/io_buffer.h"
@@ -462,89 +463,77 @@ void ServiceWorkerCache::Put(scoped_ptr<ServiceWorkerFetchRequest> request,
             put_context->response->blob_uuid);
   }
 
-  pending_operations_.push_back(base::Bind(&ServiceWorkerCache::PutImpl,
-                                           weak_ptr_factory_.GetWeakPtr(),
-                                           base::Passed(put_context.Pass())));
-
-  if (backend_state_ == BACKEND_UNINITIALIZED) {
+  if (backend_state_ == BACKEND_UNINITIALIZED)
     InitBackend();
-    return;
-  }
 
-  RunOperationIfIdle();
+  scheduler_->ScheduleOperation(base::Bind(&ServiceWorkerCache::PutImpl,
+                                           weak_ptr_factory_.GetWeakPtr(),
+                                           base::Passed(put_context.Pass())));
 }
 
 void ServiceWorkerCache::Match(scoped_ptr<ServiceWorkerFetchRequest> request,
                                const ResponseCallback& callback) {
+  switch (backend_state_) {
+    case BACKEND_UNINITIALIZED:
+      InitBackend();
+      break;
+    case BACKEND_CLOSED:
+      callback.Run(ErrorTypeStorage, scoped_ptr<ServiceWorkerResponse>(),
+                   scoped_ptr<storage::BlobDataHandle>());
+      return;
+    case BACKEND_OPEN:
+      DCHECK(backend_);
+      break;
+  }
+
   ResponseCallback pending_callback =
       base::Bind(&ServiceWorkerCache::PendingResponseCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback);
-  pending_operations_.push_back(
+  scheduler_->ScheduleOperation(
       base::Bind(&ServiceWorkerCache::MatchImpl, weak_ptr_factory_.GetWeakPtr(),
                  base::Passed(request.Pass()), pending_callback));
+}
 
+void ServiceWorkerCache::Delete(scoped_ptr<ServiceWorkerFetchRequest> request,
+                                const ErrorCallback& callback) {
   switch (backend_state_) {
     case BACKEND_UNINITIALIZED:
       InitBackend();
-      return;
+      break;
     case BACKEND_CLOSED:
-      pending_callback.Run(ErrorTypeStorage,
-                           scoped_ptr<ServiceWorkerResponse>(),
-                           scoped_ptr<storage::BlobDataHandle>());
+      callback.Run(ErrorTypeStorage);
       return;
     case BACKEND_OPEN:
       DCHECK(backend_);
-      RunOperationIfIdle();
-      return;
+      break;
   }
-  NOTREACHED();
-}
-
-void ServiceWorkerCache::Delete(scoped_ptr<ServiceWorkerFetchRequest> request,
-                                const ErrorCallback& callback) {
   ErrorCallback pending_callback =
       base::Bind(&ServiceWorkerCache::PendingErrorCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback);
-  pending_operations_.push_back(base::Bind(
+  scheduler_->ScheduleOperation(base::Bind(
       &ServiceWorkerCache::DeleteImpl, weak_ptr_factory_.GetWeakPtr(),
       base::Passed(request.Pass()), pending_callback));
+}
 
+void ServiceWorkerCache::Keys(const RequestsCallback& callback) {
   switch (backend_state_) {
     case BACKEND_UNINITIALIZED:
       InitBackend();
-      return;
+      break;
     case BACKEND_CLOSED:
-      pending_callback.Run(ErrorTypeStorage);
+      callback.Run(ErrorTypeStorage, scoped_ptr<Requests>());
       return;
     case BACKEND_OPEN:
       DCHECK(backend_);
-      RunOperationIfIdle();
-      return;
+      break;
   }
-  NOTREACHED();
-}
 
-void ServiceWorkerCache::Keys(const RequestsCallback& callback) {
   RequestsCallback pending_callback =
       base::Bind(&ServiceWorkerCache::PendingRequestsCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback);
-  pending_operations_.push_back(base::Bind(&ServiceWorkerCache::KeysImpl,
+  scheduler_->ScheduleOperation(base::Bind(&ServiceWorkerCache::KeysImpl,
                                            weak_ptr_factory_.GetWeakPtr(),
                                            pending_callback));
-
-  switch (backend_state_) {
-    case BACKEND_UNINITIALIZED:
-      InitBackend();
-      return;
-    case BACKEND_CLOSED:
-      pending_callback.Run(ErrorTypeStorage, scoped_ptr<Requests>());
-      return;
-    case BACKEND_OPEN:
-      DCHECK(backend_);
-      RunOperationIfIdle();
-      return;
-  }
-  NOTREACHED();
 }
 
 void ServiceWorkerCache::Close(const base::Closure& callback) {
@@ -555,10 +544,9 @@ void ServiceWorkerCache::Close(const base::Closure& callback) {
       base::Bind(&ServiceWorkerCache::PendingClosure,
                  weak_ptr_factory_.GetWeakPtr(), callback);
 
-  pending_operations_.push_back(base::Bind(&ServiceWorkerCache::CloseImpl,
+  scheduler_->ScheduleOperation(base::Bind(&ServiceWorkerCache::CloseImpl,
                                            weak_ptr_factory_.GetWeakPtr(),
                                            pending_callback));
-  RunOperationIfIdle();
 }
 
 int64 ServiceWorkerCache::MemoryBackedSize() const {
@@ -601,7 +589,7 @@ ServiceWorkerCache::ServiceWorkerCache(
       quota_manager_proxy_(quota_manager_proxy),
       blob_storage_context_(blob_context),
       backend_state_(BACKEND_UNINITIALIZED),
-      operation_running_(false),
+      scheduler_(new ServiceWorkerCacheScheduler()),
       initializing_(false),
       memory_only_(path.empty()),
       weak_ptr_factory_(this) {
@@ -1205,19 +1193,16 @@ void ServiceWorkerCache::CreateBackendDidCreate(
 void ServiceWorkerCache::InitBackend() {
   DCHECK(backend_state_ == BACKEND_UNINITIALIZED);
 
-  if (initializing_) {
-    DCHECK(operation_running_);
+  if (initializing_)
     return;
-  }
 
-  DCHECK(!operation_running_);  // All ops should wait for backend init.
+  DCHECK(scheduler_->Empty());
   initializing_ = true;
-  // Note that this operation pushes to the front of the queue.
-  pending_operations_.push_front(base::Bind(
+
+  scheduler_->ScheduleOperation(base::Bind(
       &ServiceWorkerCache::CreateBackend, weak_ptr_factory_.GetWeakPtr(),
       base::Bind(&ServiceWorkerCache::InitDone,
                  weak_ptr_factory_.GetWeakPtr())));
-  RunOperationIfIdle();
 }
 
 void ServiceWorkerCache::InitDone(ErrorType error) {
@@ -1226,37 +1211,24 @@ void ServiceWorkerCache::InitDone(ErrorType error) {
                     backend_state_ == BACKEND_UNINITIALIZED)
                        ? BACKEND_OPEN
                        : BACKEND_CLOSED;
-  CompleteOperationAndRunNext();
-}
-
-void ServiceWorkerCache::CompleteOperationAndRunNext() {
-  DCHECK(!pending_operations_.empty());
-  operation_running_ = false;
-  pending_operations_.pop_front();
-  RunOperationIfIdle();
-}
-
-void ServiceWorkerCache::RunOperationIfIdle() {
-  DCHECK(!operation_running_ || !pending_operations_.empty());
-
-  if (!operation_running_ && !pending_operations_.empty()) {
-    operation_running_ = true;
-    // TODO(jkarlin): Run multiple operations in parallel where allowed (e.g.,
-    // if they're for different keys then they won't interfere). See
-    // https://crbug.com/451174.
-    pending_operations_.front().Run();
-  }
+  scheduler_->CompleteOperationAndRunNext();
 }
 
 void ServiceWorkerCache::PendingClosure(const base::Closure& callback) {
+  base::WeakPtr<ServiceWorkerCache> cache = weak_ptr_factory_.GetWeakPtr();
+
   callback.Run();
-  CompleteOperationAndRunNext();
+  if (cache)
+    scheduler_->CompleteOperationAndRunNext();
 }
 
 void ServiceWorkerCache::PendingErrorCallback(const ErrorCallback& callback,
                                               ErrorType error) {
+  base::WeakPtr<ServiceWorkerCache> cache = weak_ptr_factory_.GetWeakPtr();
+
   callback.Run(error);
-  CompleteOperationAndRunNext();
+  if (cache)
+    scheduler_->CompleteOperationAndRunNext();
 }
 
 void ServiceWorkerCache::PendingResponseCallback(
@@ -1264,16 +1236,22 @@ void ServiceWorkerCache::PendingResponseCallback(
     ErrorType error,
     scoped_ptr<ServiceWorkerResponse> response,
     scoped_ptr<storage::BlobDataHandle> blob_data_handle) {
+  base::WeakPtr<ServiceWorkerCache> cache = weak_ptr_factory_.GetWeakPtr();
+
   callback.Run(error, response.Pass(), blob_data_handle.Pass());
-  CompleteOperationAndRunNext();
+  if (cache)
+    scheduler_->CompleteOperationAndRunNext();
 }
 
 void ServiceWorkerCache::PendingRequestsCallback(
     const RequestsCallback& callback,
     ErrorType error,
     scoped_ptr<Requests> requests) {
+  base::WeakPtr<ServiceWorkerCache> cache = weak_ptr_factory_.GetWeakPtr();
+
   callback.Run(error, requests.Pass());
-  CompleteOperationAndRunNext();
+  if (cache)
+    scheduler_->CompleteOperationAndRunNext();
 }
 
 }  // namespace content

content/browser/service_worker/service_worker_cache.h

@@ -30,6 +30,7 @@ class QuotaManagerProxy;
 namespace content {
 class ChromeBlobStorageContext;
 class ServiceWorkerCacheMetadata;
+class ServiceWorkerCacheScheduler;
 class TestServiceWorkerCache;
 
 // Represents a ServiceWorker Cache as seen in
@@ -182,8 +183,6 @@ class CONTENT_EXPORT ServiceWorkerCache
   void InitBackend();
   void InitDone(ErrorType error);
 
-  void CompleteOperationAndRunNext();
-  void RunOperationIfIdle();
   void PendingClosure(const base::Closure& callback);
   void PendingErrorCallback(const ErrorCallback& callback, ErrorType error);
   void PendingResponseCallback(
@@ -204,8 +203,7 @@ class CONTENT_EXPORT ServiceWorkerCache
   scoped_refptr<storage::QuotaManagerProxy> quota_manager_proxy_;
   base::WeakPtr<storage::BlobStorageContext> blob_storage_context_;
   BackendState backend_state_;
-  std::list<base::Closure> pending_operations_;
-  bool operation_running_;
+  scoped_ptr<ServiceWorkerCacheScheduler> scheduler_;
   bool initializing_;
 
   // Whether or not to store data in disk or memory.

content/browser/service_worker/service_worker_cache_scheduler.cc

+// Copyright 2015 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_cache_scheduler.h"
+
+#include <string>
+
+#include "base/logging.h"
+
+namespace content {
+
+ServiceWorkerCacheScheduler::ServiceWorkerCacheScheduler()
+    : operation_running_(false) {
+}
+
+ServiceWorkerCacheScheduler::~ServiceWorkerCacheScheduler() {
+}
+
+void ServiceWorkerCacheScheduler::ScheduleOperation(
+    const base::Closure& closure) {
+  pending_operations_.push_back(closure);
+  RunOperationIfIdle();
+}
+
+void ServiceWorkerCacheScheduler::CompleteOperationAndRunNext() {
+  DCHECK(!pending_operations_.empty());
+  operation_running_ = false;
+  pending_operations_.pop_front();
+  RunOperationIfIdle();
+}
+
+bool ServiceWorkerCacheScheduler::Empty() const {
+  return pending_operations_.empty();
+}
+
+void ServiceWorkerCacheScheduler::RunOperationIfIdle() {
+  DCHECK(!operation_running_ || !pending_operations_.empty());
+
+  if (!operation_running_ && !pending_operations_.empty()) {
+    operation_running_ = true;
+    // TODO(jkarlin): Run multiple operations in parallel where allowed.
+    pending_operations_.front().Run();
+  }
+}
+
+}  // namespace content

content/browser/service_worker/service_worker_cache_scheduler.h

+// Copyright 2015 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_CACHE_SCHEDULER_H_
+#define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_CACHE_SCHEDULER_H_
+
+#include <list>
+
+#include "base/callback.h"
+#include "content/common/content_export.h"
+
+namespace content {
+
+class CONTENT_EXPORT ServiceWorkerCacheScheduler {
+ public:
+  ServiceWorkerCacheScheduler();
+  virtual ~ServiceWorkerCacheScheduler();
+
+  // Adds the operation to the tail of the queue and starts it if the scheduler
+  // is idle.
+  void ScheduleOperation(const base::Closure& closure);
+
+  // Call this after each operation completes. It cleans up the current
+  // operation and starts the next.
+  void CompleteOperationAndRunNext();
+
+  bool Empty() const;
+
+ private:
+  void RunOperationIfIdle();
+
+  // The list of operations waiting on initialization.
+  std::list<base::Closure> pending_operations_;
+  bool operation_running_;
+
+  DISALLOW_COPY_AND_ASSIGN(ServiceWorkerCacheScheduler);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_CACHE_SCHEDULER_H_

content/browser/service_worker/service_worker_cache_scheduler_unittest.cc

+// Copyright 2015 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_cache_scheduler.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/run_loop.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+
+class JobStats {
+ public:
+  JobStats(ServiceWorkerCacheScheduler* scheduler)
+      : scheduler_(scheduler), callback_count_(0) {}
+
+  virtual void Run() = 0;
+
+  int callback_count() const { return callback_count_; }
+
+ protected:
+  ServiceWorkerCacheScheduler* scheduler_;
+  int callback_count_;
+};
+
+class SyncJob : public JobStats {
+ public:
+  SyncJob(ServiceWorkerCacheScheduler* scheduler) : JobStats(scheduler) {}
+
+  void Run() override {
+    callback_count_++;
+    scheduler_->CompleteOperationAndRunNext();
+  }
+};
+
+class AsyncJob : public JobStats {
+ public:
+  AsyncJob(ServiceWorkerCacheScheduler* scheduler) : JobStats(scheduler) {}
+
+  void Run() override { callback_count_++; }
+  void Done() { scheduler_->CompleteOperationAndRunNext(); }
+};
+
+}  // namespace
+
+class ServiceWorkerCacheSchedulerTest : public testing::Test {
+ protected:
+  ServiceWorkerCacheSchedulerTest()
+      : async_job_(AsyncJob(&scheduler_)), sync_job_(SyncJob(&scheduler_)) {}
+
+  ServiceWorkerCacheScheduler scheduler_;
+  AsyncJob async_job_;
+  SyncJob sync_job_;
+};
+
+TEST_F(ServiceWorkerCacheSchedulerTest, ScheduleOne) {
+  scheduler_.ScheduleOperation(
+      base::Bind(&JobStats::Run, base::Unretained(&sync_job_)));
+  EXPECT_EQ(1, sync_job_.callback_count());
+}
+
+TEST_F(ServiceWorkerCacheSchedulerTest, ScheduleTwo) {
+  scheduler_.ScheduleOperation(
+      base::Bind(&JobStats::Run, base::Unretained(&sync_job_)));
+  scheduler_.ScheduleOperation(
+      base::Bind(&JobStats::Run, base::Unretained(&sync_job_)));
+  EXPECT_EQ(2, sync_job_.callback_count());
+}
+
+TEST_F(ServiceWorkerCacheSchedulerTest, Block) {
+  scheduler_.ScheduleOperation(
+      base::Bind(&JobStats::Run, base::Unretained(&async_job_)));
+  EXPECT_EQ(1, async_job_.callback_count());
+  scheduler_.ScheduleOperation(
+      base::Bind(&JobStats::Run, base::Unretained(&sync_job_)));
+  EXPECT_EQ(0, sync_job_.callback_count());
+  async_job_.Done();
+  EXPECT_EQ(1, sync_job_.callback_count());
+}
+
+}  // namespace content

content/browser/service_worker/service_worker_cache_storage.h

@@ -26,12 +26,13 @@ class BlobStorageContext;
 }
 
 namespace content {
+class ServiceWorkerCacheScheduler;
 
 // TODO(jkarlin): Constrain the total bytes used per origin.
 
 // ServiceWorkerCacheStorage holds the set of caches for a given origin. It is
 // owned by the ServiceWorkerCacheStorageManager. This class expects to be run
-// on the IO thread.
+// on the IO thread. The asynchronous methods are executed serially.
 class CONTENT_EXPORT ServiceWorkerCacheStorage {
  public:
   enum CacheStorageError {
@@ -94,12 +95,19 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
   void MatchAllCaches(scoped_ptr<ServiceWorkerFetchRequest> request,
                       const ServiceWorkerCache::ResponseCallback& callback);
 
+  // Calls close on each cache and runs the callback after all of them have
+  // closed.
   void CloseAllCaches(const base::Closure& callback);
 
   // The size of all of the origin's contents in memory. Returns 0 if the cache
-  // backend is not a memory backend.
+  // backend is not a memory backend. Runs synchronously.
   int64 MemoryBackedSize() const;
 
+  // The functions below are for tests to verify that the operations run
+  // serially.
+  void StartAsyncOperationForTesting();
+  void CompleteAsyncOperationForTesting();
+
  private:
   class MemoryLoader;
   class SimpleCacheLoader;
@@ -112,17 +120,15 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
   scoped_refptr<ServiceWorkerCache> GetLoadedCache(
       const std::string& cache_name);
 
-  // Initializer and its callback are below. While LazyInit is running any new
-  // operations will be queued and started in order after initialization.
-  void LazyInit(const base::Closure& closure);
+  // Initializer and its callback are below.
+  void LazyInit();
+  void LazyInitImpl();
   void LazyInitDidLoadIndex(
-      const base::Closure& callback,
       scoped_ptr<std::vector<std::string> > indexed_cache_names);
 
-  void AddCacheToMap(const std::string& cache_name,
-                     base::WeakPtr<ServiceWorkerCache> cache);
-
-  // The CreateCache callbacks are below.
+  // The Open and CreateCache callbacks are below.
+  void OpenCacheImpl(const std::string& cache_name,
+                     const CacheAndErrorCallback& callback);
   void CreateCacheDidCreateCache(
       const std::string& cache_name,
       const CacheAndErrorCallback& callback,
@@ -131,7 +137,14 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
                                 const scoped_refptr<ServiceWorkerCache>& cache,
                                 bool success);
 
+  // The HasCache callbacks are below.
+  void HasCacheImpl(const std::string& cache_name,
+                    const BoolAndErrorCallback& callback);
+
   // The DeleteCache callbacks are below.
+  void DeleteCacheImpl(const std::string& cache_name,
+                       const BoolAndErrorCallback& callback);
+
   void DeleteCacheDidClose(const std::string& cache_name,
                            const BoolAndErrorCallback& callback,
                            const StringVector& ordered_cache_names,
@@ -142,7 +155,13 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
   void DeleteCacheDidCleanUp(const BoolAndErrorCallback& callback,
                              bool success);
 
+  // The EnumerateCache callbacks are below.
+  void EnumerateCachesImpl(const StringsAndErrorCallback& callback);
+
   // The MatchCache callbacks are below.
+  void MatchCacheImpl(const std::string& cache_name,
+                      scoped_ptr<ServiceWorkerFetchRequest> request,
+                      const ServiceWorkerCache::ResponseCallback& callback);
   void MatchCacheDidMatch(const scoped_refptr<ServiceWorkerCache>& cache,
                           const ServiceWorkerCache::ResponseCallback& callback,
                           ServiceWorkerCache::ErrorType error,
@@ -150,6 +169,8 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
                           scoped_ptr<storage::BlobDataHandle> handle);
 
   // The MatchAllCaches callbacks are below.
+  void MatchAllCachesImpl(scoped_ptr<ServiceWorkerFetchRequest> request,
+                          const ServiceWorkerCache::ResponseCallback& callback);
   void MatchAllCachesDidMatch(scoped_refptr<ServiceWorkerCache> cache,
                               const base::Closure& barrier_closure,
                               ServiceWorkerCache::ResponseCallback* callback,
@@ -159,11 +180,32 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
   void MatchAllCachesDidMatchAll(
       scoped_ptr<ServiceWorkerCache::ResponseCallback> callback);
 
+  // The CloseAllCaches callbacks are below.
+  void CloseAllCachesImpl(const base::Closure& callback);
+
+  void PendingClosure(const base::Closure& callback);
+  void PendingBoolAndErrorCallback(const BoolAndErrorCallback& callback,
+                                   bool found,
+                                   CacheStorageError error);
+  void PendingCacheAndErrorCallback(
+      const CacheAndErrorCallback& callback,
+      const scoped_refptr<ServiceWorkerCache>& cache,
+      CacheStorageError error);
+  void PendingStringsAndErrorCallback(const StringsAndErrorCallback& callback,
+                                      const StringVector& strings,
+                                      CacheStorageError error);
+  void PendingResponseCallback(
+      const ServiceWorkerCache::ResponseCallback& callback,
+      ServiceWorkerCache::ErrorType error,
+      scoped_ptr<ServiceWorkerResponse> response,
+      scoped_ptr<storage::BlobDataHandle> blob_data_handle);
+
   // Whether or not we've loaded the list of cache names into memory.
   bool initialized_;
+  bool initializing_;
 
-  // The list of operations waiting on initialization.
-  std::vector<base::Closure> init_callbacks_;
+  // The pending operation scheduler.
+  scoped_ptr<ServiceWorkerCacheScheduler> scheduler_;
 
   // The map of cache names to ServiceWorkerCache objects.
   CacheMap cache_map_;

content/browser/service_worker/service_worker_cache_storage_manager_unittest.cc

@@ -22,7 +22,7 @@
 namespace content {
 
 class ServiceWorkerCacheStorageManagerTest : public testing::Test {
- protected:
+ public:
   ServiceWorkerCacheStorageManagerTest()
       : browser_thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP),
         callback_bool_(false),
@@ -424,13 +424,13 @@ TEST_P(ServiceWorkerCacheStorageManagerTestP, StorageMatchInOneOfMany) {
 }
 
 TEST_P(ServiceWorkerCacheStorageManagerTestP, Chinese) {
-  EXPECT_TRUE(Open(origin1_, "������"));
+  EXPECT_TRUE(Open(origin1_, "你好"));
   scoped_refptr<ServiceWorkerCache> cache = callback_cache_;
-  EXPECT_TRUE(Open(origin1_, "������"));
+  EXPECT_TRUE(Open(origin1_, "你好"));
   EXPECT_EQ(callback_cache_.get(), cache.get());
   EXPECT_TRUE(Keys(origin1_));
   EXPECT_EQ(1u, callback_strings_.size());
-  EXPECT_STREQ("������", callback_strings_[0].c_str());
+  EXPECT_STREQ("你好", callback_strings_[0].c_str());
 }
 
 TEST_F(ServiceWorkerCacheStorageManagerTest, EmptyKey) {
@@ -522,6 +522,25 @@ TEST_P(ServiceWorkerCacheStorageManagerTestP, DeleteBeforeRelease) {
   EXPECT_TRUE(callback_cache_->AsWeakPtr());
 }
 
+TEST_P(ServiceWorkerCacheStorageManagerTestP, OpenRunsSerially) {
+  EXPECT_FALSE(Delete(origin1_, "tmp"));  // Init storage.
+  ServiceWorkerCacheStorage* cache_storage = CacheStorageForOrigin(origin1_);
+  cache_storage->StartAsyncOperationForTesting();
+
+  scoped_ptr<base::RunLoop> open_loop(new base::RunLoop());
+  cache_manager_->OpenCache(
+      origin1_, "foo",
+      base::Bind(&ServiceWorkerCacheStorageManagerTest::CacheAndErrorCallback,
+                 base::Unretained(this), base::Unretained(open_loop.get())));
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(callback_cache_);
+
+  cache_storage->CompleteAsyncOperationForTesting();
+  open_loop->Run();
+  EXPECT_TRUE(callback_cache_);
+}
+
 TEST_F(ServiceWorkerCacheStorageManagerMemoryOnlyTest, MemoryBackedSize) {
   ServiceWorkerCacheStorage* cache_storage = CacheStorageForOrigin(origin1_);
   EXPECT_EQ(0, cache_storage->MemoryBackedSize());

content/content_browser.gypi

@@ -1222,6 +1222,8 @@
       'browser/service_worker/service_worker_cache_listener.h',
       'browser/service_worker/service_worker_cache_quota_client.cc',
       'browser/service_worker/service_worker_cache_quota_client.h',
+      'browser/service_worker/service_worker_cache_scheduler.cc',
+      'browser/service_worker/service_worker_cache_scheduler.h',
       'browser/service_worker/service_worker_cache_storage.cc',
       'browser/service_worker/service_worker_cache_storage.h',
       'browser/service_worker/service_worker_cache_storage_manager.cc',

content/content_tests.gypi

@@ -520,8 +520,9 @@
       'browser/service_worker/embedded_worker_instance_unittest.cc',
       'browser/service_worker/embedded_worker_test_helper.cc',
       'browser/service_worker/embedded_worker_test_helper.h',
-      'browser/service_worker/service_worker_cache_unittest.cc',
+      'browser/service_worker/service_worker_cache_scheduler_unittest.cc',
       'browser/service_worker/service_worker_cache_storage_manager_unittest.cc',
+      'browser/service_worker/service_worker_cache_unittest.cc',
       'browser/service_worker/service_worker_context_unittest.cc',
       'browser/service_worker/service_worker_controllee_request_handler_unittest.cc',
       'browser/service_worker/service_worker_context_request_handler_unittest.cc',