Closed
Bug 862825
Opened 8 years ago
Closed 8 years ago
crashtest-ipc fails if a test messes up its element prototype chains
Categories
(Testing :: Reftest, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla24
People
(Reporter: bzbarsky, Assigned: bzbarsky)
Details
Attachments
(1 file)
1.70 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
STEPS TO REPRODUCE: 1) Create a crashtest that does this: HTMLElement.prototype.__proto__ = Proxy.create({}, {}); 2) Try to push it to try. ACTUAL RESULTS: Passes all tests except crashtest-ipc EXPECTED RESULTS: Passes crashtest-ipc What happens is that crashtest-ipc is doing this, in SynchronizeForSnapshot: var dummyCanvas = content.document.createElementNS(XHTML_NS, "canvas"); and this throws if the test messed up its DOM. So obvious questions: 1) Should we be bailing early from SynchronizeForSnapshot if the test is TYPE_LOAD or TYPE_SCRIPT? Seems silly to do canvas stuff in those cases, right? 2) Should we be catching an exception from the createElement here and bailing out or something if that happens?
Flags: needinfo?(roc)
Flags: needinfo?(dbaron)
1) I guess so. 2) I don't know why we're using the test's document here. Seems to me we should use the outer window's document and avoid this problem that way.
Flags: needinfo?(roc)
![]() |
Assignee | |
Comment 2•8 years ago
|
||
Chris, do you remember anything about this stuff?
Flags: needinfo?(cjones.bugs)
Pinging Chris is probably not a good idea. I say just flip it to create the canvas using the outer window's document. I think that should work fine.
Flags: needinfo?(cjones.bugs)
Comment 4•8 years ago
|
||
No opinion here, really. roc's answers sound fine. (I feel like at some point we need to have better automated tests for the reftest harness.)
Flags: needinfo?(dbaron)
I agree with roc re: (2). The current impl is just buggy.
![]() |
Assignee | |
Comment 6•8 years ago
|
||
Hmm. So the outer window's document is in the parent process, while the reftest-content.js is running in the child process. :( I guess I'll post the patch to do #1 and call that good enough for now.
![]() |
Assignee | |
Comment 7•8 years ago
|
||
Attachment #747783 -
Flags: review?(roc)
![]() |
Assignee | |
Updated•8 years ago
|
Assignee: nobody → bzbarsky
![]() |
Assignee | |
Updated•8 years ago
|
Whiteboard: [need review]
Attachment #747783 -
Flags: review?(roc) → review+
![]() |
Assignee | |
Comment 8•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc0bef5a2cfd
Flags: in-testsuite?
Whiteboard: [need review]
Target Milestone: --- → mozilla24
Comment 9•8 years ago
|
||
Backed out for mochitest-bc, mochitest-other, and xpcshell orange. https://hg.mozilla.org/integration/mozilla-inbound/rev/9182c3e6a967
![]() |
Assignee | |
Comment 10•8 years ago
|
||
The problem was from bug 869195. Relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/1d3b60ab634a
Comment 11•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1d3b60ab634a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•