WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED CONFIGURATION CHANGED
171781
expose File to web workers
https://bugs.webkit.org/show_bug.cgi?id=171781
Summary
expose File to web workers
Ben Kelly
Reported
2017-05-06 19:27:35 PDT
Bug 156511
implemented the latest File interface, but it appears it did not expose File on web workers. I'd like to try to fix this so that I can reference it in FormData on workers in
bug 171589
.
Attachments
Patch
(8.25 KB, patch)
2017-05-10 20:28 PDT
,
Ben Kelly
no flags
Details
Formatted Diff
Diff
Patch
(8.38 KB, patch)
2017-05-13 18:33 PDT
,
Ben Kelly
no flags
Details
Formatted Diff
Diff
Patch
(10.53 KB, patch)
2017-05-13 19:12 PDT
,
Ben Kelly
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1
2017-05-07 00:09:31 PDT
I would expect the primary challenge to be making this code thread safe. Not much of WebCore is ready to be run on secondary threads.
Ben Kelly
Comment 2
2017-05-07 17:58:59 PDT
Well, in this case File extends Blob which is already exposed on Workers. Adding [Exposed=(Worker)] to the idl lets webkit pass this test:
http://w3c-test.org/FileAPI/file/Worker-read-file-constructor.worker.html
That seems too easy, though.
Ben Kelly
Comment 3
2017-05-10 20:28:58 PDT
Created
attachment 309686
[details]
Patch
Ben Kelly
Comment 4
2017-05-10 20:29:29 PDT
This patch depends on
bug 171960
, so I don't think I can try EWS until that lands.
Ben Kelly
Comment 5
2017-05-13 18:33:15 PDT
Created
attachment 310045
[details]
Patch
Ben Kelly
Comment 6
2017-05-13 19:12:46 PDT
Created
attachment 310049
[details]
Patch
Alexey Proskuryakov
Comment 7
2017-05-14 12:39:13 PDT
What investigations did you perform to ensure that the newly exposed code is safe to run in workers? Personally, I don't know how much has been done for Blobs either, so those might not be very safe.
Ben Kelly
Comment 8
2017-05-14 20:50:54 PDT
(In reply to Alexey Proskuryakov from
comment #7
)
> What investigations did you perform to ensure that the newly exposed code is > safe to run in workers?
Currently, File extends Blob and adds three extra parameters: String m_path; String m_name; std::optional<int64_t> m_overrideLastModifiedDate; AFAICT these are all immutable after construction. I don't see anything obviously dangerous with this, assuming the Blob base class is adequately supported on workers.
> Personally, I don't know how much has been done for Blobs either, so those > might not be very safe.
I looked at Blob.idl blame and it appears that its been exposed to workers for at least 4 years. I didn't try tracing it back further. Features like IDB depend on Blob as well. There are a number of worker blob tests cases in: LayoutTests/fast/files/workers The tests I'm trying to import in
bug 171960
provide a worker test that exercises the File() constructor and its additional getters. I've wondered if there is something I need to do to support structure clone on workers properly. I haven't gotten as far as digging into that yet. Any pointers on how structured clone is implemented are appreciated. Anything else you recommend investigating? Thanks.
Alexey Proskuryakov
Comment 9
2017-05-15 09:13:22 PDT
String objects can't be shared across threads, so any time they are passed over, there needs to be a copy made. More generally, it's not just about the object data, but also about the implementation. For example, are there are IPC messages sent?
> // FIXME: This should be exposed on Workers as well.
The person who added this FIXME must have expected the effort to be larger than a one line change. Who added it, what does svn blame say?
Chris Dumez
Comment 10
2017-05-15 09:21:35 PDT
(In reply to Alexey Proskuryakov from
comment #9
)
> String objects can't be shared across threads, so any time they are passed > over, there needs to be a copy made. More generally, it's not just about the > object data, but also about the implementation. For example, are there are > IPC messages sent? > > > // FIXME: This should be exposed on Workers as well. > > The person who added this FIXME must have expected the effort to be larger > than a one line change. Who added it, what does svn blame say?
Sam Weinig.
Chris Dumez
Comment 11
2017-05-15 09:28:02 PDT
(In reply to Alexey Proskuryakov from
comment #9
)
> String objects can't be shared across threads, so any time they are passed > over, there needs to be a copy made. More generally, it's not just about the > object data, but also about the implementation. For example, are there are > IPC messages sent? > > > // FIXME: This should be exposed on Workers as well. > > The person who added this FIXME must have expected the effort to be larger > than a one line change. Who added it, what does svn blame say?
As long as the File object is not passed across threads, then we'd be fine. I am not super familiar with our current File API implementation but assuming the File object cannot be passed across thread, then it should not matter if we are in a worker thread or the main thread. It seems File has a constructor and I expect this constructor would just work in a worker thread.
Alexey Proskuryakov
Comment 12
2017-05-15 09:56:35 PDT
I recall there being quite a bit of IPC, possibly even sync IPC - which is not supported in non-main threads.
Chris Dumez
Comment 13
2017-05-15 10:02:53 PDT
(In reply to Alexey Proskuryakov from
comment #12
)
> I recall there being quite a bit of IPC, possibly even sync IPC - which is > not supported in non-main threads.
Yes, I believe there will be IPC in WK2 but it is via ThreadableBlobRegistry::registerFileBlobURL() which properly deals with threads.
Ben Kelly
Comment 14
2017-05-15 10:17:31 PDT
(In reply to Chris Dumez from
comment #11
)
> It seems File has a constructor and I expect this constructor would just > work in a worker thread.
The other ways you can produce a File in a worker are at least: 1. postMessage() 2. IDB Both of these are based on structured clone. I can write some tests for these, but if anyone has info on how structured clone is implemented for DOM objects in webkit that would be helpful too.
Sam Weinig
Comment 15
2020-09-30 20:12:48 PDT
File has been exposed to workers for a while at this point, I don't think there is anything left to do here.
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