Bug 80325

Summary: Merge AsyncFileStream with FileStreamProxy
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: WebCore Misc.Assignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: dimich, ericu, fpizlo, japhet, jianli, kinuko, levin+threading, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 79327    
Attachments:
Description Flags
proposed patch
none
updated to ToT fpizlo: review+, webkit.review.bot: commit-queue-

Alexey Proskuryakov
Reported 2012-03-05 13:46:38 PST
There is only one subclass.
Attachments
proposed patch (68.13 KB, patch)
2012-03-05 14:16 PST, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
updated to ToT (68.37 KB, patch)
2012-03-05 14:58 PST, Alexey Proskuryakov 		fpizlo: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2012-03-05 14:16:31 PST
Created attachment 130204 [details] proposed patch
Alexey Proskuryakov
Comment 2 2012-03-05 14:19:18 PST
ResourceHandle still serves as interface to blob: URL loading, and has a subclass, but it no longer knows anything about these directly.
Alexey Proskuryakov
Comment 3 2012-03-05 14:20:29 PST
So this improves modularization of WebKit, too.
Alexey Proskuryakov
Comment 4 2012-03-05 14:58:56 PST
Created attachment 130209 [details] updated to ToT Would like EWS to run...
Filip Pizlo
Comment 5 2012-03-06 22:53:40 PST
Comment on attachment 130209 [details] updated to ToT View in context: https://bugs.webkit.org/attachment.cgi?id=130209&action=review I am not an expert on this code, but this feels like a refactoring step that leaves the code in an irregular state. Is this the only way to do it? > Source/WebCore/ChangeLog:26 > + * fileapi/AsyncFileStream.cpp: Copied from Source/WebCore/fileapi/FileStreamProxy.cpp. It feels strange to me that this creates files called "AsyncFileStream.[h|cpp]" that have a class called "FileStreamProxy". > Source/WebCore/fileapi/AsyncFileStream.h:37 > +#include "AsyncFileStream.h" Is this file including itself?
Alexey Proskuryakov
Comment 6 2012-03-06 23:03:21 PST
> I am not an expert on this code, but this feels like a refactoring step that leaves the code in an irregular state. Is this the only way to do it? It does not. This is how svn-create-patch works for moved files - it first shows the whole moved file, and then modifications to it. > Is this file including itself? Yes, that include is wrong and should be removed.
Filip Pizlo
Comment 7 2012-03-06 23:10:23 PST
(In reply to comment #6) > > I am not an expert on this code, but this feels like a refactoring step that leaves the code in an irregular state. Is this the only way to do it? > > It does not. This is how svn-create-patch works for moved files - it first shows the whole moved file, and then modifications to it. Aha!! I was massively confused, apparently. > > > Is this file including itself? > > Yes, that include is wrong and should be removed. Looks like you did remove the #include. I just missed it because of my confusion regarding how svn-create-patch worked.
Filip Pizlo
Comment 8 2012-03-06 23:15:57 PST
Comment on attachment 130209 [details] updated to ToT R=me. After learning how to grok svn-create-patch's handling of moved-and-modified files, I find this patch to be perfectly sensible. :-)
Alexey Proskuryakov
Comment 9 2012-03-07 08:54:28 PST
Comment on attachment 130209 [details] updated to ToT Thank you for review!
WebKit Review Bot
Comment 10 2012-03-07 08:57:57 PST
Comment on attachment 130209 [details] updated to ToT Rejecting attachment 130209 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ileStream.h' patching file Source/WebCore/platform/network/BlobRegistry.h patching file Source/WebCore/platform/network/BlobRegistryImpl.cpp patching file Source/WebCore/platform/network/BlobRegistryImpl.h patching file Source/WebCore/platform/network/ResourceHandle.cpp patching file Source/WebCore/platform/network/ResourceHandle.h Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Filip Pizlo']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/11852116
Alexey Proskuryakov
Comment 11 2012-03-07 09:48:36 PST
Committed <http://trac.webkit.org/changeset/110066>. Post-landing comments from CC'ed experts are still welcome.
Alexey Proskuryakov
Comment 12 2012-03-07 11:58:53 PST
Build fix in <http://trac.webkit.org/changeset/110080>.
Note You need to log in before you can comment on or make changes to this bug.