mandoline filesystem: add a sqlite3 vfs to proxy filesystem usage.

sql/mojo/enable_mojo_fs.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 "sql/mojo/enable_mojo_fs.h"
+
+#include "base/logging.h"
+#include "components/filesystem/public/interfaces/file.mojom.h"
+#include "components/filesystem/public/interfaces/file_system.mojom.h"
+#include "components/filesystem/public/interfaces/types.mojom.h"
+#include "mojo/public/cpp/bindings/lib/template_util.h"
+#include "mojo/util/capture_util.h"
+#include "third_party/sqlite/sqlite3.h"
+
+using mojo::Capture;
+
+namespace sql {
+namespace {
+
+// Implementation of the sqlite3 Mojo proxying vfs.

[sky, 2015/06/10 21:26:27]

I'm assuming shess is reviewing all this.

+//
+// This is a bunch of C callback objects which transparently proxy sqlite3's
+// filesystem reads/writes over the mojo:filesystem service. The main
+// entrypoint is sqlite3_mojovfs(), which proxies all the file open/delete/etc
+// operations. mojo:filesystem has support for passing a raw file descriptor
+// over the IPC barrier, and most of the implementation of sqlite3_io_methods
+// is derived from the default sqlite3 unix VFS and operates on the raw file
+// descriptors.
+
+#if defined(OS_WIN)
+const char kParentVFS[] = "win32";
+#elif defined(OS_POSIX)
+const char kParentVFS[] = "unix";
+#endif
+
+const int kMaxPathName = 512;
+
+// Additional data that we store in the sqlite3_vfs's |pAppData| member.
+struct MojoVFSData {
+  // The default vfs at the time MojoVFS was installed. We use the to pass
+  // through things like randomness requests and per-platform sleep calls.
+  sqlite3_vfs* parent;
+
+  // When we initialize the subsystem, we are given a filesystem::Directory
+  // object, which is the root directory of a mojo:filesystem. All access to
+  // various files are specified from this root directory.
+  filesystem::DirectoryPtr root_directory;

[Scott Hess - ex-Googler, 2015/06/10 22:35:46]

To make sure I'm clear on this - all databases for all connections used by all
clients of this will use this single root directory, right?

[Elliot Glaysher, 2015/06/12 22:36:43]

...within a service/origin combination.

The current model that I'm targeting is having a top level profile dir, and then
each subdir is passed to a service origin. (ie, the network service gets
sandboxed to the //profile/mojo:network_service/ directory.) Later, we're going
to have to deal with combinations of service/origin pairs (ie,
//profile/mojo:html_viewer/http___google.com/), but that's a bit out.

[Scott Hess - ex-Googler, 2015/06/17 19:32:58]

So all connections used by all clients in a single process, since each process
has a global default vfs?

[Elliot Glaysher, 2015/06/17 21:59:34]

All sql::Connections in a process will use the same vfs. That vfs in that one
process connects to the filesystem service. The filesystem service will vend a
directory which is origin bound to the origin of the calling program (currently
mojo://sql_apptests and soon mojo://network_service).

[Scott Hess - ex-Googler, 2015/06/19 20:48:01]

Acknowledged.

+};
+
+sqlite3_vfs* GetParentVFS(sqlite3_vfs* mojo_vfs) {
+  return reinterpret_cast<MojoVFSData*>(mojo_vfs->pAppData)->parent;
+}
+
+filesystem::DirectoryPtr& GetRootDirectory(sqlite3_vfs* mojo_vfs) {
+  return reinterpret_cast<MojoVFSData*>(mojo_vfs->pAppData)->root_directory;
+}
+
+// A struct which extends the base sqlite3_file to also hold on to a file
+// pipe. We reinterpret_cast our sqlite3_file structs to this struct
+// instead. This is "safe" because this struct is really just a slab of
+// malloced memory, of which we tell sqlite how large we want with szOsFile.
+struct MojoVFSFile {
+  // The "vtable" of our sqlite3_file "subclass".
+  sqlite3_file base;
+
+  // We keep an open pipe to the File object to keep it from cleaning itself
+  // up.
+  filesystem::FilePtr file_ptr;
+};
+
+filesystem::FilePtr& GetFSFile(sqlite3_file* vfs_file) {
+  return reinterpret_cast<MojoVFSFile*>(vfs_file)->file_ptr;
+}
+
+int MojoVFSClose(sqlite3_file* file) {
+  DVLOG(1) << "MojoVFSClose(*)";
+  using mojo::InterfacePtr;
+  GetFSFile(file).~InterfacePtr<filesystem::File>();

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

Symmetry-wise, why not:
   delete (&GetFSFile(file)) filesystem::~FilePtr();
?  I probably have the syntax wrong, basically just wondering why the placement
new is entirely different from what probably is the placement delete but maybe
isn't.

In the long run it might make sense to just construct/destruct the MojoVFSFile
instance itself, and let the contents unwind naturally.

[Elliot Glaysher, 2015/06/12 22:36:44]

Because there's no such thing as placement delete.

http://www.stroustrup.com/bs_faq2.html#placement-delete

[Scott Hess - ex-Googler, 2015/06/17 19:32:58]

But you need to call the destructor, and as a casual reader, I have no idea
where InterfacePtr<>() is coming from, since this is the only point in the file
where it is referenced.  I can guess that InterfacePtr<filesystem::File> might
somehow relate to filesystem::FilePtr, but it seems like it would be more
straightforward to just call ~filesystem::FilePtr() directly.

[Elliot Glaysher, 2015/06/17 21:59:34]

Ah. I see what you're saying. Calling ~FilePtr(). (FilePtr is a typedef of
InterfacePtr<File>.)

(Have to use the using because you can't call ~filesystem::FilePtr() because you
can't use full qualifiers when manualy calling destructors.)

[Scott Hess - ex-Googler, 2015/06/19 20:48:01]

Ewwww.

+  return SQLITE_OK;
+}
+
+int MojoVFSRead(sqlite3_file* sql_file,
+                void* buffer,
+                int size,
+                sqlite3_int64 offset) {
+  DVLOG(1) << "MojoVFSRead (" << size << " @ " << offset << ")";
+  filesystem::FileError error = filesystem::FILE_ERROR_FAILED;
+  mojo::Array<uint8_t> mojo_data;
+  GetFSFile(sql_file)->Read(size, offset, filesystem::WHENCE_FROM_BEGIN,
+                            Capture(&error, &mojo_data));
+  GetFSFile(sql_file).WaitForIncomingResponse();
+
+  if (error != filesystem::FILE_ERROR_OK) {
+    // TODO(erg): Better implementation here.
+    NOTIMPLEMENTED();
+    return SQLITE_IOERR_READ;
+  }
+
+  if (mojo_data.size() == static_cast<size_t>(size)) {
+    memcpy(buffer, &mojo_data.front(), mojo_data.size());
+    return SQLITE_OK;
+  }
+
+  // We didn't read the entire buffer. We need to copy the data AND zero fill
+  // the rest of the buffer.
+  if (mojo_data.size())
+    memcpy(buffer, &mojo_data.front(), mojo_data.size());
+  memset(reinterpret_cast<char*>(buffer) + mojo_data.size(), 0,
+         size - mojo_data.size());
+
+  return SQLITE_IOERR_SHORT_READ;

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

Good to know about!

+}
+
+int MojoVFSWrite(sqlite3_file* sql_file,
+                 const void* buffer,
+                 int size,
+                 sqlite_int64 offset) {
+  DVLOG(1) << "MojoVFSWrite(*, " << size << ", " << offset << ")";
+  mojo::Array<uint8_t> mojo_data(size);
+  memcpy(&mojo_data.front(), buffer, size);
+
+  filesystem::FileError error = filesystem::FILE_ERROR_FAILED;
+  uint32_t num_bytes_written = 0;
+  GetFSFile(sql_file)->Write(mojo_data.Pass(), offset,
+                             filesystem::WHENCE_FROM_BEGIN,
+                             Capture(&error, &num_bytes_written));
+  GetFSFile(sql_file).WaitForIncomingResponse();
+  if (error != filesystem::FILE_ERROR_OK) {
+    // TODO(erg): Better implementation here.
+    NOTIMPLEMENTED();
+    return SQLITE_IOERR_WRITE;
+  }
+  if (num_bytes_written != static_cast<uint32_t>(size)) {
+    NOTIMPLEMENTED();
+    return SQLITE_IOERR_WRITE;
+  }
+
+  return SQLITE_OK;
+}
+
+int MojoVFSTruncate(sqlite3_file* sql_file, sqlite_int64 size) {
+  DVLOG(1) << "MojoVFSTruncate(*, " << size << ")";
+  filesystem::FileError error = filesystem::FILE_ERROR_FAILED;
+  GetFSFile(sql_file)->Truncate(size, Capture(&error));
+  GetFSFile(sql_file).WaitForIncomingResponse();
+  if (error != filesystem::FILE_ERROR_OK) {
+    // TODO(erg): Better implementation here.
+    NOTIMPLEMENTED();
+    return SQLITE_IOERR_TRUNCATE;
+  }
+
+  return SQLITE_OK;
+}
+
+int MojoVFSSync(sqlite3_file* mojo_vfs, int flags) {
+  // TODO(erg): We need to flush data. This requires cooperation in
+  // mojo:filesystem.
+  DVLOG(1) << "MojoVFSSync(*, " << flags << ")";
+  NOTIMPLEMENTED();
+
+  return SQLITE_OK;
+}
+
+int MojoVFSFileSize(sqlite3_file* sql_file, sqlite_int64* size) {
+  DVLOG(1) << "MojoVFSFileSize(*)";
+
+  filesystem::FileError err = filesystem::FILE_ERROR_FAILED;
+  filesystem::FileInformationPtr file_info;
+  GetFSFile(sql_file)->Stat(Capture(&err, &file_info));
+  GetFSFile(sql_file).WaitForIncomingResponse();
+
+  if (err != filesystem::FILE_ERROR_OK) {
+    // TODO(erg): Better implementation here.
+    NOTIMPLEMENTED();
+    return SQLITE_IOERR_FSTAT;
+  }
+
+  *size = file_info->size;
+  return SQLITE_OK;
+}
+
+// TODO(erg): The current base::File interface isn't sufficient to handle
+// sqlite's locking primitives, which are done on byte ranges in the file. (See
+// "File Locking Notes" in sqlite3.c.)
+int MojoVFSLock(sqlite3_file* pFile, int eLock) {
+  DVLOG(1) << "MojoVFSLock(*, " << eLock << ")";
+  return SQLITE_OK;
+}
+int MojoVFSUnlock(sqlite3_file* pFile, int eLock) {
+  DVLOG(1) << "MojoVFSUnlock(*, " << eLock << ")";
+  return SQLITE_OK;
+}
+int MojoVFSCheckReservedLock(sqlite3_file* pFile, int* pResOut) {
+  DVLOG(1) << "MojoVFSCheckReservedLock(*)";
+  *pResOut = 0;
+  return SQLITE_OK;
+}
+
+// TODO(erg): This is the minimal implementation to get a few tests passing;
+// lots more needs to be done here.
+int MojoVFSFileControl(sqlite3_file* pFile, int op, void* pArg) {
+  DVLOG(1) << "MojoVFSFileControl(*, " << op << ", *)";
+  if (op == SQLITE_FCNTL_PRAGMA) {
+    // Returning NOTFOUND tells sqlite that we aren't doing any processing.
+    return SQLITE_NOTFOUND;
+  }
+  // TODO(erg): Unsure if we need to manually handle SQLITE_FCNTL_FILE_POINTER.

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

SQLITE_FCNTL_FILE_POINTER is magic, and is handled up in sqlite3_file_control()
in main.c.

[Elliot Glaysher, 2015/06/12 22:36:44]

Done.

+
+  return SQLITE_OK;
+}
+
+int MojoVFSSectorSize(sqlite3_file* pFile) {
+  DVLOG(1) << "MojoVFSSectorSize(*)";
+  NOTIMPLEMENTED();
+  return SQLITE_OK;

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

Returns a value, SQLITE_OK being 0 will just happen to work.  Instead return 0
and SQLite will use SQLITE_DEFAULT_SECTOR_SIZE.

[Elliot Glaysher, 2015/06/12 22:36:43]

Done.

+}
+
+int MojoVFSDeviceCharacteristics(sqlite3_file* pFile) {
+  DVLOG(1) << "MojoVFSDeviceCharacteristics(*)";
+  NOTIMPLEMENTED();
+  return SQLITE_OK;

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

This returns a bitmask.  Again, SQLITE_OK happens to be 0 which will happen to
work, but probably better to just return 0.

[Elliot Glaysher, 2015/06/12 22:36:44]

Done.

+}
+
+static sqlite3_io_methods mojo_vfs_io_methods = {
+    1,                            /* iVersion */
+    MojoVFSClose,                 /* xClose */
+    MojoVFSRead,                  /* xRead */
+    MojoVFSWrite,                 /* xWrite */
+    MojoVFSTruncate,              /* xTruncate */
+    MojoVFSSync,                  /* xSync */
+    MojoVFSFileSize,              /* xFileSize */
+    MojoVFSLock,                  /* xLock */
+    MojoVFSUnlock,                /* xUnlock */
+    MojoVFSCheckReservedLock,     /* xCheckReservedLock */
+    MojoVFSFileControl,           /* xFileControl */
+    MojoVFSSectorSize,            /* xSectorSize */
+    MojoVFSDeviceCharacteristics, /* xDeviceCharacteristics */
+    0,                            /* xShmMap */

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

You don't need to provide these for version 1.  If you're providing them for
future reference WRT things we discussed about memory-mapped files, I think you
need to go all the way to version 3.

I believe that the version 2 stuff is required to enable WAL mode (which I plan
to do across the board at some point), and version 3 is required to do
memory-mapped I/O (which I'd also like to enable by default at some point).

[Elliot Glaysher, 2015/06/12 22:36:44]

Deleted. (I had put it for future reference, but we'll cross that bridge later.)

+    0,                            /* xShmLock */
+    0,                            /* xShmBarrier */
+    0                             /* xShmUnmap */
+};
+
+// who allocates sqlite3_file?

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

The caller provides szOsFile bytes as part of a greater allocation.  It's funky.

[Elliot Glaysher, 2015/06/12 22:36:43]

(Comment was left over from a previous time when I didn't get this. should have
deleted it; the whole placement new thing depends on this and I have the
adjustment of szOsFile otherwise correct below. Deleted.)

+int MojoVFSOpen(sqlite3_vfs* mojo_vfs,
+                const char* name,
+                sqlite3_file* file,
+                int flags,
+                int* pOutFlags) {
+  DVLOG(1) << "MojoVFSOpen(*, " << name << ", *, " << flags << ")";
+  int open_flags = 0;
+  if (flags & SQLITE_OPEN_EXCLUSIVE) {
+    DCHECK(flags & SQLITE_OPEN_CREATE);
+    open_flags = filesystem::kFlagCreate;
+  } else if (flags & SQLITE_OPEN_CREATE) {
+    DCHECK(flags & SQLITE_OPEN_READWRITE);
+    open_flags = filesystem::kFlagOpenAlways;
+  } else {
+    open_flags = filesystem::kFlagOpen;
+  }
+  open_flags |= filesystem::kFlagRead;
+  if (flags & SQLITE_OPEN_READWRITE)
+    open_flags |= filesystem::kFlagWrite;
+
+  // Grab the incoming file
+  filesystem::FilePtr file_ptr;
+  filesystem::FileError error = filesystem::FILE_ERROR_FAILED;
+  GetRootDirectory(mojo_vfs)->OpenFile(mojo::String(name), GetProxy(&file_ptr),
+                                       open_flags, Capture(&error));
+  GetRootDirectory(mojo_vfs).WaitForIncomingResponse();
+  if (error != filesystem::FILE_ERROR_OK) {
+    // TODO(erg): Translate more of the mojo error codes into sqlite error
+    // codes.
+    return SQLITE_CANTOPEN;
+  }
+
+  // Set the method table so we can be closed (and run the manual dtor call to
+  // match the following placement news).
+  file->pMethods = &mojo_vfs_io_methods;
+
+  // |file| is actually a malloced buffer of size szOsFile. This means that we
+  // need to manually use placement new to construct the C++ object which owns
+  // the pipe to our file.
+  new (&GetFSFile(file)) filesystem::FilePtr(file_ptr.Pass());
+
+  return SQLITE_OK;
+}
+
+int MojoVFSDelete(sqlite3_vfs* mojo_vfs, const char* filename, int sync_dir) {
+  DVLOG(1) << "MojoVFSDelete(*, " << filename << ", " << sync_dir << ")";
+  filesystem::FileError error = filesystem::FILE_ERROR_FAILED;
+  GetRootDirectory(mojo_vfs)->Delete(filename, 0, Capture(&error));
+  GetRootDirectory(mojo_vfs).WaitForIncomingResponse();
+
+  // TODO(erg): sync_dir?

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

This may have per-platform warts, unfortunately.  I haven't dug in to see what
they might be.  The default for sql::Connection is to use TRUNCATE mode, so the
transaction state is signaled by zero-sized log rather than missing log, so if
this is wrong we might never notice.  [I don't know what to make of that, but
*shrug*.]

[Elliot Glaysher, 2015/06/12 22:36:43]

I have added a Flush() on sync_dir, which is what the unix vfs does. (syncDir is
unused on Windows.)

(I have also added a comment here about how windows has crazy retry behaviour
and that we might need to support that in the future.)

+
+  return error == filesystem::FILE_ERROR_OK ? SQLITE_OK : SQLITE_IOERR_DELETE;
+}
+
+int MojoVFSAccess(sqlite3_vfs* mojo_vfs,
+                  const char* filename,
+                  int flags,
+                  int* result) {
+  DVLOG(1) << "MojoVFSAccess(*, " << filename << ", " << flags << ", *)";
+  filesystem::FileError error = filesystem::FILE_ERROR_FAILED;
+
+  if (flags == SQLITE_ACCESS_READWRITE || flags == SQLITE_ACCESS_READ) {
+    bool is_wriable = false;

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

is_writable.

[Elliot Glaysher, 2015/06/12 22:36:44]

Done.

+    GetRootDirectory(mojo_vfs)
+        ->IsWritable(filename, Capture(&error, &is_wriable));
+    GetRootDirectory(mojo_vfs).WaitForIncomingResponse();
+    *result = is_wriable;
+    return SQLITE_OK;
+  }
+
+  if (flags == SQLITE_ACCESS_EXISTS) {
+    bool exists = false;
+    GetRootDirectory(mojo_vfs)->Exists(filename, Capture(&error, &exists));
+    GetRootDirectory(mojo_vfs).WaitForIncomingResponse();
+    *result = exists;
+    return SQLITE_OK;
+  }
+
+  return SQLITE_IOERR;

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

So ... does someone guarantee that any file which exists is readable by the
current user?  I realize that the intention would be that things are managed,
etc, but in practice when someone on the Internet mucks up their filesystem,
will the error happen in a sensible place like here, or will it be come later
IOERR on read or write?

+}
+
+int MojoVFSFullPathname(sqlite3_vfs* mojo_vfs,
+                        const char* relative_path,
+                        int absolute_path_size,
+                        char* absolute_path) {
+  // The sandboxed process doesn't need to know the absolute path of the file.
+  sqlite3_snprintf(absolute_path_size, absolute_path, "%s", relative_path);
+  return SQLITE_OK;
+}
+
+// If we are using the mojo:filesystem proxying VFS, then it's highly likely
+// that we are sandboxed and that any attempt to dlopen() a shared object is
+// folly.

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

IMHO you should upgrade this to "Don't let SQLite dynamically load things".  So
far it has never come up.

[Hmm, I see that we halfway-define SQLITE_OMIT_LOAD_EXTENSION , which seems
likely to be a bug.  With that this should never even be called, but I think it
would be best to code it to error rather than CHECK.]

[Elliot Glaysher, 2015/06/12 22:36:44]

Done.

+void* MojoVFSDlOpen(sqlite3_vfs*, const char*) {
+  return 0;
+}
+
+void MojoVFSDlError(sqlite3_vfs*, int buf_size, char* error_msg) {
+  sqlite3_snprintf(buf_size, error_msg, "Dynamic loading not supported");
+}
+
+void (*MojoVFSDlSym(sqlite3_vfs*, void*, const char*))(void) {
+  return 0;
+}
+
+void MojoVFSDlClose(sqlite3_vfs*, void*) {
+  return;
+}
+
+// Proxy the rest of the calls down to the OS specific handler.
+int MojoVFSRandomness(sqlite3_vfs* mojo_vfs, int byte, char* out) {
+  return GetParentVFS(mojo_vfs)->xRandomness(GetParentVFS(mojo_vfs), byte, out);
+}

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

This one in particular might make more sense to implement in terms of our
randomness generator.  The way SQLite's randomness pool is handled has some
funky edges, and we probably already have APIs to implement it in a few lines.

The other functions below might also make sense to implement that way, too. 
xGetLastError() was an experiment that didn't pan out, it could just return 0.

[Elliot Glaysher, 2015/06/12 22:36:44]

The timing stuff looks complicated enough that I'm not going to mess with it,
but I did the randomness and the last error suggestion.

+
+int MojoVFSSleep(sqlite3_vfs* mojo_vfs, int micro) {
+  return GetParentVFS(mojo_vfs)->xSleep(GetParentVFS(mojo_vfs), micro);
+}
+
+int MojoVFSCurrentTime(sqlite3_vfs* mojo_vfs, double* time) {
+  return GetParentVFS(mojo_vfs)->xCurrentTime(GetParentVFS(mojo_vfs), time);
+}
+
+int MojoVFSGetLastError(sqlite3_vfs* mojo_vfs, int a, char* b) {
+  return GetParentVFS(mojo_vfs)->xGetLastError(GetParentVFS(mojo_vfs), a, b);
+}
+
+int MojoVFSCurrentTimeInt64(sqlite3_vfs* mojo_vfs, sqlite3_int64* out) {
+  return GetParentVFS(mojo_vfs)->xCurrentTimeInt64(GetParentVFS(mojo_vfs), out);
+}
+
+static sqlite3_vfs mojo_vfs = {
+    1,                      /* iVersion */
+    sizeof(MojoVFSFile),    /* szOsFile */
+    kMaxPathName,           /* mxPathname */
+    0,                      /* pNext */
+    "mojo",                 /* zName */
+    0,                      /* pAppData */
+    MojoVFSOpen,            /* xOpen */
+    MojoVFSDelete,          /* xDelete */
+    MojoVFSAccess,          /* xAccess */
+    MojoVFSFullPathname,    /* xFullPathname */
+    MojoVFSDlOpen,          /* xDlOpen */
+    MojoVFSDlError,         /* xDlError */
+    MojoVFSDlSym,           /* xDlSym */
+    MojoVFSDlClose,         /* xDlClose */
+    MojoVFSRandomness,      /* xRandomness */
+    MojoVFSSleep,           /* xSleep */
+    MojoVFSCurrentTime,     /* xCurrentTime */
+    MojoVFSGetLastError,    /* xGetLastError */
+    MojoVFSCurrentTimeInt64 /* xCurrentTimeInt64 */
+};
+
+}  // namespace
+
+bool ScopedMojoFilesystemVFS::vfs_set_ = false;
+
+ScopedMojoFilesystemVFS::ScopedMojoFilesystemVFS(
+    filesystem::DirectoryPtr root_directory) {
+  CHECK(!vfs_set_);
+  vfs_set_ = true;

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

This could be implemented in terms of a check on the current default vfs, rather
than a parallel check.

+
+  MojoVFSData* data = new MojoVFSData;
+  data->parent = sqlite3_vfs_find(kParentVFS);

[Scott Hess - ex-Googler, 2015/06/10 22:35:46]

Use NULL to say "The default VFS for this system", and restore the default when
you unregister.

+  data->root_directory = root_directory.Pass();
+  mojo_vfs.pAppData = data;
+
+  mojo_vfs.mxPathname = data->parent->mxPathname;
+
+  CHECK(sqlite3_vfs_register(&mojo_vfs, 1) == SQLITE_OK);
+}
+
+ScopedMojoFilesystemVFS::~ScopedMojoFilesystemVFS() {

[Scott Hess - ex-Googler, 2015/06/10 22:35:45]

Before clearing the pAppData, this needs to reset the default vfs to the
original, and then call sqlite3_vfs_unregister().  I don't know if
sqlite3_vfs_unregister() does the right thing WRT the new default if the
existing default is unregistered, but IMHO it would be  better to restore the
default from point of registration than to rely on that.

[Elliot Glaysher, 2015/06/12 22:36:43]

These three comments about how we deal with the default fs are Done.

+  delete static_cast<MojoVFSData*>(mojo_vfs.pAppData);
+
+  CHECK(vfs_set_);
+  vfs_set_ = false;
+}
+
+filesystem::DirectoryPtr& ScopedMojoFilesystemVFS::GetDirectory() {
+  return static_cast<MojoVFSData*>(mojo_vfs.pAppData)->root_directory;
+}
+
+}  // namespace sql