Bug 49940 – [Chromium] Implement FileWriterSync

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Eric U.]

Add the Chromium-specific implementation for FileWriterSync.

[Eric U.]

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 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.]

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 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 on attachment 74784 [details]
Added a mode for the runloop.

Actually I think the single mode is broken.

[David Levin]

(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.]

Created attachment 74796 [details]
Fixed runloop to have a unique ID.

[David Levin]

Comment on attachment 74796 [details]
Fixed runloop to have a unique ID.

Thanks!

[WebKit Commit Bot]

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 [e-mail] and [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

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]

All reviewed patches have been landed.  Closing bug.