Add some assertions to validate that mobile viewport zoom factors are positive
Categories
(Core :: Layout: Scrolling and Overflow, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox70 | --- | fixed |
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
As noted in bug 1566824, it's apparently possible for MobileViewportManager to call into nsLayoutUtils::SetVisualViewportSize with a negative (or perhaps NaN) aSize arg, which trips a fatal assertion.
Per bug 1566824 comment 2, I only see this being possible if we're somehow using a zoom factor whose value is 0 or negative for the expression CSSSize compSize = compositionSize / aZoom;
I don't see a lot of clamping around this aZoom value. Let's add some assertions to help catch this problem a bit earlier on and shine light on places where clamping may be missing.
|
Assignee | |
Comment 1•5 years ago
|
||
|
||
Comment 2•5 years ago
|
||
|
|
Assignee | |
Comment 3•5 years ago
|
||
Try run to be sure these assertions don't insta-fail in our existing testsuite:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=24e87784821fb0afaa0bf2cf14a9d90f10d931f8
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/58eb45e75568 Add some assertions to sanity-check that our mobile viewport zoom factors are positive. r=botond
|
||
Comment 5•5 years ago
|
||
Backed out changeset 58eb45e75568 (bug 1566991) for build bustages at MobileViewportManager.cpp:400:24.
Backout: https://hg.mozilla.org/integration/autoland/rev/7906ac21f5601f00e0695b4db018cd0d1edbd42c
Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=58eb45e755687894a505d86ff8e907a8a4c913d3&selectedJob=257906252
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=257906252&repo=autoland&lineNumber=23271
|
|
Assignee | |
Comment 6•5 years ago
|
||
Oops, sorry -- I noticed that build failure locally & forgot to post my tweak before landing.
|
|
Assignee | |
Comment 7•5 years ago
|
||
The build-fixing tweak was included in my Try run in comment 3 (even though I forgot to update phabricator), so I'm pretty confident that we're good with that.
Retriggered autoland.
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ac8dd9828c25 Add some assertions to sanity-check that our mobile viewport zoom factors are positive. r=botond
|
|
||
Comment 9•5 years ago
|
||
| bugherder | ||
Description
•