Port more components to WorkerRef

RESOLVED FIXED in Firefox 62

Status

()

P2
normal
RESOLVED FIXED
8 months ago
6 months ago

People

(Reporter: baku, Assigned: baku)

Tracking

58 Branch
mozilla62
Points:
---

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(6 attachments, 3 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

8 months ago
Created attachment 8969208 [details] [diff] [review]
part 1- IPCStream
Attachment #8969208 - Flags: review?(bugmail)
(Assignee)

Comment 2

8 months ago
Created attachment 8969210 [details] [diff] [review]
part 1 - WorkerProxyToMainThreadRunnable
Attachment #8969210 - Flags: review?(bugmail)
(Assignee)

Comment 3

8 months ago
Created attachment 8969211 [details] [diff] [review]
part 2 - IPCStream
Attachment #8969208 - Attachment is obsolete: true
Attachment #8969208 - Flags: review?(bugmail)
Attachment #8969211 - Flags: review?(bugmail)
(Assignee)

Comment 4

8 months ago
Created attachment 8969212 [details] [diff] [review]
part 3 - WebCrypto
Attachment #8969212 - Flags: review?(bugmail)
(Assignee)

Comment 5

8 months ago
Created attachment 8969213 [details] [diff] [review]
part 4 - IDB transaction
Attachment #8969213 - Flags: review?(jvarga)
(Assignee)

Comment 6

8 months ago
Created attachment 8969214 [details] [diff] [review]
part 5 - XHR
Attachment #8969214 - Flags: review?(bugmail)
(Assignee)

Updated

8 months ago
Duplicate of this bug: 1445883
(Assignee)

Comment 8

8 months ago
Created attachment 8969219 [details] [diff] [review]
part 1 - WorkerProxyToMainThreadRunnable
Attachment #8969210 - Attachment is obsolete: true
Attachment #8969210 - Flags: review?(bugmail)
Attachment #8969219 - Flags: review?(bugmail)

Updated

8 months ago
Priority: -- → P2
(Assignee)

Comment 9

7 months ago
Created attachment 8972792 [details] [diff] [review]
part 3 - WebCrypto
Attachment #8969212 - Attachment is obsolete: true
Attachment #8969212 - Flags: review?(bugmail)
Attachment #8972792 - Flags: review?(bugmail)
(Assignee)

Comment 10

7 months ago
Created attachment 8972793 [details] [diff] [review]
part 6 - Image canvas
Attachment #8972793 - Flags: review?(bugmail)
(Assignee)

Updated

6 months ago
Attachment #8969211 - Flags: review?(bugmail) → review?(mrbkap)
(Assignee)

Updated

6 months ago
Attachment #8969214 - Flags: review?(bugmail) → review?(mrbkap)
(Assignee)

Updated

6 months ago
Attachment #8969219 - Flags: review?(bugmail) → review?(mrbkap)
(Assignee)

Updated

6 months ago
Attachment #8972792 - Flags: review?(bugmail) → review?(mrbkap)
(Assignee)

Updated

6 months ago
Attachment #8972793 - Flags: review?(bugmail) → review?(mrbkap)
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.
Attachment #8969219 - Flags: review?(mrbkap) → review+
Attachment #8969211 - Flags: review?(mrbkap) → review+
Attachment #8972792 - Flags: review?(mrbkap) → review+
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!
Attachment #8972793 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 13

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

Can you review this patch too? :)
Attachment #8969213 - Flags: review?(jvarga) → review?(bugmail)
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
Attachment #8969214 - Flags: review?(mrbkap) → review+
Attachment #8969213 - Flags: review?(bugmail) → review+

Comment 15

6 months ago
Pushed by amarchesini@mozilla.com:
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
You need to log in before you can comment on or make changes to this bug.