WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
49940
[Chromium] Implement FileWriterSync
https://bugs.webkit.org/show_bug.cgi?id=49940
Summary
[Chromium] Implement FileWriterSync
Eric U.
Reported
2010-11-22 14:28:22 PST
Add the Chromium-specific implementation for FileWriterSync.
Attachments
Chromium bits to implement AsyncFileWriter support for synchronous operations
(6.64 KB, patch)
2010-11-22 17:26 PST
,
Eric U.
levin
: review-
Details
Formatted Diff
Diff
Added a mode for the runloop.
(7.38 KB, patch)
2010-11-24 11:40 PST
,
Eric U.
levin
: review-
Details
Formatted Diff
Diff
Fixed runloop to have a unique ID.
(7.57 KB, patch)
2010-11-24 14:41 PST
,
Eric U.
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Eric U.
Comment 1
2010-11-22 17:26:48 PST
Created
attachment 74620
[details]
Chromium bits to implement AsyncFileWriter support for synchronous operations See
https://bugs.webkit.org/show_bug.cgi?id=49939
for context, but the patches can go in in whichever order.
David Levin
Comment 2
2010-11-23 20:44:49 PST
Comment on
attachment 74620
[details]
Chromium bits to implement AsyncFileWriter support for synchronous operations View in context:
https://bugs.webkit.org/attachment.cgi?id=74620&action=review
> WebKit/chromium/src/WorkerFileWriterCallbacksBridge.cpp:208 > + if (context->thread()->runLoop().runInMode(context, WorkerRunLoop::defaultMode()) == MessageQueueTerminated)
Don't run a nested loop in defaultMode. It should have a name (and ideally have a number appended). See other places where this is done. Other than that this change looks great!
Eric U.
Comment 3
2010-11-24 11:40:53 PST
Created
attachment 74784
[details]
Added a mode for the runloop. I decided not to make a unique per-invocation [numbered] mode string. It's extra string manipulation on every call for a feature that may never be needed, and it'll be easy to add later. I did put in a warning comment about it, though.
David Levin
Comment 4
2010-11-24 13:59:44 PST
Comment on
attachment 74784
[details]
Added a mode for the runloop. View in context:
https://bugs.webkit.org/attachment.cgi?id=74784&action=review
> WebKit/chromium/src/WorkerFileWriterCallbacksBridge.cpp:205 > + m_proxy->postTaskForModeToWorkerContext(createCallbackTask(&runTaskOnWorkerThread, this, task), fileWriterOperationsMode);
Ok so this posts to a mode for the sync calls and the async calls. In other words, it is using a mode for the async calls when it really isn't trying to post to a nested message loop. That will work and results in less code... (it feels mildly odd for some reason but) ok.
David Levin
Comment 5
2010-11-24 14:03:28 PST
Comment on
attachment 74784
[details]
Added a mode for the runloop. Actually I think the single mode is broken.
David Levin
Comment 6
2010-11-24 14:04:14 PST
(In reply to
comment #5
)
> (From update of
attachment 74784
[details]
) > Actually I think the single mode is broken.
If this function is used by async functions for callbacks, then those callbacks will happen during this sync call and that would be bad.
Eric U.
Comment 7
2010-11-24 14:41:49 PST
Created
attachment 74796
[details]
Fixed runloop to have a unique ID.
David Levin
Comment 8
2010-11-24 15:18:54 PST
Comment on
attachment 74796
[details]
Fixed runloop to have a unique ID. Thanks!
WebKit Commit Bot
Comment 9
2010-11-24 17:25:09 PST
The commit-queue encountered the following flaky tests while processing
attachment 74796
[details]
: fast/css/font-face-download-error.html animations/suspend-resume-animation-events.html Please file bugs against the tests. These tests were authored by
cmarrin@apple.com
and
yuzo@google.com
. The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 10
2010-11-24 17:55:36 PST
Comment on
attachment 74796
[details]
Fixed runloop to have a unique ID. Clearing flags on attachment: 74796 Committed
r72711
: <
http://trac.webkit.org/changeset/72711
>
WebKit Commit Bot
Comment 11
2010-11-24 17:55:42 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug