mojo leveldb: Get profile and leveldb connected to DOMStorageContext.

content/browser/dom_storage/dom_storage_context_wrapper.cc

Index: content/browser/dom_storage/dom_storage_context_wrapper.cc
diff --git a/content/browser/dom_storage/dom_storage_context_wrapper.cc b/content/browser/dom_storage/dom_storage_context_wrapper.cc
index d028d6faab5145e18d5ecc6bef40bfcd4c819801..e43b849547c352578460887fe3a4e8be8ac08898 100644
--- a/content/browser/dom_storage/dom_storage_context_wrapper.cc
+++ b/content/browser/dom_storage/dom_storage_context_wrapper.cc
@@ -13,14 +13,18 @@
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
+#include "components/filesystem/public/interfaces/file_system.mojom.h"
 #include "content/browser/dom_storage/dom_storage_area.h"
 #include "content/browser/dom_storage/dom_storage_context_impl.h"
 #include "content/browser/dom_storage/dom_storage_task_runner.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/leveldb_wrapper_impl.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/local_storage_usage_info.h"
+#include "content/public/browser/mojo_app_connection.h"
 #include "content/public/browser/session_storage_usage_info.h"
+#include "mojo/common/common_type_converters.h"
 
 namespace content {
 namespace {
@@ -67,15 +71,18 @@ void GetSessionStorageUsageHelper(
 }  // namespace
 
 DOMStorageContextWrapper::DOMStorageContextWrapper(
-    const base::FilePath& data_path,
-    storage::SpecialStoragePolicy* special_storage_policy) {
+    BrowserContext* context,
+    const base::FilePath& data_path)
+    : browser_context_(context),
+      connection_state_(NO_CONNECTION),
+      weak_ptr_factory_(this) {
   base::SequencedWorkerPool* worker_pool = BrowserThread::GetBlockingPool();
   context_ = new DOMStorageContextImpl(
       data_path.empty() ? data_path
                         : data_path.AppendASCII(kLocalStorageDirectory),
       data_path.empty() ? data_path
                         : data_path.AppendASCII(kSessionStorageDirectory),
-      special_storage_policy,
+      context->GetSpecialStoragePolicy(),
       new DOMStorageWorkerPoolTaskRunner(
           worker_pool,
           worker_pool->GetNamedSequenceToken("dom_storage_primary"),
@@ -84,8 +91,7 @@ DOMStorageContextWrapper::DOMStorageContextWrapper(
               .get()));
 }
 
-DOMStorageContextWrapper::~DOMStorageContextWrapper() {
-}
+DOMStorageContextWrapper::~DOMStorageContextWrapper() {}
 
 void DOMStorageContextWrapper::GetLocalStorageUsage(
     const GetLocalStorageUsageCallback& callback) {
@@ -170,25 +176,81 @@ void DOMStorageContextWrapper::Flush() {
 void DOMStorageContextWrapper::OpenLocalStorage(
     const mojo::String& origin,
     mojo::InterfaceRequest<LevelDBWrapper> request) {
+  // If we don't have a filesystem_connection_, we'll need to establish one.
+  if (connection_state_ == NO_CONNECTION) {
+    profile_connection_ = MojoAppConnection::Create(
+        browser_context_, "mojo:profile", kBrowserMojoAppUrl);
+    profile_connection_->GetInterface(&profile_service_);
+
+    profile_service_->GetDirectory(

[michaeln, 2016/03/05 01:38:54]

What do we do about incognito profiles? How is that represented by the
profile_service?

[Elliot Glaysher, 2016/03/10 21:00:35]

I was under the impression that incognito stuff never went to disk; that
incognito local storage was stuffed into an in-memory std::map<>.

+        GetProxy(&directory_),
+        base::Bind(&DOMStorageContextWrapper::OnDirectoryOpened,
+                   weak_ptr_factory_.GetWeakPtr()));
+    connection_state_ = CONNECITON_IN_PROGRESS;
+  }
+
+  if (connection_state_ == CONNECITON_IN_PROGRESS) {
+    // Queue this OpenLocalStorage call for when we have a level db pointer.
+    on_database_opened_callbacks_.push_back(
+        base::Bind(&DOMStorageContextWrapper::BindLocalStorage,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   origin,
+                   base::Passed(&request)));
+    return;
+  }
+
+  BindLocalStorage(origin, std::move(request));
+}
+
+void DOMStorageContextWrapper::LevelDBWrapperImplHasNoBindings(
+    const std::string& origin) {
+  DCHECK(level_db_wrappers_.find(origin) != level_db_wrappers_.end());
+  level_db_wrappers_.erase(origin);
+}
+
+void DOMStorageContextWrapper::OnDirectoryOpened(filesystem::FileError err) {
+  if (err != filesystem::FileError::OK) {
+    NOTREACHED();

[michaeln, 2016/03/05 01:38:54]

What makes this not reachable? If the native file system gives us errors, will
we not reach this?

We'll probably want to mimic behavior of the current system in the event of
errors that prevent reading/writing to disk. I'll take a look at what that that
behavior is and let you know.

If the old system is unable to read/write to its underlying database, it still
functions and will retain values for the duration of the browsing session, it
might drop things if all tabs for an origin are closed. The idea is the show
must go on despite database corruption or a readonly filesystem.

edit: Just looked at where the callback is invoked. If the GetDirectory() method
always returns OK, we should remove the |err| parameter.

[Elliot Glaysher, 2016/03/10 21:00:35]

Changed all of this so that it should work even when we've been given an error
from subdirectory creation. (work == pass a null leveldb object to
LevelDBWrapperImpl.)

+    return;
+  }
+
+  // Now that we have a directory, connect to the LevelDB service and get our
+  // database.
+  leveldb_connection_ = MojoAppConnection::Create(

[michaeln, 2016/03/05 01:38:54]

naming nit: I understand this was chosen from a mojo'ish point of view, but from
a db'ish point of view, |connection| is an odd choice. It's definitely not a
connection to a database. What's current called database_ is much closer to a
connection in the db'ish sense.

[Elliot Glaysher, 2016/03/10 21:00:35]

To match the mojo convention, I renamed |leveldb_| to |leveldb_service_|.

+      browser_context_, "mojo:leveldb", kBrowserMojoAppUrl);
+  leveldb_connection_->GetInterface(&leveldb_);
+
+  leveldb_->Open(std::move(directory_), "local_storage", GetProxy(&database_),

[michaeln, 2016/03/05 01:38:54]

I think directory_ corresponds to a particular profile dir, is that right?
Something like...

C:\Users\fred\AppData\Local\Google\Chrome\User Data\Default
  or
C:\Users\fred\AppData\Local\Google\Chrome\User Data\Profile 2

We need to be more specific than that, there's a separate Storage directory for
each StorageParition, a Profile may contain multiple partitions.

The |data_path| given the the constructor is the full path to the storage
partitions directory, unless it's an incognito profile in which case its an
empty string.

For the main storage partition |data_path| == profile dir. For sub-partitions
created by the apps/extensions system it looks like...

{profiledir}/Storage/ext/<extensionid>/def
or
{profiledir}/Storage/ext/<extensionid>/D77D9D3CCB10

[Elliot Glaysher, 2016/03/10 21:00:35]

Done, but not like this. |directory_| now points to the per partition subdir,
and then under that goes the leveldb dir.

(I believe we talked about this in person.)

+                 base::Bind(&DOMStorageContextWrapper::OnDatabaseOpened,
+                            weak_ptr_factory_.GetWeakPtr()));

[michaeln, 2016/03/05 01:38:54]

We currently keep individual local storage sqlite databases in
"{paritiondir}/Local Storage". I'd suggest we keep this new database here...

{partitiondir}/Local Storage/leveldb/

[Elliot Glaysher, 2016/03/10 21:00:35]

Done.

+}
+
+void DOMStorageContextWrapper::OnDatabaseOpened(leveldb::DatabaseError status) {
+  if (status != leveldb::DatabaseError::OK) {
+    NOTREACHED();

[michaeln, 2016/03/05 01:38:54]

This is certainly reachable, per the show must go on rule, we'll have to
continue with a null database_ ptr.

+    return;
+  }
+
+  // |database_| should be valid now. Run our delayed bindings.
+  connection_state_ = CONNECTED;
+  for (size_t i = 0; i < on_database_opened_callbacks_.size(); ++i)
+    on_database_opened_callbacks_[i].Run();
+  on_database_opened_callbacks_.clear();
+}
+
+void DOMStorageContextWrapper::BindLocalStorage(
+    const mojo::String& origin,
+    mojo::InterfaceRequest<LevelDBWrapper> request) {
   if (level_db_wrappers_.find(origin) == level_db_wrappers_.end()) {
     level_db_wrappers_[origin] = make_scoped_ptr(new LevelDBWrapperImpl(
+        database_.get(),
         origin,
         base::Bind(&DOMStorageContextWrapper::LevelDBWrapperImplHasNoBindings,
                    base::Unretained(this),
                    origin.get())));
   }
-  // TODO(jam): call LevelDB service (once per this object) to open the database
-  // for LocalStorage and keep a pointer to it in this class. Then keep a map
-  // from origins to LevelDBWrapper object. Each call here for the same origin
-  // should use the same LevelDBWrapper object.
 
   level_db_wrappers_[origin]->Bind(std::move(request));
 }
 
-void DOMStorageContextWrapper::LevelDBWrapperImplHasNoBindings(
-    const std::string& origin) {
-  DCHECK(level_db_wrappers_.find(origin) != level_db_wrappers_.end());
-  level_db_wrappers_.erase(origin);
-}
-
 }  // namespace content