Bug 91631 - [Chromium] Call SQLiteFileSystem-related functions directly
Summary: [Chromium] Call SQLiteFileSystem-related functions directly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Pilgrim (Google)
URL:
Keywords:
Depends on:
Blocks: 82948
  Show dependency treegraph
 
Reported: 2012-07-18 09:26 PDT by Mark Pilgrim (Google)
Modified: 2012-07-18 15:51 PDT (History)
9 users (show)

See Also:


Attachments
Patch (15.20 KB, patch)
2012-07-18 09:27 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (15.16 KB, patch)
2012-07-18 11:45 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2012-07-18 09:26:26 PDT
[Chromium] Call SQLiteFileSystem-related functions directly
Comment 1 Mark Pilgrim (Google) 2012-07-18 09:27:46 PDT
Created attachment 153028 [details]
Patch
Comment 2 WebKit Review Bot 2012-07-18 09:29:24 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Adam Barth 2012-07-18 09:40:37 PDT
Comment on attachment 153028 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=153028&action=review

> Source/Platform/chromium/public/Platform.h:120
> +    virtual FileHandle databaseOpenFile(
> +                                        const WebString& vfsFileName, int desiredFlags) { return FileHandle(); }

I would just merge these lines.
Comment 4 Mark Pilgrim (Google) 2012-07-18 11:45:54 PDT
Created attachment 153061 [details]
Patch
Comment 5 Mark Pilgrim (Google) 2012-07-18 11:46:17 PDT
Comment on attachment 153061 [details]
Patch

Nits addressed.
Comment 6 WebKit Review Bot 2012-07-18 13:51:45 PDT
Comment on attachment 153061 [details]
Patch

Clearing flags on attachment: 153061

Committed r123014: <http://trac.webkit.org/changeset/123014>
Comment 7 WebKit Review Bot 2012-07-18 13:51:51 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Tony Chang 2012-07-18 14:52:38 PDT
This broke the windows compile:
11>..\platform\sql\chromium\SQLiteFileSystemChromiumWin.cpp(85): error C2660: 'WebKit::Platform::databaseDeleteFile' : function does not take 1 arguments

I tried to fix it in http://trac.webkit.org/changeset/123023 .  Please double check the change.
Comment 9 Adam Barth 2012-07-18 15:02:49 PDT
> I tried to fix it in http://trac.webkit.org/changeset/123023 .  Please double check the change.

Thanks for fixing the build!  Yes, that's the right fix.
Comment 10 Ryosuke Niwa 2012-07-18 15:30:31 PDT
Still broken:
http://build.webkit.org/builders/Chromium%20Win%20Release/builds/46439/steps/compile-webkit/logs/stdio
7>e:\google-windows-1\chromium-win-release\build\source\webkit\chromium\public\platform\../../../../Platform/chromium/public/Platform.h(74): error C2146: syntax error : missing ';' before identifier 'FileHandle'
7>e:\google-windows-1\chromium-win-release\build\source\webkit\chromium\public\platform\../../../../Platform/chromium/public/Platform.h(74): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int

It's probably complaining that HANDLE isn't defined.
Comment 11 Adam Barth 2012-07-18 15:44:48 PDT
> It's probably complaining that HANDLE isn't defined.

Maybe we should roll out the patch and try again.
Comment 12 Ryosuke Niwa 2012-07-18 15:51:54 PDT
Landed another build fix in http://trac.webkit.org/changeset/123034.

Please be more careful in the future. In particular, if you're creating a patch with #if-def for Windows, then please build it on Windows.