Bug 171781

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.

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.

Created attachment 309686 [details] Patch

This patch depends on bug 171960, so I don't think I can try EWS until that lands.

Created attachment 310045 [details] Patch

Created attachment 310049 [details] Patch

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.

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

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?

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

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

I recall there being quite a bit of IPC, possibly even sync IPC - which is not supported in non-main threads.

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

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

File has been exposed to workers for a while at this point, I don't think there is anything left to do here.