Closed
Bug 943516
Opened 11 years ago
Closed 10 years ago
Need to call SetPreserveWrapperCallback on workers
Categories
(Core :: DOM: Workers, defect)
Core
DOM: Workers
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(1 file, 1 obsolete file)
3.39 KB,
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
Currently this is being done only on the main thread, which means on workers we'll end up discarding live weakmap keys under some circumstances. A slightly different callback will be needed as I think the existing one refers to XPC-only concepts.
Assignee | ||
Comment 2•10 years ago
|
||
Jesse has a test case in bug 989668.
Assignee | ||
Comment 4•10 years ago
|
||
People are apparently actually hitting this, so I should fix it.
Assignee: nobody → continuation
Assignee | ||
Comment 5•10 years ago
|
||
This fixes the crash in Jesse's test case. PreserveWrapper can't be shared between worker and main thread because on the main thread we have to reject XPConnect reflectors. I should probably port over the mainthread test of this stuff, so we have some checking of correctness and not just that it doesn't crash...
Assignee | ||
Comment 6•10 years ago
|
||
This is just a basic test, but the code is basically the same as main thread so it should be ok. Basically, we need to preserve wrappers on DOM bindings stuff used as weak map keys, because otherwise wrapper optimization can throw them out, which just makes them disappear. The main thread version of this code has to detect and ignore XPConnect wrappers, but we shouldn't have to worry about that on workers. If you are curious, the test that guards this call is in TryPreserveReflector(), in js/src/jsweakmap.cpp try run: https://tbpl.mozilla.org/?tree=Try&rev=829d7aa36496
Attachment #8500672 -
Flags: review?(peterv)
Updated•10 years ago
|
Attachment #8500672 -
Flags: review?(peterv) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #8498553 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5576c6cb1302
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•