[Webkit-unassigned] [Bug 26940] [Chromium] Add a SQLite VFS for Chromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26940





--- Comment #7 from Dumitru Daniliuc <dumi at chromium.org>  2009-07-06 18:10:41 PDT ---
(In reply to comment #4)
> (From update of attachment 32206 [details])
> > Index: WebCore/platform/sql/chromium/SQLiteFileSystemChromium.cpp
> 
> If this is a Windows-specific file, it should be
> SQLiteFileSystemChromiumWin.cpp
> 
> > +        if (desiredFlags & SQLITE_OPEN_READWRITE) {
> > +            int newFlags = (desiredFlags | SQLITE_OPEN_READONLY) & ~SQLITE_OPEN_READWRITE;
> > +            return chromiumOpen(0, fileName, id, newFlags, usedFlags);
> > +        } else {
> > +            return SQLITE_CANTOPEN;
> > +	}
> 
> Fix indentation, no need for brackets for one-liners.

removed the brackets.

> > +#ifndef SQLITE_OMIT_LOAD_EXTENSION
> > +// Returns NULL, thus disallowing loading libraries in the renderer process.
> > +//
> > +// vfs - pointer to the sqlite3_vfs object.
> > +// fileName - the name of the shared library file.
> > +void* chromiumDlOpen(sqlite3_vfs* vfs, const char* fileName)
> > +{
> > +    return NULL;
> 
> return 0;

done.

> > +    static sqlite3_vfs chromium_vfs = {
> > +      1,
> > +      win32_vfs->szOsFile,
> > +      win32_vfs->mxPathname,
> > +      0,
> > +      "chromium_vfs",
> > +      0,
> > +      chromiumOpen,
> > +      chromiumDelete,
> > +      chromiumAccess,
> > +      chromiumFullPathname,
> > +      chromiumDlOpen,
> > +      win32_vfs->xDlError,
> > +      win32_vfs->xDlSym,
> > +      win32_vfs->xDlClose,
> > +      win32_vfs->xRandomness,
> > +      win32_vfs->xSleep,
> > +      win32_vfs->xCurrentTime,
> > +      win32_vfs->xGetLastError
> 
> 4 space indent.

done.

> > +
> > +bool SQLiteFileSystem::ensureDatabaseDirectoryExists(const String& path)
> > +{
> 
> Should probably eliminate unused param for consistency.

fixed everywhere.

> > +bool SQLiteFileSystem::ensureDatabaseFileExists(const String& fileName, bool checkPathOnly)
> > +{
> 
> Ditto.

done.

> > +bool SQLiteFileSystem::deleteEmptyDatabaseDirectory(const String& path)
> > +{
> 
> Ditto.

done.

> Also, instead of including windows.h and having platform-specific #ifdef in
> ChromiumBridge, just use PlatformFileHandle from FileSystem.h.

i didn't change this.

from what i understand, webkit has only one chromium port: PLATFORM(CHROMIUM).
it doesn't know how to differentiate between chromium compiled on windows and
chromium compiled on linux/mac. so we can't use #defines in FileSystem.h.
basically, if we define something in FileSystem.h for PLATFORM(CHROMIUM), we'd
have to use those definitions for all OSes we're compiling chromium on, and it
just doesn't work.

i talked a bit to john about this and he said that separate, per-OS function
signatures should be fine in this case.

let me know what you think.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.