1455256 - Port more components to WorkerRef

Status: RESOLVED FIXED

Description

[Andrea Marchesini [:baku]]

Comment 1

[Andrea Marchesini [:baku]]

Comment 2

[Andrea Marchesini [:baku]]

Comment 3

[Andrea Marchesini [:baku]]

Comment 4

[Andrea Marchesini [:baku]]

Comment 5

[Andrea Marchesini [:baku]]

Comment 6

[Andrea Marchesini [:baku]]

Comment 8

[Andrea Marchesini [:baku]]

Comment 9

[Andrea Marchesini [:baku]]

Comment 10

[Andrea Marchesini [:baku]]

Comment 11

[Andrew Sutherland [:asuth]]

Comment on attachment 8969219 [details] [diff] [review]
part 1 - WorkerProxyToMainThreadRunnable

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

Restating: Eliminates SimpleWorkerHolder, switching to a ThreadSafeWorkerRef to keep the worker alive during dispatch to the main thread.  mWorkerPrivate eliminated in favor of passing aWorkerPrivate into methods where possible.

Comment 12

[Andrew Sutherland [:asuth]]

Comment on attachment 8972793 [details] [diff] [review]
part 6 - Image canvas

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

Loving all these nice cleanups enabled by the WorkerRef family!

Comment 13

[Andrea Marchesini [:baku]]

Comment on attachment 8969213 [details] [diff] [review]
part 4 - IDB transaction

Can you review this patch too? :)

Comment 14

[Andrew Sutherland [:asuth]]

Comment on attachment 8969214 [details] [diff] [review]
part 5 - XHR

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

Restating:
- mRooting previously tracked whether (a) the WorkerHolder was actively holding the worker and (b) a manual refcount bump was in effect.  mWorkerRef, a strong worker-ref with cleanup callback that does what the WorkerHolder::Notify used to do.
- (Since bug 1449138) there is no change in when the underlying WorkerHolder is released.  The WorkerHolder (and StrongWorkerRef, post-patch) gets removed in a WorkerControlRunnable dispatched back to the main thread from the aptly named SyncTeardownRunnable.  The WorkerControlRunnable should be in the control queue prior to the SyncTeardownRunnable returning.

::: dom/xhr/XMLHttpRequestWorker.cpp
@@ +1667,5 @@
> +      if (!self->mCanceled) {
> +        self->mCanceled = true;
> +        self->ReleaseProxy(WorkerIsGoingAway);
> +      }
> +    });            

lint nit: trailing whitespace

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/163ef7073f35
Port more components to WorkerRef - part 1 - WorkerProxyToMainThreadRunnable, r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0e4a463e0cf
Port more components to WorkerRef - part 2 - IPCStream, r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/922e5af62299
Port more components to WorkerRef - part 3 - WebCrypto, r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4dc5ef254a7
Port more components to WorkerRef - part 4 - IDB Transaction, r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e673a720f1a
Port more components to WorkerRef - part 5 - XHR, r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/247a38811560
Port more components to WorkerRef - part 6 - ImageCanvas, r=asuth

Comment 16

[Raul Gurzau (:RaulGurzau)]

https://hg.mozilla.org/mozilla-central/rev/163ef7073f35
https://hg.mozilla.org/mozilla-central/rev/f0e4a463e0cf
https://hg.mozilla.org/mozilla-central/rev/922e5af62299
https://hg.mozilla.org/mozilla-central/rev/e4dc5ef254a7
https://hg.mozilla.org/mozilla-central/rev/1e673a720f1a
https://hg.mozilla.org/mozilla-central/rev/247a38811560