Bug 37186 – [Chromium] Register Chromium's SQLite VFS even when running in single-process mode

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dumitru Daniliuc]

Chromium's SQLite VFS has not been the default VFS for a while now. We should register it even when !ChromiumBridge::sandboxEnabled() (which is equivalent to running in single-process mode). This will make DBs work correctly with --single-process too.

[Dumitru Daniliuc]

Created attachment 52698 [details]
patch

[Dimitri Glazkov (Google)]

Comment on attachment 52698 [details]
patch

ok.

[Michael Nordman]

like we talked about, this looks like it breaks DOMStorage in chrome?

[Dumitru Daniliuc]

Created attachment 52788 [details]
patch

Make WebSQLDatabases happy while not breaking other features.

[Dumitru Daniliuc]

Created attachment 52795 [details]
patch

Same patch, minus DatabaseTracker changes.

[Jeremy Orlow]

I remember we considered this originally and decided against it, but I can't remember why.  + darin in case he remembers.  If not, I don't see any problem with this other than it seeming a bit ugly.

[Dumitru Daniliuc]

(In reply to comment #6)
> I remember we considered this originally and decided against it, but I can't
> remember why.  + darin in case he remembers.  If not, I don't see any problem
> with this other than it seeming a bit ugly.

I think when we previously talked about this, the Chromium VFS was registered as the default VFS. It _is_ a big ugly, but I don't see any other way of doing this.

[Jeremy Orlow]

Comment on attachment 52795 [details]
patch

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 57235)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,29 @@
> [phone]  Dumitru Daniliuc  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Always register and use Chromium's SQLite VFS for
> +        WebSQLDatabases. Keep using the default VFS in all other
> +        cases. This change should allow Chromium to support
> +        WebSQLDatabases in --single-process mode.
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=37186.

Please follow the standard format for change logs.

Title
url

description

Also, please describe the "full mutex" part of the change.


> Index: WebCore/platform/sql/SQLiteFileSystem.h
> ===================================================================
> --- WebCore/platform/sql/SQLiteFileSystem.h	(revision 57230)
> +++ WebCore/platform/sql/SQLiteFileSystem.h	(working copy)
> @@ -52,7 +52,8 @@ public:
>      // fileName - The name of the database file.
>      // database - The SQLite structure that represents the database stored
>      //            in the given file.
> -    static int openDatabase(const String& fileName, sqlite3** database);
> +    // forWebSQLDatabase - True, if and only if we're opening a Web SQL Database file.

Probably you should explain why this is necessary.  (Chromium needs this info.)

[Dumitru Daniliuc]

Created attachment 52809 [details]
patch

Done. Please take another look.

[Jeremy Orlow]

Comment on attachment 52809 [details]
patch

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 57235)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,35 @@
> [phone]  Dumitru Daniliuc  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Fix Chromium's HTML5 DB support in --single-process mode.
> +

No newline here.

> +        https://bugs.webkit.org/show_bug.cgi?id=37186.
> +
> +        Always register and use Chromium's SQLite VFS for
> +        WebSQLDatabases. Keep using the default VFS in all other
> +        cases. This change should allow Chromium to support
> +        WebSQLDatabases in --single-process mode.
> +
> +        Also, cleaning up a big SQLiteFileSystemChromium and getting rid

This sentence doesn't parse in my head.....a big what?


r=me, provided Michael signs off on it as well and you double check that LocalStorage still works under --single-process.

[Dumitru Daniliuc]

(In reply to comment #10)
> (From update of attachment 52809 [details])
> > Index: WebCore/ChangeLog
> > ===================================================================
> > --- WebCore/ChangeLog	(revision 57235)
> > +++ WebCore/ChangeLog	(working copy)
> > @@ -1,3 +1,35 @@
> > [phone]  Dumitru Daniliuc  <[e-mail]>
> > +
> > +        Reviewed by NOBODY (OOPS!).
> > +
> > +        Fix Chromium's HTML5 DB support in --single-process mode.
> > +
> 
> No newline here.

Removed.

> > +        https://bugs.webkit.org/show_bug.cgi?id=37186.
> > +
> > +        Always register and use Chromium's SQLite VFS for
> > +        WebSQLDatabases. Keep using the default VFS in all other
> > +        cases. This change should allow Chromium to support
> > +        WebSQLDatabases in --single-process mode.
> > +
> > +        Also, cleaning up a big SQLiteFileSystemChromium and getting rid
> 
> This sentence doesn't parse in my head.....a big what?

s/big/bit/.

> r=me, provided Michael signs off on it as well and you double check that
> LocalStorage still works under --single-process.

Will do.

[Michael Nordman]

LGTM

[Dumitru Daniliuc]

Landed as r57245.