Comment on attachment 606821 [details] [diff] [review] Part 2: Add Initinternal method to nsDOMMultipartFile Maybe rename "GetNative" to "GetXPConnectNative" or some such. And please add a typedef for the function pointer type. The c-syntax for function pointers is horrible.

Comment on attachment 606822 [details] [diff] [review] Part 3: Implement Blob constructor in Worker Review of attachment 606822 [details] [diff] [review]: ----------------------------------------------------------------- Feel free to request bent's review too if you want it.

I won't review Part 1 since I don't know whether we need something equivalent to context pushing on the worker thread. Also, you rock!!!

Comment on attachment 606822 [details] [diff] [review] Part 3: Implement Blob constructor in Worker I'd like to look at this, yeah.

Folded part 2 & 3 and resolved review comments.

Comment on attachment 606828 [details] [diff] [review] Part 2: Implement Blob constructor in Worker. r=jonas Review of attachment 606828 [details] [diff] [review]: ----------------------------------------------------------------- This looks great, thanks! ::: dom/workers/File.cpp @@ +112,5 @@ > return NULL; > } > > + static nsIDOMBlob* > + GetPrivate(JSContext* aCx, JSObject* aObj) { Nit: I'd name this differently, something like "Unwrap" maybe? Also, please put the { on the next line.

renamed a function

I'm kind of wary of using nsCxPusher off the main thread.

Comment on attachment 606820 [details] [diff] [review] Part 1: Make dictionary initializers callable off main thread Review of attachment 606820 [details] [diff] [review]: ----------------------------------------------------------------- It's technically safe now, but that might change in the future. I'd like to avoid nsCxPusher entirely off the main thread.

Kyle, how would we do that while still keeping the nsCxPusher as a guard object? I had the same concern as you originally, but it seems like any alternative will make it a lot harder to write code that runs on both the main thread and a worker thread, something that we should do more of.

I added a wrapper class to ensure that nsCxPusher will never be even instantiated off the main thread.

Replaced a reinterpret_cast, a reference and a boolean flag with a pointer.

Comment on attachment 608740 [details] [diff] [review] Part 1: Make dictionary initializers callable off main thread Review of attachment 608740 [details] [diff] [review]: ----------------------------------------------------------------- Much better, but can we use Maybe (http://mxr.mozilla.org/mozilla-central/source/mfbt/Util.h#245) instead of rolling our own here. It does some stuff with alignment that we might need here.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #16) > Much better, but can we use Maybe > (http://mxr.mozilla.org/mozilla-central/source/mfbt/Util.h#245) instead of > rolling our own here. It does some stuff with alignment that we might need > here. Oh, I didn't know about that. Thanks for the advice. I've stopped reinventing the wheel.

Comment on attachment 608759 [details] [diff] [review] Part 1: Make dictionary initializers callable off main thread Review of attachment 608759 [details] [diff] [review]: ----------------------------------------------------------------- Excellent. Thanks for bearing with me on this.

Autoland Patchset: Patches: 608759, 606940 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=8b3417b99101 Try run started, revision 8b3417b99101. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=8b3417b99101

Try run for 8b3417b99101 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=8b3417b99101 Results (out of 225 total builds): exception: 1 success: 185 warnings: 39 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-8b3417b99101

http://hg.mozilla.org/integration/mozilla-inbound/rev/120e179a4af7 http://hg.mozilla.org/integration/mozilla-inbound/rev/6009d836249f

https://hg.mozilla.org/mozilla-central/rev/120e179a4af7 https://hg.mozilla.org/mozilla-central/rev/6009d836249f

Doc updated: https://developer.mozilla.org/en-US/Firefox/Releases/14 https://developer.mozilla.org/en-US/docs/Web/API/Blob.Blob