Closed
Bug 1481184
Opened Last year
Closed Last year
Remove remaining JSAutoRealmAllowCCW uses in toolkit/recordreplay
Categories
(Core :: Web Replay, enhancement)
Core
Web Replay
Not set
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(1 file)
2.67 KB,
patch
|
bhackett
:
review+
|
Details | Diff | Splinter Review |
There are two of them left: * https://searchfox.org/mozilla-central/rev/3fdc491e118c5cdfbaf6e2d52f3466d2b27ad1de/toolkit/recordreplay/ipc/JSControl.cpp#204 * https://searchfox.org/mozilla-central/rev/3fdc491e118c5cdfbaf6e2d52f3466d2b27ad1de/toolkit/recordreplay/ipc/JSControl.cpp#344 It looks like both of these could be arbitrary objects (and therefore CCWs) passed in from JS. Our options are: (1) Change them to JSAutoRealm, this will diagnostic-assert the object is a CCW. (2) Use CheckedUnwrap or UncheckedUnwrap and wrap the return value in the cx compartment. (3) Check for CCWs and throw an exception in the JSNatives receiving those objects. (4) Store a global object in addition to these objects, and use that with JSAutoRealm (a bit more complicated).
Flags: needinfo?(bhackett1024)
Assignee | ||
Comment 1•Last year
|
||
(In reply to Jan de Mooij [:jandem] from comment #0) > (1) Change them to JSAutoRealm, this will diagnostic-assert the object is a > CCW. is *not*, of course.
Comment 2•Last year
|
||
Hmm, I think these objects will either never be CCWs, or will always be CCWs. These APIs are used exclusively by devtools/server/actors/replay/debugger.js, and the debugger/handlers used there will always be in the same compartment as the ReplayDebugger object itself which is calling into RecordReplayControl. However, there is some oddness where RecordReplayControl as accessed here came from a sandbox different from the realm where the ReplayDebugger object lives, see devtools/shared/builtin-modules.js. I don't know if that realm will be in a different compartment from the ReplayDebugger (or, maybe more importantly, whether it is guaranteed to always be in the same compartment as the ReplayDebugger; FWIW this is all chrome code that is running). Doing 2) when the objects are initially passed in and then 1) seems like the safest way forward here.
Flags: needinfo?(bhackett1024)
Assignee | ||
Comment 3•Last year
|
||
Brian, do we build/test this code on OS X in automation? In that case I'll do a Mac Try push.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8998202 -
Flags: review?(bhackett1024)
Comment 4•Last year
|
||
Comment on attachment 8998202 [details] [diff] [review] Use JSAutoRealm instead of JSAutoRealmAllowCCW in toolkit/recordreplay Review of attachment 8998202 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! Right now there is one devtools mochitest which runs this code, browser_dbg_rr_breakpoints-01.js (more tests coming soon!)
Attachment #8998202 -
Flags: review?(bhackett1024) → review+
Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/28d1ff679d50 Use JSAutoRealm instead of JSAutoRealmAllowCCW in toolkit/recordreplay. r=bhackett
Comment 6•Last year
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/28d1ff679d50
Status: ASSIGNED → RESOLVED
Closed: Last year
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•