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-
Added a mode for the runloop. (7.38 KB, patch)
2010-11-24 11:40 PST, Eric U.
levin: review-
Fixed runloop to have a unique ID. (7.57 KB, patch)
2010-11-24 14:41 PST, Eric U.
no flags
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.