Bug 79449

Summary: Move FILE_SYSTEM code out of WorkerContext and into the fileapi folder
Product: WebKit Reporter: Ashod Nakashian <ashodnakashian>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, gns, gustavo.noronha, ojan, pnormand, rakuco, vestbo, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 79327    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Ashod Nakashian 2012-02-23 23:07:13 PST
Move FILE_SYSTEM code out of WorkerContext and into the fileapi/WorkerContextFileSystem folder[1].

[1]: https://docs.google.com/spreadsheet/ccc?key=0AppchfQ5mBrEdFlodHlLb0prdEd1ZEZyUHdCbEpoc2c#gid=0
Comment 1 Ashod Nakashian 2012-02-25 05:39:51 PST
Created attachment 128863 [details]
Patch
Comment 2 WebKit Review Bot 2012-02-25 05:42:56 PST
Attachment 128863 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/fileapi/WorkerContextFileSystem.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Gustavo Noronha (kov) 2012-02-25 05:55:20 PST
Comment on attachment 128863 [details]
Patch

Attachment 128863 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11632335
Comment 4 Ashod Nakashian 2012-02-25 06:07:23 PST
Created attachment 128864 [details]
Patch
Comment 5 WebKit Review Bot 2012-02-25 06:35:18 PST
Comment on attachment 128864 [details]
Patch

Attachment 128864 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11627441
Comment 6 Philippe Normand 2012-02-25 07:14:13 PST
Comment on attachment 128864 [details]
Patch

Attachment 128864 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11629405
Comment 7 Collabora GTK+ EWS bot 2012-02-25 08:28:08 PST
Comment on attachment 128864 [details]
Patch

Attachment 128864 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11626400
Comment 8 Adam Barth 2012-02-25 21:58:40 PST
Comment on attachment 128864 [details]
Patch

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

Looks like you're having some trouble building.

> Source/WebCore/fileapi/WorkerContextFileSystem.h:44
> +    void webkitRequestFileSystem(int type, long long size, PassRefPtr<FileSystemCallback> successCallback, PassRefPtr<ErrorCallback>);
> +    PassRefPtr<DOMFileSystemSync> webkitRequestFileSystemSync(int type, long long size, ExceptionCode&);
> +    void webkitResolveLocalFileSystemURL(const String& url, PassRefPtr<EntryCallback> successCallback, PassRefPtr<ErrorCallback>);
> +    PassRefPtr<EntrySync> webkitResolveLocalFileSystemSyncURL(const String& url, ExceptionCode&);

These all should be static and take a WorkerContext as the first argument.

Also, WorkerContextFileSystem should have a private constructor and destructor, like DOMWindowFileSystem.

> Source/WebCore/workers/WorkerContext.h:130
> +        // FIXME: Move these constants to WorkerContextFileSystem.

You should be able to move these now.  We didn't move them right away in the other patch because of a bug in the code generator that has since been fixed.
Comment 9 Ashod Nakashian 2012-02-26 07:57:42 PST
Created attachment 128915 [details]
Patch
Comment 10 Philippe Normand 2012-02-26 08:03:09 PST
Comment on attachment 128915 [details]
Patch

Attachment 128915 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11627727
Comment 11 WebKit Review Bot 2012-02-26 08:03:21 PST
Attachment 128915 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Ashod Nakashian 2012-02-26 08:14:53 PST
Created attachment 128917 [details]
Patch
Comment 13 Philippe Normand 2012-02-26 08:18:47 PST
Comment on attachment 128917 [details]
Patch

Attachment 128917 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11626667
Comment 14 Philippe Normand 2012-02-26 08:27:49 PST
Comment on attachment 128917 [details]
Patch

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

> Source/WebCore/GNUmakefile.list.am:4871
> +	$(WebCore)/fileapi/WorkerContextFileSystem.idl \

Remove trailing backslah. That should make EWS happier.
Comment 15 Ashod Nakashian 2012-02-26 08:37:41 PST
Created attachment 128918 [details]
Patch
Comment 16 Philippe Normand 2012-02-26 08:42:17 PST
Comment on attachment 128918 [details]
Patch

Attachment 128918 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11627736
Comment 17 Philippe Normand 2012-02-26 08:51:15 PST
Comment on attachment 128918 [details]
Patch

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

> Source/WebCore/GNUmakefile.list.am:4871
>  	$(WebCore)/fileapi/MetadataCallback.idl
> +	$(WebCore)/fileapi/WorkerContextFileSystem.idl \

This won't work either. Move the backslash to the end of previous line:

$(WebCore)/fileapi/MetadataCallback.idl \
$(WebCore)/fileapi/WorkerContextFileSystem.idl

(without forgetting the tabulations :)
Comment 18 Adam Barth 2012-02-26 09:35:25 PST
Comment on attachment 128918 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

Looks like you're getting close.  You'll need to remove this land before we can land your patch.  I'd probably replace it with a sentence that says that there shouldn't be any change in behavior from this patch (and therefore there's nothing to test).
Comment 19 Ashod Nakashian 2012-02-26 11:13:50 PST
Created attachment 128923 [details]
Patch
Comment 20 WebKit Review Bot 2012-02-26 11:44:11 PST
Comment on attachment 128923 [details]
Patch

Attachment 128923 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11632682
Comment 21 Ashod Nakashian 2012-02-26 20:25:18 PST
Created attachment 128942 [details]
Patch
Comment 22 Ashod Nakashian 2012-02-28 09:24:36 PST
The patch has passed the build scripts and is ready for review.
Comment 23 Adam Barth 2012-02-28 11:04:22 PST
Comment on attachment 128942 [details]
Patch

Looks great.  Thanks.
Comment 24 WebKit Review Bot 2012-02-28 11:53:12 PST
Comment on attachment 128942 [details]
Patch

Clearing flags on attachment: 128942

Committed r109133: <http://trac.webkit.org/changeset/109133>
Comment 25 WebKit Review Bot 2012-02-28 11:53:21 PST
All reviewed patches have been landed.  Closing bug.