Revert of ServiceWorker: Initialize ServiceWorkerStorage with ServiceWorkerDatabase (https://codereview.chromium.org/268453002/)

Reason for revert:
Breaking "Linux ASan LSan Tests (2)":
http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Tests%20%282%29/builds/2378

Original issue's description:
> ServiceWorker: Initialize ServiceWorkerStorage with ServiceWorkerDatabase
> 
> This patch implements initial wiring part ServiceWorkerStorage and
> ServiceWorkerDatabase.
> 
> - The storage owns the database that lives on dedicated task runner.
> - The storage queries the database about available ids and registered origins
> on initialization.
> - The storage gets disabled when database operation fails.
> - Fixing some bugs of the database detected by tests after wiring.
> 
> 
> BUG=369464
> TEST=content_unittests --gtest_filter=ServiceWorker*
> R=michaeln@chromium.org
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268691

TBR=michaeln@chromium.org,serviceworker-reviews@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=369464

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@268700 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 50 insertions, 220 deletions

diff --git a/content/browser/service_worker/service_worker_database.cc b/content/browser/service_worker/service_worker_database.cc
index 588c4d7..18e7102 100644
--- a/content/browser/service_worker/service_worker_database.cc
+++ b/content/browser/service_worker/service_worker_database.cc
@@ -231,7 +231,6 @@ ServiceWorkerDatabase::ServiceWorkerDatabase(const base::FilePath& path)
       is_disabled_(false),
       was_corruption_detected_(false),
       is_initialized_(false) {
-  sequence_checker_.DetachFromSequence();
 }
 
 ServiceWorkerDatabase::~ServiceWorkerDatabase() {
@@ -248,15 +247,8 @@ bool ServiceWorkerDatabase::GetNextAvailableIds(
   DCHECK(next_avail_version_id);
   DCHECK(next_avail_resource_id);
 
-  if (!LazyOpen(false)) {
-    if (is_disabled_)
-      return false;
-    // Database has never been used.
-    *next_avail_registration_id = 0;
-    *next_avail_version_id = 0;
-    *next_avail_resource_id = 0;
-    return true;
-  }
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
 
   if (!ReadNextAvailableId(kNextRegIdKey, &next_avail_registration_id_) ||
       !ReadNextAvailableId(kNextVerIdKey, &next_avail_version_id_) ||
@@ -275,13 +267,8 @@ bool ServiceWorkerDatabase::GetOriginsWithRegistrations(
   DCHECK(sequence_checker_.CalledOnValidSequencedThread());
   DCHECK(origins);
 
-  if (!LazyOpen(false)) {
-    if (is_disabled_)
-      return false;
-    // Database has never been used.
-    origins->clear();
-    return true;
-  }
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
 
   scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
   for (itr->Seek(kUniqueOriginKey); itr->Valid(); itr->Next()) {
diff --git a/content/browser/service_worker/service_worker_database_unittest.cc b/content/browser/service_worker/service_worker_database_unittest.cc
index ae8f2d6..b0cae286 100644
--- a/content/browser/service_worker/service_worker_database_unittest.cc
+++ b/content/browser/service_worker/service_worker_database_unittest.cc
@@ -139,12 +139,9 @@ TEST(ServiceWorkerDatabaseTest, GetNextAvailableIds) {
       CreateDatabase(database_dir.path()));
   AvailableIds ids;
 
-  // The database has never been used, so returns initial values.
-  EXPECT_TRUE(database->GetNextAvailableIds(
+  // Should be false because the database hasn't been opened.
+  EXPECT_FALSE(database->GetNextAvailableIds(
       &ids.reg_id, &ids.ver_id, &ids.res_id));
-  EXPECT_EQ(0, ids.reg_id);
-  EXPECT_EQ(0, ids.ver_id);
-  EXPECT_EQ(0, ids.res_id);
 
   ASSERT_TRUE(database->LazyOpen(true));
   EXPECT_TRUE(database->GetNextAvailableIds(
@@ -191,7 +188,7 @@ TEST(ServiceWorkerDatabaseTest, GetOriginsWithRegistrations) {
   scoped_ptr<ServiceWorkerDatabase> database(CreateDatabaseInMemory());
 
   std::set<GURL> origins;
-  EXPECT_TRUE(database->GetOriginsWithRegistrations(&origins));
+  EXPECT_FALSE(database->GetOriginsWithRegistrations(&origins));
   EXPECT_TRUE(origins.empty());
 
   std::vector<Resource> resources;
diff --git a/content/browser/service_worker/service_worker_storage.cc b/content/browser/service_worker/service_worker_storage.cc
index 607d146..07addb2 100644
--- a/content/browser/service_worker/service_worker_storage.cc
+++ b/content/browser/service_worker/service_worker_storage.cc
@@ -6,17 +6,14 @@
 
 #include <string>
 
-#include "base/bind_helpers.h"
 #include "base/message_loop/message_loop.h"
 #include "base/sequenced_task_runner.h"
-#include "base/task_runner_util.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_disk_cache.h"
 #include "content/browser/service_worker/service_worker_info.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_utils.h"
 #include "content/browser/service_worker/service_worker_version.h"
-#include "content/common/service_worker/service_worker_types.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/net_errors.h"
 #include "webkit/browser/quota/quota_manager_proxy.h"
@@ -25,9 +22,6 @@ namespace content {
 
 namespace {
 
-typedef base::Callback<void(ServiceWorkerStorage::InitialData* data,
-                            bool success)> InitializeCallback;
-
 void RunSoon(const tracked_objects::Location& from_here,
              const base::Closure& closure) {
   base::MessageLoop::current()->PostTask(from_here, closure);
@@ -51,77 +45,42 @@ void CompleteFindSoon(
 const base::FilePath::CharType kServiceWorkerDirectory[] =
     FILE_PATH_LITERAL("ServiceWorker");
 
+
 const int kMaxMemDiskCacheSize = 10 * 1024 * 1024;
 
 void EmptyCompletionCallback(int) {}
 
-void InitializeOnDatabaseTaskRunner(
-    ServiceWorkerDatabase* database,
-    scoped_refptr<base::SequencedTaskRunner> original_task_runner,
-    const InitializeCallback& callback) {
-  DCHECK(database);
-  ServiceWorkerStorage::InitialData* data =
-      new ServiceWorkerStorage::InitialData();
-  bool success =
-      database->GetNextAvailableIds(&data->next_registration_id,
-                                    &data->next_version_id,
-                                    &data->next_resource_id) &&
-      database->GetOriginsWithRegistrations(&data->origins);
-  original_task_runner->PostTask(
-      FROM_HERE, base::Bind(callback, base::Owned(data), success));
-}
-
 }  // namespace
 
-ServiceWorkerStorage::InitialData::InitialData()
-    : next_registration_id(kInvalidServiceWorkerRegistrationId),
-      next_version_id(kInvalidServiceWorkerVersionId),
-      next_resource_id(kInvalidServiceWorkerResourceId) {
-}
-
-ServiceWorkerStorage::InitialData::~InitialData() {
-}
-
 ServiceWorkerStorage::ServiceWorkerStorage(
     const base::FilePath& path,
     base::WeakPtr<ServiceWorkerContextCore> context,
     base::SequencedTaskRunner* database_task_runner,
     quota::QuotaManagerProxy* quota_manager_proxy)
-    : next_registration_id_(kInvalidServiceWorkerRegistrationId),
-      next_version_id_(kInvalidServiceWorkerVersionId),
-      next_resource_id_(kInvalidServiceWorkerResourceId),
-      state_(UNINITIALIZED),
+    : last_registration_id_(0),
+      last_version_id_(0),
+      last_resource_id_(0),
+      simulated_lazy_initted_(false),
       context_(context),
       database_task_runner_(database_task_runner),
-      quota_manager_proxy_(quota_manager_proxy),
-      weak_factory_(this) {
+      quota_manager_proxy_(quota_manager_proxy) {
   if (!path.empty())
     path_ = path.Append(kServiceWorkerDirectory);
-
-  // TODO(nhiroki): Create a database on-disk after the database schema gets
-  // stable.
-  database_.reset(new ServiceWorkerDatabase(base::FilePath()));
 }
 
 ServiceWorkerStorage::~ServiceWorkerStorage() {
-  weak_factory_.InvalidateWeakPtrs();
-  database_task_runner_->DeleteSoon(FROM_HERE, database_.release());
 }
 
 void ServiceWorkerStorage::FindRegistrationForPattern(
     const GURL& scope,
     const FindRegistrationCallback& callback) {
+  simulated_lazy_initted_ = true;
   scoped_refptr<ServiceWorkerRegistration> null_registration;
-  if (!LazyInitialize(base::Bind(
-          &ServiceWorkerStorage::FindRegistrationForPattern,
-          weak_factory_.GetWeakPtr(), scope, callback))) {
-    if (state_ != INITIALIZING || !context_) {
-      CompleteFindSoon(FROM_HERE, null_registration,
-                       SERVICE_WORKER_ERROR_FAILED, callback);
-    }
+  if (!context_) {
+    CompleteFindSoon(
+        FROM_HERE, null_registration, SERVICE_WORKER_ERROR_FAILED, callback);
     return;
   }
-  DCHECK_EQ(INITIALIZED, state_);
 
   scoped_refptr<ServiceWorkerRegistration> installing_registration =
       FindInstallingRegistrationForPattern(scope);
@@ -165,15 +124,12 @@ void ServiceWorkerStorage::FindRegistrationForPattern(
 void ServiceWorkerStorage::FindRegistrationForDocument(
     const GURL& document_url,
     const FindRegistrationCallback& callback) {
+  simulated_lazy_initted_ = true;
   scoped_refptr<ServiceWorkerRegistration> null_registration;
-  if (!LazyInitialize(base::Bind(
-          &ServiceWorkerStorage::FindRegistrationForDocument,
-          weak_factory_.GetWeakPtr(), document_url, callback))) {
-    if (state_ != INITIALIZING || !context_)
-      CompleteFindNow(null_registration, SERVICE_WORKER_ERROR_FAILED, callback);
+  if (!context_) {
+    CompleteFindNow(null_registration, SERVICE_WORKER_ERROR_FAILED, callback);
     return;
   }
-  DCHECK_EQ(INITIALIZED, state_);
 
   // See if there are any registrations for the origin.
   OriginRegistrationsMap::const_iterator
@@ -239,16 +195,12 @@ void ServiceWorkerStorage::FindRegistrationForDocument(
 void ServiceWorkerStorage::FindRegistrationForId(
     int64 registration_id,
     const FindRegistrationCallback& callback) {
+  simulated_lazy_initted_ = true;
   scoped_refptr<ServiceWorkerRegistration> null_registration;
-  if (!LazyInitialize(base::Bind(
-          &ServiceWorkerStorage::FindRegistrationForId,
-          weak_factory_.GetWeakPtr(), registration_id, callback))) {
-    if (state_ != INITIALIZING || !context_)
-      CompleteFindNow(null_registration, SERVICE_WORKER_ERROR_FAILED, callback);
+  if (!context_) {
+    CompleteFindNow(null_registration, SERVICE_WORKER_ERROR_FAILED, callback);
     return;
   }
-  DCHECK_EQ(INITIALIZED, state_);
-
   scoped_refptr<ServiceWorkerRegistration> installing_registration =
       FindInstallingRegistrationForId(registration_id);
   if (installing_registration) {
@@ -274,19 +226,14 @@ void ServiceWorkerStorage::FindRegistrationForId(
 
 void ServiceWorkerStorage::GetAllRegistrations(
     const GetAllRegistrationInfosCallback& callback) {
-  if (!LazyInitialize(base::Bind(
-          &ServiceWorkerStorage::GetAllRegistrations,
-          weak_factory_.GetWeakPtr(), callback))) {
-    if (state_ != INITIALIZING || !context_) {
-      RunSoon(FROM_HERE, base::Bind(
-          callback, std::vector<ServiceWorkerRegistrationInfo>()));
-    }
+  simulated_lazy_initted_ = true;
+  std::vector<ServiceWorkerRegistrationInfo> registrations;
+  if (!context_) {
+    RunSoon(FROM_HERE, base::Bind(callback, registrations));
     return;
   }
-  DCHECK_EQ(INITIALIZED, state_);
 
   // Add all stored registrations.
-  std::vector<ServiceWorkerRegistrationInfo> registrations;
   for (RegistrationPtrMap::const_iterator it = registrations_by_id_.begin();
        it != registrations_by_id_.end(); ++it) {
     ServiceWorkerRegistration* registration =
@@ -323,9 +270,8 @@ void ServiceWorkerStorage::StoreRegistration(
     const StatusCallback& callback) {
   DCHECK(registration);
   DCHECK(version);
-
-  DCHECK(state_ == INITIALIZED || state_ == DISABLED);
-  if (state_ != INITIALIZED || !context_) {
+  DCHECK(simulated_lazy_initted_);
+  if (!context_) {
     RunSoon(FROM_HERE, base::Bind(callback, SERVICE_WORKER_ERROR_FAILED));
     return;
   }
@@ -349,13 +295,11 @@ void ServiceWorkerStorage::StoreRegistration(
   RunSoon(FROM_HERE, base::Bind(callback, SERVICE_WORKER_OK));
 }
 
-void ServiceWorkerStorage::UpdateToActiveState(
-    ServiceWorkerRegistration* registration,
-    const StatusCallback& callback) {
-  DCHECK(registration);
-
-  DCHECK(state_ == INITIALIZED || state_ == DISABLED);
-  if (state_ != INITIALIZED || !context_) {
+ void ServiceWorkerStorage::UpdateToActiveState(
+      ServiceWorkerRegistration* registration,
+      const StatusCallback& callback) {
+  DCHECK(simulated_lazy_initted_);
+  if (!context_) {
     RunSoon(FROM_HERE, base::Bind(callback, SERVICE_WORKER_ERROR_FAILED));
     return;
   }
@@ -374,12 +318,7 @@ void ServiceWorkerStorage::UpdateToActiveState(
 void ServiceWorkerStorage::DeleteRegistration(
     int64 registration_id,
     const StatusCallback& callback) {
-  DCHECK(state_ == INITIALIZED || state_ == DISABLED);
-  if (state_ != INITIALIZED || !context_) {
-    RunSoon(FROM_HERE, base::Bind(callback, SERVICE_WORKER_ERROR_FAILED));
-    return;
-  }
-
+  DCHECK(simulated_lazy_initted_);
   RegistrationPtrMap::iterator
       found = registrations_by_id_.find(registration_id);
   if (found == registrations_by_id_.end()) {
@@ -414,24 +353,18 @@ ServiceWorkerStorage::CreateResponseWriter(int64 response_id) {
 }
 
 int64 ServiceWorkerStorage::NewRegistrationId() {
-  if (state_ == DISABLED)
-    return kInvalidServiceWorkerRegistrationId;
-  DCHECK_EQ(INITIALIZED, state_);
-  return next_registration_id_++;
+  DCHECK(simulated_lazy_initted_);
+  return ++last_registration_id_;
 }
 
 int64 ServiceWorkerStorage::NewVersionId() {
-  if (state_ == DISABLED)
-    return kInvalidServiceWorkerVersionId;
-  DCHECK_EQ(INITIALIZED, state_);
-  return next_version_id_++;
+  DCHECK(simulated_lazy_initted_);
+  return ++last_version_id_;
 }
 
 int64 ServiceWorkerStorage::NewResourceId() {
-  if (state_ == DISABLED)
-    return kInvalidServiceWorkerResourceId;
-  DCHECK_EQ(INITIALIZED, state_);
-  return next_resource_id_++;
+  DCHECK(simulated_lazy_initted_);
+  return ++last_resource_id_;
 }
 
 void ServiceWorkerStorage::NotifyInstallingRegistration(
@@ -444,58 +377,6 @@ void ServiceWorkerStorage::NotifyDoneInstallingRegistration(
   installing_registrations_.erase(registration->id());
 }
 
-bool ServiceWorkerStorage::LazyInitialize(const base::Closure& callback) {
-  if (!context_)
-    return false;
-
-  switch (state_) {
-    case INITIALIZED:
-      return true;
-    case DISABLED:
-      return false;
-    case INITIALIZING:
-      pending_tasks_.push_back(callback);
-      return false;
-    case UNINITIALIZED:
-      pending_tasks_.push_back(callback);
-      // Fall-through.
-  }
-
-  state_ = INITIALIZING;
-  database_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&InitializeOnDatabaseTaskRunner,
-                 database_.get(),
-                 base::MessageLoopProxy::current(),
-                 base::Bind(&ServiceWorkerStorage::DidInitialize,
-                            weak_factory_.GetWeakPtr())));
-  return false;
-}
-
-void ServiceWorkerStorage::DidInitialize(
-    InitialData* data,
-    bool success) {
-  DCHECK(data);
-  DCHECK_EQ(INITIALIZING, state_);
-
-  if (success) {
-    next_registration_id_ = data->next_registration_id;
-    next_version_id_ = data->next_version_id;
-    next_resource_id_ = data->next_resource_id;
-    registered_origins_.swap(data->origins);
-    state_ = INITIALIZED;
-  } else {
-    DLOG(WARNING) << "Failed to initialize.";
-    state_ = DISABLED;
-  }
-
-  for (std::vector<base::Closure>::const_iterator it = pending_tasks_.begin();
-       it != pending_tasks_.end(); ++it) {
-    RunSoon(FROM_HERE, *it);
-  }
-  pending_tasks_.clear();
-}
-
 scoped_refptr<ServiceWorkerRegistration>
 ServiceWorkerStorage::CreateRegistration(
     const ServiceWorkerDatabase::RegistrationData* data) {
diff --git a/content/browser/service_worker/service_worker_storage.h b/content/browser/service_worker/service_worker_storage.h
index 369273a..4c06e2f 100644
--- a/content/browser/service_worker/service_worker_storage.h
+++ b/content/browser/service_worker/service_worker_storage.h
@@ -6,7 +6,6 @@
 #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_STORAGE_H_
 
 #include <map>
-#include <set>
 #include <vector>
 
 #include "base/bind.h"
@@ -52,16 +51,6 @@ class CONTENT_EXPORT ServiceWorkerStorage {
       void(ServiceWorkerStatusCode status, int result)>
           CompareCallback;
 
-  struct InitialData {
-    int64 next_registration_id;
-    int64 next_version_id;
-    int64 next_resource_id;
-    std::set<GURL> origins;
-
-    InitialData();
-    ~InitialData();
-  };
-
   ServiceWorkerStorage(const base::FilePath& path,
                        base::WeakPtr<ServiceWorkerContextCore> context,
                        base::SequencedTaskRunner* database_task_runner,
@@ -127,12 +116,6 @@ class CONTENT_EXPORT ServiceWorkerStorage {
  private:
   friend class ServiceWorkerStorageTest;
 
-  bool LazyInitialize(
-      const base::Closure& callback);
-  void DidInitialize(
-      InitialData* data,
-      bool success);
-
   scoped_refptr<ServiceWorkerRegistration> CreateRegistration(
       const ServiceWorkerDatabase::RegistrationData* data);
   ServiceWorkerRegistration* FindInstallingRegistrationForDocument(
@@ -163,36 +146,17 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   // Lazy disk_cache getter.
   ServiceWorkerDiskCache* disk_cache();
 
-  // Origins having registations.
-  std::set<GURL> registered_origins_;
-
-  // Pending database tasks waiting for initialization.
-  std::vector<base::Closure> pending_tasks_;
-
-  int64 next_registration_id_;
-  int64 next_version_id_;
-  int64 next_resource_id_;
-
-  enum State {
-    UNINITIALIZED,
-    INITIALIZING,
-    INITIALIZED,
-    DISABLED,
-  };
-  State state_;
+  int64 last_registration_id_;
+  int64 last_version_id_;
+  int64 last_resource_id_;
+  bool simulated_lazy_initted_;
 
   base::FilePath path_;
   base::WeakPtr<ServiceWorkerContextCore> context_;
-
-  // Only accessed on |database_task_runner_|.
-  scoped_ptr<ServiceWorkerDatabase> database_;
-
   scoped_refptr<base::SequencedTaskRunner> database_task_runner_;
   scoped_refptr<quota::QuotaManagerProxy> quota_manager_proxy_;
   scoped_ptr<ServiceWorkerDiskCache> disk_cache_;
 
-  base::WeakPtrFactory<ServiceWorkerStorage> weak_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerStorage);
 };
 
diff --git a/content/browser/service_worker/service_worker_storage_unittest.cc b/content/browser/service_worker/service_worker_storage_unittest.cc
index 9637929..6d8b30f 100644
--- a/content/browser/service_worker/service_worker_storage_unittest.cc
+++ b/content/browser/service_worker/service_worker_storage_unittest.cc
@@ -79,6 +79,7 @@ class ServiceWorkerStorageTest : public testing::Test {
         NULL,
         scoped_ptr<ServiceWorkerProcessManager>()));
     context_ptr_ = context_->AsWeakPtr();
+    storage()->simulated_lazy_initted_ = true;
   }
 
   virtual void TearDown() OVERRIDE {
@@ -97,8 +98,8 @@ TEST_F(ServiceWorkerStorageTest, StoreFindUpdateDeleteRegistration) {
   const GURL kScope("http://www.test.com/scope/*");
   const GURL kScript("http://www.test.com/script.js");
   const GURL kDocumentUrl("http://www.test.com/scope/document.html");
-  const int64 kRegistrationId = 0;
-  const int64 kVersionId = 0;
+  const int64 kRegistrationId = storage()->NewRegistrationId();
+  const int64 kVersionId = storage()->NewVersionId();
 
   bool was_called = false;
   ServiceWorkerStatusCode result = SERVICE_WORKER_OK;
@@ -306,8 +307,8 @@ TEST_F(ServiceWorkerStorageTest, InstallingRegistrationsAreFindable) {
   const GURL kScope("http://www.test.com/scope/*");
   const GURL kScript("http://www.test.com/script.js");
   const GURL kDocumentUrl("http://www.test.com/scope/document.html");
-  const int64 kRegistrationId = 0;
-  const int64 kVersionId = 0;
+  const int64 kRegistrationId = storage()->NewRegistrationId();
+  const int64 kVersionId = storage()->NewVersionId();
 
   bool was_called = false;
   ServiceWorkerStatusCode result = SERVICE_WORKER_OK;