Closed
Bug 1455256
Opened 7 years ago
Closed 7 years ago
Port more components to WorkerRef
Categories
(Core :: DOM: Workers, enhancement, P2)
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: baku, Assigned: baku)
References
Details
Attachments
(6 files, 3 obsolete files)
4.87 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
6.38 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
7.14 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
15.39 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
6.12 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
6.84 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8969208 -
Flags: review?(bugmail)
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8969210 -
Flags: review?(bugmail)
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8969208 -
Attachment is obsolete: true
Attachment #8969208 -
Flags: review?(bugmail)
Attachment #8969211 -
Flags: review?(bugmail)
Assignee | ||
Comment 4•7 years ago
|
||
Attachment #8969212 -
Flags: review?(bugmail)
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8969213 -
Flags: review?(jvarga)
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8969214 -
Flags: review?(bugmail)
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8969210 -
Attachment is obsolete: true
Attachment #8969210 -
Flags: review?(bugmail)
Attachment #8969219 -
Flags: review?(bugmail)
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Comment 9•7 years ago
|
||
Attachment #8969212 -
Attachment is obsolete: true
Attachment #8969212 -
Flags: review?(bugmail)
Attachment #8972792 -
Flags: review?(bugmail)
Assignee | ||
Comment 10•7 years ago
|
||
Attachment #8972793 -
Flags: review?(bugmail)
Assignee | ||
Updated•7 years ago
|
Attachment #8969211 -
Flags: review?(bugmail) → review?(mrbkap)
Assignee | ||
Updated•7 years ago
|
Attachment #8969214 -
Flags: review?(bugmail) → review?(mrbkap)
Assignee | ||
Updated•7 years ago
|
Attachment #8969219 -
Flags: review?(bugmail) → review?(mrbkap)
Assignee | ||
Updated•7 years ago
|
Attachment #8972792 -
Flags: review?(bugmail) → review?(mrbkap)
Assignee | ||
Updated•7 years ago
|
Attachment #8972793 -
Flags: review?(bugmail) → review?(mrbkap)
Comment 11•7 years ago
|
||
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+
Updated•7 years ago
|
Attachment #8969211 -
Flags: review?(mrbkap) → review+
Updated•7 years ago
|
Attachment #8972792 -
Flags: review?(mrbkap) → review+
Comment 12•7 years ago
|
||
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•7 years 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 14•7 years ago
|
||
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+
Updated•7 years ago
|
Attachment #8969213 -
Flags: review?(bugmail) → review+
Comment 15•7 years 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
Comment 16•7 years ago
|
||
bugherder |
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
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•