Closed
Bug 1336447
Opened 7 years ago
Closed 4 years ago
IPDL failures may no longer cause reftest failures
Categories
(Testing :: Reftest, defect)
Testing
Reftest
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: kats, Unassigned)
References
Details
Attachments
(1 file)
3.29 KB,
text/plain
|
Details |
At some point it looks IPDL failures were changed to not trigger crashes. This means that reftests that run into IPDL failures may not fail. In particular, a couple of reftests added in bug 1298218 relied on APZ, which isn't present in non-e10s. So the tests should have failed in non-e10s, but they didn't. See the attached log snippet for the reftest output from an example run. I'm pretty sure the IPDL not-crashing change was intentional, but I don't know if this was foreseen as a side-effect. :dvander, do you have any thoughts on what the intended behaviour here is?
This definitely wasn't intended. I think it's a regression either from bug 792652 or bug 1314254.
Flags: needinfo?(wmccloskey)
Actually, I guess I'm wrong. Unless the top-level protocol implements ProcessingError, it seems that we don't do anything when a message handler returns false. Do you think we should change this David? The difficulty in such cases is with properly generating crash reports if we do want to kill the other process.
Flags: needinfo?(wmccloskey)
I can't convince myself that it's worth doing, the crash report will be very misleading. The tests should probably be structured in a way that fails when APZ is not functionally present. But maybe we could have a testing pref that makes ProcessingError always crash or something.
Flags: needinfo?(dvander)
Reporter | ||
Comment 5•7 years ago
|
||
In this case I can probably check the return value of [1] and crash explicitly if it's false. But if you're going to make it crash during tests or during debug builds or something then I can wait. [1] http://searchfox.org/mozilla-central/rev/ca7015fa45b30b29176fbaa70ba0a36fe9263c38/dom/base/nsDOMWindowUtils.cpp#2564
Oh, I wasn't planning on doing it :) It'd probably take a while to flag all the new crashes that are introduced.
Reporter | ||
Comment 7•4 years ago
|
||
Not worth keeping this bug open
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•