845545 - Use the cycle collector in workers

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 1 - Create a worker implementation of CycleCollectedJSRuntime

This is mostly moving code around.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 2 - Port ImageData to workers

Make the main thread implementation of ImageData work on worker threads.

Comment 3

[Andrew McCreight [:mccr8]]

Comment on attachment 766726 [details] [diff] [review]
Part 1 - Create a worker implementation of CycleCollectedJSRuntime

Review of attachment 766726 [details] [diff] [review]:
-----------------------------------------------------------------

Looks okay to me, with the proviso I know nothing about workers. ;)

::: dom/workers/RuntimeService.cpp
@@ +886,5 @@
>  
>      workerPrivate->AssertIsOnWorkerThread();
>  
> +    {
> +      nsCycleCollector_startup(CCSingleThread);

I made sure to double check this call...

@@ +891,3 @@
>  
> +      JSContext* cx;
> +      WorkerJSRuntime runtime(workerPrivate, &cx);

So, Run() really lives as long as the worker?  Kind of weird that it is a local variable.

@@ +899,2 @@
>  
> +      JSRuntime* rt = JS_GetRuntime(cx);

Maybe assert that the runtime of cx is the same as the runtime of WorkerJSRuntime?

Comment 4

[Andrew McCreight [:mccr8]]

Comment on attachment 766728 [details] [diff] [review]
Part 2 - Port ImageData to workers

Review of attachment 766728 [details] [diff] [review]:
-----------------------------------------------------------------

r- for the layout statics stuff.

I only lightly reviewed the bindings and worker stuff.

::: dom/workers/RuntimeService.cpp
@@ -839,5 @@
>    }
>  
>    ~WorkerJSRuntime()
>    {
> -    nsCycleCollector_forgetJSRuntime();

Technically, this should go in the other patch. ;)

::: dom/workers/WorkerPrivate.cpp
@@ +256,5 @@
>      }
>  
>      // See if this is an ImageData object.
> +    {
> +      ImageData* imageData = nullptr;

Does this need to be a refPtr?

@@ +261,5 @@
> +      if (NS_SUCCEEDED(UnwrapObject<ImageData>(aCx, aObj, imageData))) {
> +        // Prepare the ImageData internals.
> +        uint32_t width = imageData->Width();
> +        uint32_t height = imageData->Height();
> +        JS::Rooted<JSObject*> dataArray(aCx, imageData->GetDataObject());

Do you really need to root |dataArray|?

::: dom/workers/WorkerScope.cpp
@@ +1034,5 @@
>    }
>  
>    // Init other paris-bindings.
>    if (!FileReaderSyncBinding_workers::GetConstructorObject(aCx, global) ||
> +      !ImageDataBinding::GetConstructorObject(aCx, global) ||

This is going to get a little tedious to manually write if we keep adding these. :)

::: xpcom/base/CycleCollectedJSRuntime.cpp
@@ -835,5 @@
>  void
>  CycleCollectedJSRuntime::AddJSHolder(void* aHolder, nsScriptObjectTracer* aTracer)
>  {
>    MOZ_ASSERT(aTracer->Trace, "AddJSHolder needs a non-null Trace function");
> -  bool wasEmpty = mJSHolders.Count() == 0;

Why is this okay to remove the LayoutStatics stuff?

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 766728 [details] [diff] [review]
Part 2 - Port ImageData to workers

The nsLayoutStatics stuff existed to keep nsContentUtils from shutting down when we still had outstanding JS holders.  If we let nsContentUtils shut down nsContentUtils::sXPConnect would be null and NS_HOLD/DROP_JS_OBJECTS would crash with a null deref.  This is no longer an issue since we go through the cycle collector code instead of XPConnect, and the pointer to the runtime is available until we get to the runtime's dtor.

I removed it in this patch because AddRef/Releaseing nsLayoutStatics off the main thread has predictably bad results.

See bug 810618 for an example of what used to go wrong.

Comment 6

[Andrew McCreight [:mccr8]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #5)
> The nsLayoutStatics stuff existed to keep nsContentUtils from shutting down
> when we still had outstanding JS holders.  If we let nsContentUtils shut
> down nsContentUtils::sXPConnect would be null and NS_HOLD/DROP_JS_OBJECTS
> would crash with a null deref.  This is no longer an issue since we go
> through the cycle collector code instead of XPConnect, and the pointer to
> the runtime is available until we get to the runtime's dtor.

Thanks for the explanation!  I was figuring something like that, but I wasn't sure off the top of my head precisely how it was going.

> I removed it in this patch because AddRef/Releaseing nsLayoutStatics off the
> main thread has predictably bad results.
Yeah, I realized that seemed super bad. ;)

So, the patches that are here so far don't deal with scheduling of the CC in workers, you will only run them at shutdown, right?  I assume there's another patch coming that will do something for that?

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(Just to manage expectations it is extremely unlikely that I'll be able to get to this review this week)

Comment 8

[Boris Zbarsky [:bzbarsky]]

> This is no longer an issue 

Does that mean we can remove the contentutils refcounting in CallbackObject as well (probably in a separate bug)?

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I believe so, but yes, separate bug.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Filed bug 887116.

Comment 11

[Peter Van der Beken [:peterv]]

Comment on attachment 766728 [details] [diff] [review]
Part 2 - Port ImageData to workers

Review of attachment 766728 [details] [diff] [review]:
-----------------------------------------------------------------

I had to pinch myself after reading this :-). ++khuey.

::: xpcom/base/CycleCollectedJSRuntime.cpp
@@ -835,5 @@
>  void
>  CycleCollectedJSRuntime::AddJSHolder(void* aHolder, nsScriptObjectTracer* aTracer)
>  {
>    MOZ_ASSERT(aTracer->Trace, "AddJSHolder needs a non-null Trace function");
> -  bool wasEmpty = mJSHolders.Count() == 0;

Blindly removing the nsLayoutStatics stuff seems dangerous. Are you sure it's safe? We could move it behind a main-thread check for now if needed? Though we'll have to figure out how to make various code deal with the fact that they can't use nsLayoutStatics off the main-thread as we share the code between workers and main-thread DOM.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Part 2 looks pretty awesome, yeah.

Why is the change from a dict to a list in Bindings.conf needed?

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Boris Zbarsky (:bz) (reading mail, but on paternity leave) from comment #12)
> Part 2 looks pretty awesome, yeah.
> 
> Why is the change from a dict to a list in Bindings.conf needed?

It's not, I'll remove it.  An artifact of iterating.

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Peter Van der Beken [:peterv] from comment #11)
> Comment on attachment 766728 [details] [diff] [review]
> Part 2 - Port ImageData to workers
> 
> Review of attachment 766728 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I had to pinch myself after reading this :-). ++khuey.
> 
> ::: xpcom/base/CycleCollectedJSRuntime.cpp
> @@ -835,5 @@
> >  void
> >  CycleCollectedJSRuntime::AddJSHolder(void* aHolder, nsScriptObjectTracer* aTracer)
> >  {
> >    MOZ_ASSERT(aTracer->Trace, "AddJSHolder needs a non-null Trace function");
> > -  bool wasEmpty = mJSHolders.Count() == 0;
> 
> Blindly removing the nsLayoutStatics stuff seems dangerous. Are you sure
> it's safe? We could move it behind a main-thread check for now if needed?
> Though we'll have to figure out how to make various code deal with the fact
> that they can't use nsLayoutStatics off the main-thread as we share the code
> between workers and main-thread DOM.

Did you see comment 5?  It's hardly blind removal ;-)

Comment 15

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 766726 [details] [diff] [review]
Part 1 - Create a worker implementation of CycleCollectedJSRuntime

Review of attachment 766726 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/RuntimeService.cpp
@@ +759,3 @@
>  {
> +public:
> +  WorkerJSRuntime(WorkerPrivate* aWorkerPrivate, JSContext** aOutContext)

Ick, let's not have an outparam from a constructor.

In fact, I don't see why this class is needed at all. You could avoid changing all this by simply making a stack CycleCollectedJSRuntime, constructing it, and then passing it to CreateJSContextForWorker.

If you just need a scoped cycle collector class then a) it should probably call nsCycleCollector_startup() in the constructor, and b) it could be tiny :)

@@ +839,5 @@
>    }
>  
> +  ~WorkerJSRuntime()
> +  {
> +    nsCycleCollector_forgetJSRuntime();

Is it weird that you don't pass a JSRuntime* here? I guess we're assuming no one ever creates more than one JSRuntime per thread?

@@ +847,5 @@
>  
> +protected:
> +  void TraverseAdditionalNativeRoots(nsCycleCollectionNoteRootCallback& aCb) MOZ_OVERRIDE
> +  { }
> +  void TraceAdditionalNativeGrayRoots(JSTracer* aTracer) MOZ_OVERRIDE

Nit: return types on their own lines

@@ +881,2 @@
>    NS_IMETHOD
>    Run()

Did anything significant change here? I can't tell if it's all whitespace or not.

::: xpcom/base/nsCycleCollector.cpp
@@ +2635,5 @@
>  #ifdef DEBUG
> +    nsIThread* currentThread = NS_GetCurrentThread();
> +    // XXXkhuey we can be called so late in shutdown that NS_GetCurrentThread
> +    // returns null (after the thread manager has shut down)
> +    MOZ_ASSERT(mThread == currentThread || !currentThread);

I think you need IsOnCurrentThread() here, not checking pointer equality. LazyIdleThread would break here, for instance.

Comment 16

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 766728 [details] [diff] [review]
Part 2 - Port ImageData to workers

Review of attachment 766728 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/WorkerPrivate.cpp
@@ +261,5 @@
> +      if (NS_SUCCEEDED(UnwrapObject<ImageData>(aCx, aObj, imageData))) {
> +        // Prepare the ImageData internals.
> +        uint32_t width = imageData->Width();
> +        uint32_t height = imageData->Height();
> +        JS::Rooted<JSObject*> dataArray(aCx, imageData->GetDataObject());

This probably doesn't need to be rooted (nor do you need the nsRefPtr that mccr8 was talking about) as long as we're guaranteed that aObj is rooted. It's a JS::Handle<> so I think we're fine.

Comment 17

[Boris Zbarsky [:bzbarsky]]

dataArray doesn't need to be a Rooted as long as it doesn't exist.  But if it's on the stack, it needs to be a Rooted, because the JSAutoCompartment can GC and that can move the object, making the pointer stale..

Comment 18

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 766726 [details] [diff] [review]
Part 1 - Create a worker implementation of CycleCollectedJSRuntime

It's confusing that mccr8 gave r+ and I just canceled...

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 1: Fix heap dumping to work off the main thread.

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 2: Refactor context callbacks

Tagging mccr8 for the CC bits.

Comment 21

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 782748 [details] [diff] [review]
Part 2: Refactor context callbacks

jorendorff for jsapi changes

Comment 22

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 782748 [details] [diff] [review]
Part 2: Refactor context callbacks

And bholley for the XPConnect/xpcshell changes.

Comment 23

[Andrew McCreight [:mccr8]]

Comment on attachment 782747 [details] [diff] [review]
Part 1: Fix heap dumping to work off the main thread.

Review of attachment 782747 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/CycleCollectedJSRuntime.cpp
@@ +1049,5 @@
>  
> +void
> +CycleCollectedJSRuntime::DumpJSHeap(FILE* file)
> +{
> +  js::DumpHeapComplete(Runtime(), file);

Do we have any kind of "main thread or worker thread" assertion bumping around?  That would be nice to have here, though not a big deal.

Comment 24

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 3: Give the cycle collector embedding more control over SnowWhite

Please review this one carefully ;-)

Comment 25

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 4: Create a worker implementation of CycleCollectedJSRuntime

Comment 26

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 782751 [details] [diff] [review]
Part 4: Create a worker implementation of CycleCollectedJSRuntime

I think this changed enough that Andrew should look at it again too.

Comment 27

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 5: Port ImageData to workers.

This didn't really change.

Comment 28

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 6: Add a switch to disable QI and use it on ImageData.

Comment 29

[Bobby Holley (:bholley)]

Comment on attachment 782748 [details] [diff] [review]
Part 2: Refactor context callbacks

Review of attachment 782748 [details] [diff] [review]:
-----------------------------------------------------------------

r=bholley. I don't think this needs jorendorff's review.

::: js/xpconnect/shell/xpcshell.cpp
@@ +1640,4 @@
>              return 1;
>          }
>  
> +        rtsvc->RegisterContextCallback(ContextCallback);

Should we unregister it somewhere just to be tidy?

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +3058,5 @@
> +XPCJSRuntime::RemoveContextCallback(xpcContextCallback cb)
> +{
> +    NS_ASSERTION(cb, "null callback");
> +    bool found = extraContextCallbacks.RemoveElement(cb);
> +    if (!found) {

MOZ_ASSERT this, please.

Comment 30

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 782753 [details] [diff] [review]
Part 6: Add a switch to disable QI and use it on ImageData.

r=me

Comment 31

[Andrew McCreight [:mccr8]]

Comment on attachment 782748 [details] [diff] [review]
Part 2: Refactor context callbacks

Review of attachment 782748 [details] [diff] [review]:
-----------------------------------------------------------------

I only looked at the two CycleCollected* files.

::: xpcom/base/CycleCollectedJSRuntime.cpp
@@ +556,4 @@
>  {
>    JSContext* iter = nullptr;
>    while (JSContext* acx = JS_ContextIterator(Runtime(), &iter)) {
> +    MOZ_ASSERT(js::HasUnrootedGlobal(acx));

Please add a comment or a message to the assert that this is needed to prevent leaks or whatever.  Or something explaining why you do this.

@@ +1209,4 @@
>    switch (aStatus) {
>      case JSGC_BEGIN:
>      {
> +      // XXXkhuey do we still need this?

Please file a followup bug about removing this, and I guess refer to it in the comment here.  I'd be surprised if we still need this, but anything is possible...

Comment 32

[Andrew McCreight [:mccr8]]

Comment on attachment 782748 [details] [diff] [review]
Part 2: Refactor context callbacks

bholley already r+'d this

Comment 33

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 782750 [details] [diff] [review]
Part 3: Give the cycle collector embedding more control over SnowWhite

I think I'd just do null checks in nsCycleCollector_doDeferredDeletion() so that
we don't get random null pointer crashes.

I know the coding style of CC hasn't been updated to follow
the normal rules, but I'd prefer Type* variable; not Type *variable;

Comment 34

[Andrew McCreight [:mccr8]]

Comment on attachment 782751 [details] [diff] [review]
Part 4: Create a worker implementation of CycleCollectedJSRuntime

Review of attachment 782751 [details] [diff] [review]:
-----------------------------------------------------------------

r- for the compartment merging thing

::: dom/workers/RuntimeService.cpp
@@ +841,5 @@
> +    // All JSContexts except mLastJSContext should be destroyed now.  The
> +    // worker global will be unrooted and the shutdown cycle collection
> +    // should break all remaining cycles.  Destroying mLastJSContext will run
> +    // the GC the final time and finalize any JSObjects that were participating
> +    // in cycles that were broken during CC shutdown.

This comment sounds wrong to me.  When we're in CC shutdown, if the CC frees anything, we should go around the loop again and trigger another GC.  I mean, I can believe there's something that ends up triggering later or whatever so you need another GC, but I don't think it would be anything from the CC.

@@ -865,5 @@
> -      JSAutoRequest ar(cx);
> -      workerPrivate->DoRunLoop(cx);
> -    }
> -
> -    // XXX Bug 666963 - CTypes can create another JSContext for use with

Maybe this explanation should be moved up to where you explain that the Ctypes context can't be the last one?  As it is, there's just this requirement without any explanation or bug number reference.

::: xpcom/base/CycleCollectedJSRuntime.cpp
@@ +989,4 @@
>  bool
>  CycleCollectedJSRuntime::UsefulToMergeZones() const
>  {
> +  if (!NS_IsMainThread()) {

This will break compartment merging for the main thread's CC thread.  Please fix that.

I guess compartment merging on worker threads is just always going to return false because there are no windows there?

You could make this method virtual, return false here, then move the body of this function to an override in XPConnect, maybe?

Comment 35

[Andrew McCreight [:mccr8]]

If you don't want to actually fix the merging problem, I guess it would be okay to instead add a fatal assertion to nsCycleCollector_startup or something when the CC thread is enabled that says that we don't do compartment merging, so we don't decide to turn it on again in the future and forget that it completely disables this particular optimization...

Comment 36

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Would you be happy with me removing the CC thread entirely in a followup bug?

Comment 37

[Bobby Holley (:bholley)]

Just FYI - bug 899245 just landed on inbound, which renames a few of the functions used here.

Comment 38

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 782750 [details] [diff] [review]
Part 3: Give the cycle collector embedding more control over SnowWhite

Review of attachment 782750 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +224,5 @@
>  
> +class AsyncFreeSnowWhite : public nsRunnable
> +{
> +public:
> +  NS_IMETHOD Run()

This code uses 4-space indentation, I'm afraid

::: xpcom/base/CycleCollectedJSRuntime.h
@@ +204,4 @@
>    void DeferredFinalize(nsISupports* aSupports);
>  
>    void DumpJSHeap(FILE* aFile);
> +  

Trailing ws

Comment 39

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 782753 [details] [diff] [review]
Part 6: Add a switch to disable QI and use it on ImageData.

Review of attachment 782753 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/Bindings.conf
@@ +29,5 @@
>  #                   true for workers, false otherwise).
>  #   * customFinalize - The native class will use a custom finalize hook
>  #                      (defaults to true for workers, false otherwise).
> +#   * wantsQI - Indicates whether the interface should have a QueryInterface
> +#               method available to chrome.

Defaults to...?

::: dom/bindings/Configuration.py
@@ +488,5 @@
>  
> +    def wantsQI(self):
> +        # If it was specified explicitly use that.
> +        if hasattr(self, '_wantsQI'):
> +            return self._wantsQI

Maybe a tri-state None/True/False? That would simplify the setting of _wantsQI too.

Comment 40

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 7: Only set up XRay jsids on the main thread.

Without this xrays are busted after setting up the binding off the main thread.  Surprisingly the JS engine doesn't assert here.

Comment 41

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 783924 [details] [diff] [review]
Part 7: Only set up XRay jsids on the main thread.

This is not the patch I meant to write.

Comment 42

[Andrew McCreight [:mccr8]]

Comment on attachment 782751 [details] [diff] [review]
Part 4: Create a worker implementation of CycleCollectedJSRuntime

r=me with some kind of assertion if we use the CC thread, talking about JS merging and whatever stuff I said before.

Comment 43

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 7: Only setup xray jsids on the main thread.

Comment 44

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 2: Refactor context callbacks.

Comment 45

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 785053 [details] [diff] [review]
Part 7: Only setup xray jsids on the main thread.

r=me

Comment 46

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 782751 [details] [diff] [review]
Part 4: Create a worker implementation of CycleCollectedJSRuntime

Review of attachment 782751 [details] [diff] [review]:
-----------------------------------------------------------------

I only looked at RuntimeService.cpp changes here so if you wanted me to look at other stuff please re-r? me.

::: dom/workers/RuntimeService.cpp
@@ +822,5 @@
> +{
> +public:
> +  WorkerJSRuntime(WorkerPrivate* aWorkerPrivate)
> +  // The number passed here doesn't matter, we will change it later in the call
> +  // to JS_SetGCParameter.

Nit: Put the comment above the constructor instead of somewhere in the middle, and then make the comment mention the heap size and that we change the value in CreateJSContextForWorker.

@@ +849,5 @@
> +    mLastJSContext = nullptr;
> +  }
> +
> +  // Make this public for now.  Ideally we'd hide the JSRuntime inside.
> +  JSRuntime* Runtime() const

Nit: here and below, return types on their own line.

@@ +886,5 @@
>  
>      workerPrivate->AssertIsOnWorkerThread();
>  
> +    {
> +      nsCycleCollector_startup(CCSingleThread);

This should go into the WorkerJSRuntime constructor like I mentioned previously.

Comment 47

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

> @@ +886,5 @@
> >  
> >      workerPrivate->AssertIsOnWorkerThread();
> >  
> > +    {
> > +      nsCycleCollector_startup(CCSingleThread);
> 
> This should go into the WorkerJSRuntime constructor like I mentioned
> previously.

I thought I mentioned this earlier, but apparently I didn't.  nsCycleCollector_startup must be called before nsCycleCollector_registerJSRuntime, which is called in CycleCollectedJSRuntime's ctor.  nsCycleCollector_startup can't be moved into the WorkerJSRuntime's ctor since that runs after the base class ctor.

Comment 48

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/efe4ad163c58
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7dbdc2a38f9
https://hg.mozilla.org/integration/mozilla-inbound/rev/d18e1e6db0dc
https://hg.mozilla.org/integration/mozilla-inbound/rev/688333343bdd
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3ae42b64ce9
https://hg.mozilla.org/integration/mozilla-inbound/rev/f88dd557de90
https://hg.mozilla.org/integration/mozilla-inbound/rev/780fd3893cc9

Comment 49

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/efe4ad163c58
https://hg.mozilla.org/mozilla-central/rev/b7dbdc2a38f9
https://hg.mozilla.org/mozilla-central/rev/d18e1e6db0dc
https://hg.mozilla.org/mozilla-central/rev/688333343bdd
https://hg.mozilla.org/mozilla-central/rev/c3ae42b64ce9
https://hg.mozilla.org/mozilla-central/rev/f88dd557de90
https://hg.mozilla.org/mozilla-central/rev/780fd3893cc9

Comment 50

[Jean-Yves Perrier [:teoli]]

Am I correct in understanding that this bug has the user-visible effect to allow the use of ImageData in workers?

Comment 51

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

No, ImageData was already supported in workers. The change here was to make ImageData-in-workers and ImageData-on-the-main-thread use the same implementation.

Comment 52

[Jean-Yves Perrier [:teoli]]

Thank you, so no dev doc needed :-)

Comment 53

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

What Ms2ger said ;-)