Delete and try to recreate localstorage database on invalid schema version.

content/browser/dom_storage/local_storage_context_mojo.cc

 // Copyright 2016 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/dom_storage/local_storage_context_mojo.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "components/leveldb/public/cpp/util.h"
 #include "components/leveldb/public/interfaces/leveldb.mojom.h"
@@ skipping 32 matching lines @@
 std::vector<uint8_t> CreateMetaDataKey(const url::Origin& origin) {
   auto serialized_origin = leveldb::StdStringToUint8Vector(origin.Serialize());
   std::vector<uint8_t> key;
   key.reserve(arraysize(kMetaPrefix) + serialized_origin.size());
   key.insert(key.end(), kMetaPrefix, kMetaPrefix + arraysize(kMetaPrefix));
   key.insert(key.end(), serialized_origin.begin(), serialized_origin.end());
   return key;
 }
 
 void NoOpSuccess(bool success) {}
-}
+
+}  // namespace
 
 LocalStorageContextMojo::LocalStorageContextMojo(
     service_manager::Connector* connector,
     const base::FilePath& subdirectory)
     : connector_(connector),
       subdirectory_(subdirectory),
       weak_ptr_factory_(this) {}
 
 LocalStorageContextMojo::~LocalStorageContextMojo() {}
 
@@ skipping 52 matching lines @@
     CHECK(connector_);
     file_service_connection_ = connector_->Connect(file::mojom::kServiceName);
     connection_state_ = CONNECTION_IN_PROGRESS;
     file_service_connection_->AddConnectionCompletedClosure(
         base::Bind(&LocalStorageContextMojo::OnUserServiceConnectionComplete,
                    weak_ptr_factory_.GetWeakPtr()));
     file_service_connection_->SetConnectionLostClosure(
         base::Bind(&LocalStorageContextMojo::OnUserServiceConnectionError,
                    weak_ptr_factory_.GetWeakPtr()));
 
-    if (!subdirectory_.empty()) {
-      // We were given a subdirectory to write to. Get it and use a disk backed
-      // database.
-      file_service_connection_->GetInterface(&file_system_);
-      file_system_->GetSubDirectory(
-          subdirectory_.AsUTF8Unsafe(), MakeRequest(&directory_),
-          base::Bind(&LocalStorageContextMojo::OnDirectoryOpened,
-                     weak_ptr_factory_.GetWeakPtr()));
-    } else {
-      // We were not given a subdirectory. Use a memory backed database.
-      file_service_connection_->GetInterface(&leveldb_service_);
-      leveldb_service_->OpenInMemory(
-          MakeRequest(&database_),
-          base::Bind(&LocalStorageContextMojo::OnDatabaseOpened,
-                     weak_ptr_factory_.GetWeakPtr()));
-    }
+    InitiateConnection();
   }
 
   if (connection_state_ == CONNECTION_IN_PROGRESS) {
     // Queue this OpenLocalStorage call for when we have a level db pointer.
     on_database_opened_callbacks_.push_back(std::move(callback));
     return;
   }
 
   std::move(callback).Run();
 }
@@ skipping 45 matching lines @@
 
 void LocalStorageContextMojo::OnUserServiceConnectionComplete() {
   CHECK_EQ(service_manager::mojom::ConnectResult::SUCCEEDED,
            file_service_connection_->GetResult());
 }
 
 void LocalStorageContextMojo::OnUserServiceConnectionError() {
   CHECK(false);
 }
 
-// Part of our asynchronous directory opening called from OpenLocalStorage().
+void LocalStorageContextMojo::InitiateConnection(bool in_memory_only) {
+  DCHECK_EQ(connection_state_, CONNECTION_IN_PROGRESS);
+  if (!subdirectory_.empty() && !in_memory_only) {
+    // We were given a subdirectory to write to. Get it and use a disk backed
+    // database.
+    file_service_connection_->GetInterface(&file_system_);
+    file_system_->GetSubDirectory(
+        subdirectory_.AsUTF8Unsafe(), MakeRequest(&directory_),
+        base::Bind(&LocalStorageContextMojo::OnDirectoryOpened,
+                   weak_ptr_factory_.GetWeakPtr()));
+  } else {
+    // We were not given a subdirectory. Use a memory backed database.
+    file_service_connection_->GetInterface(&leveldb_service_);
+    leveldb_service_->OpenInMemory(
+        MakeRequest(&database_),
+        base::Bind(&LocalStorageContextMojo::OnDatabaseOpened,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
 void LocalStorageContextMojo::OnDirectoryOpened(
     filesystem::mojom::FileError err) {
   if (err != filesystem::mojom::FileError::OK) {
     // We failed to open the directory; continue with startup so that we create
     // the |level_db_wrappers_|.
     OnDatabaseOpened(leveldb::mojom::DatabaseError::IO_ERROR);
     return;
   }
 
   // Now that we have a directory, connect to the LevelDB service and get our
   // database.
   file_service_connection_->GetInterface(&leveldb_service_);
 
+  // We might still need to directory, so create a clone.
+  filesystem::mojom::DirectoryPtr directory_clone;
+  directory_->Clone(MakeRequest(&directory_clone));
+
   auto options = leveldb::mojom::OpenOptions::New();
   options->create_if_missing = true;
   leveldb_service_->OpenWithOptions(
-      std::move(options), std::move(directory_), "leveldb",
+      std::move(options), std::move(directory_clone), "leveldb",
       MakeRequest(&database_),
       base::Bind(&LocalStorageContextMojo::OnDatabaseOpened,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void LocalStorageContextMojo::OnDatabaseOpened(
     leveldb::mojom::DatabaseError status) {
   if (status != leveldb::mojom::DatabaseError::OK) {
     // If we failed to open the database, reset the service object so we pass
     // null pointers to our wrappers.
     database_.reset();
-    leveldb_service_.reset();
   }
 
-  // We no longer need the file service; we've either transferred |directory_|
-  // to the leveldb service, or we got a file error and no more is possible.
-  directory_.reset();
-  file_system_.reset();
-
   // Verify DB schema version.
   if (database_) {
     database_->Get(leveldb::StdStringToUint8Vector(kVersionKey),
                    base::Bind(&LocalStorageContextMojo::OnGotDatabaseVersion,
                               weak_ptr_factory_.GetWeakPtr()));
     return;
   }
 
   OnGotDatabaseVersion(leveldb::mojom::DatabaseError::IO_ERROR,
                        std::vector<uint8_t>());
 }
 
 void LocalStorageContextMojo::OnGotDatabaseVersion(
     leveldb::mojom::DatabaseError status,
     const std::vector<uint8_t>& value) {
   if (status == leveldb::mojom::DatabaseError::NOT_FOUND) {
     // New database, nothing more to do. Current version will get written
     // when first data is committed.
   } else if (status == leveldb::mojom::DatabaseError::OK) {
     // Existing database, check if version number matches current schema
     // version.
     int64_t db_version;
     if (!base::StringToInt64(leveldb::Uint8VectorToStdString(value),
                              &db_version) ||
         db_version < kMinSchemaVersion || db_version > kCurrentSchemaVersion) {
-      // TODO(mek): delete and recreate database, rather than failing outright.
-      database_ = nullptr;
+      DeleteAndRecreateDatabase();
+      return;
     }
 
     database_initialized_ = true;
   } else {
     // Other read error. Possibly database corruption.
-    // TODO(mek): delete and recreate database, rather than failing outright.
-    database_ = nullptr;
+    DeleteAndRecreateDatabase();
+    return;
   }
 
+  OnConnectionFinished();
+}
+
+void LocalStorageContextMojo::OnConnectionFinished() {
+  DCHECK_EQ(connection_state_, CONNECTION_IN_PROGRESS);
+
+  // We no longer need the file service; we've either transferred |directory_|
+  // to the leveldb service, or we got a file error and no more is possible.
+  directory_.reset();
+  file_system_.reset();
+  if (!database_)
+    leveldb_service_.reset();
+
   // |database_| should be known to either be valid or invalid by now. Run our
   // delayed bindings.
   connection_state_ = CONNECTION_FINISHED;
   for (size_t i = 0; i < on_database_opened_callbacks_.size(); ++i)
     std::move(on_database_opened_callbacks_[i]).Run();
   on_database_opened_callbacks_.clear();
 }
 
+void LocalStorageContextMojo::DeleteAndRecreateDatabase() {
+  // For now don't support deletion and recreation when already connected.
+  DCHECK_EQ(connection_state_, CONNECTION_IN_PROGRESS);
+
+  bool recreate_in_memory = false;
+
+  // If tried to recreate database on disk already, try again but this time
+  // in memory.
+  if (tried_to_recreate_ && !subdirectory_.empty()) {
+    recreate_in_memory = true;
+  } else if (tried_to_recreate_) {
+    // Give up completely, run without any database.
+    database_ = nullptr;
+    OnConnectionFinished();
+    return;
+  }
+
+  tried_to_recreate_ = true;
+
+  // Unit tests might not have a file_service_connection_, in which case there
+  // is nothing to rety.
+  if (!file_service_connection_) {
+    database_ = nullptr;
+    OnConnectionFinished();
+    return;
+  }
+
+  // Close and destroy database, and try again.
+  // TODO(mek): Somehow ensure closing is processed before Destroy is called.

[Marijn Kruisselbrink, 2017/01/11 19:10:58]

This TODO is somewhat problematic, since I'm not sure how to accomplish such a
thing. Maybe the Destroy call should be part of the LevelDBDatabase interface
rather than LevelDBService (where it could make sure to close itself before
destroying itself), but then you'd lose the ability to destroy a database
without opening it first. Anything else seems like it will have the problem that
the leveldb service and leveldb database are separate mojo pipes and thus
relying on something on one pipe to have finished before calling a method on the
other pipe is processed is dangerous. Any thoughts?

[jam, 2017/01/12 17:24:42]

If Destroy is on LevelDBInterface, won't there be the same issue for ordering
since the Open is on LevelDBService?

To solve the ordering issue, we could use associated interfaces. wdyt?
https://www.chromium.org/developers/design-documents/mojo/associated-interfaces

[Marijn Kruisselbrink, 2017/01/13 19:25:11]

I suppose so, although it seemed less likely that you'd be trying to open a
database while simultaneously trying to destroy it than that you'd want to
destroy a database that you had open already.
Yes, that would work. Changed it to do so. Somehow using associated interface
does kind of always feel like doing it the wrong way to me, with it being a c++
only feature and all (or did that change recently?)...

[jam, 2017/01/14 00:17:55]

JavaScript support is being worked on :)

Associated interfaces shouldn't be viewed negatively, it's very useful to split
logically separate methods into separate interfaces, while still maintaining
order. Otherwise things that don't belong together are grouped.

+  database_ = nullptr;
+  if (directory_.is_bound()) {
+    leveldb_service_->Destroy(
+        std::move(directory_), "leveldb",
+        base::Bind(&LocalStorageContextMojo::OnDBDestroyed,
+                   weak_ptr_factory_.GetWeakPtr(), recreate_in_memory));
+  } else {
+    // No directory, so nothing to destroy. Retrying to recreate will probably
+    // fail, but try anyway.
+    InitiateConnection(recreate_in_memory);
+  }
+}
+
+void LocalStorageContextMojo::OnDBDestroyed(
+    bool recreate_in_memory,
+    leveldb::mojom::DatabaseError status) {
+  // We're essentially ignoring the status here. Even if destroying failed we
+  // still want to go ahead and try to recreate.
+  InitiateConnection(recreate_in_memory);
+}
+
 // The (possibly delayed) implementation of OpenLocalStorage(). Can be called
 // directly from that function, or through |on_database_open_callbacks_|.
 void LocalStorageContextMojo::BindLocalStorage(
     const url::Origin& origin,
     mojom::LevelDBWrapperRequest request) {
   GetOrCreateDBWrapper(origin)->Bind(std::move(request));
 }
 
 LevelDBWrapperImpl* LocalStorageContextMojo::GetOrCreateDBWrapper(
     const url::Origin& origin) {
@@ skipping 66 matching lines @@
   for (const auto& info : usage) {
     url::Origin origin_candidate(info.origin);
     if (!origin_candidate.IsSameOriginWith(origin) &&
         origin_candidate.IsSamePhysicalOriginWith(origin))
       DeleteStorage(origin_candidate);
   }
   DeleteStorage(origin);
 }
 
 }  // namespace content