1569475 - Frequent Assertion failure: zoom > CSSToScreenScale(0.0f) (zoom factor must be positive), at /builds/worker/workspace/build/src/layout/base/MobileViewportManager.cpp when when Gecko 70 merges to Beta on 2019-08-26

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Noemi Erli[:noemi_erli]]

[Tracking Requested - why for this release]:

Central as Beta: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=b8c20f279980c618671cfb1da5b253fbb739fa66&searchStr=devtools&selectedJob=258694897

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=258694897&repo=try&lineNumber=23118

[task 2019-07-28T12:17:43.218Z] 12:17:43 INFO - GECKO(1711) | Assertion failure: zoom > CSSToScreenScale(0.0f) (zoom factor must be positive), at /builds/worker/workspace/build/src/layout/base/MobileViewportManager.cpp:191
[task 2019-07-28T12:18:03.382Z] 12:18:03 INFO - GECKO(1711) | #01: MobileViewportManager::UpdateResolution(nsViewportInfo const&, mozilla::gfx::IntSizeTyped<mozilla::ScreenPixel> const&, mozilla::gfx::SizeTyped<mozilla::CSSPixel, float> const&, mozilla::Maybe<float> const&, MobileViewportManager::UpdateType) [gfx/2d/ScaleFactor.h:34]
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO -
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO - GECKO(1711) | #02: MobileViewportManager::RefreshViewportSize(bool) [layout/base/MobileViewportManager.cpp:558]
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO -
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO - GECKO(1711) | #03: mozilla::PresShell::UpdateViewportOverridden(bool) [layout/base/PresShell.cpp:10595]
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO -
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO - GECKO(1711) | #04: non-virtual thunk to nsDocShell::SetMetaViewportOverride(nsIDocShell::MetaViewportOverride) [docshell/base/nsDocShell.cpp:0]
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO -
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 23 (0x1077e2c00) [pid = 1714] [serial = 200] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 22 (0x10774f000) [pid = 1714] [serial = 197] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 21 (0x102896800) [pid = 1714] [serial = 185] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 20 (0x107748400) [pid = 1714] [serial = 186] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 19 (0x102919400) [pid = 1714] [serial = 188] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.383Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 18 (0x102923400) [pid = 1714] [serial = 190] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.384Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 17 (0x107754c00) [pid = 1714] [serial = 191] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.384Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 16 (0x107752400) [pid = 1714] [serial = 183] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.384Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 15 (0x1077df400) [pid = 1714] [serial = 194] [outer = 0x0] [url = http://example.com/browser/devtools/client/responsive.html/test/browser/touch.html]
[task 2019-07-28T12:18:03.384Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 14 (0x10774cc00) [pid = 1714] [serial = 193] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.384Z] 12:18:03 INFO - GECKO(1711) | --DOCSHELL 0x107fd5000 == 2 [pid = 1714] [id = {e75328ed-c762-6941-a35e-34efda38be6d}] [url = about:blank]
[task 2019-07-28T12:18:03.384Z] 12:18:03 INFO - GECKO(1711) | --DOCSHELL 0x107fea800 == 1 [pid = 1714] [id = {0fc5f4da-6ab1-0147-a63f-ae48620fe585}] [url = data:text/html;charset=utf-8,]
[task 2019-07-28T12:18:03.384Z] 12:18:03 INFO - GECKO(1711) | --DOCSHELL 0x107fdd000 == 0 [pid = 1714] [id = {5f03e09d-e8a2-574f-8167-17c08c927f7a}] [url = about:blank]
[task 2019-07-28T12:18:03.384Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 13 (0x11a045b60) [pid = 1714] [serial = 192] [outer = 0x0] [url = http://example.com/browser/devtools/client/responsive.html/test/browser/touch.html]
[task 2019-07-28T12:18:03.385Z] 12:18:03 INFO - GECKO(1711) | --DOMWINDOW == 12 (0x11a045d40) [pid = 1714] [serial = 196] [outer = 0x0] [url = about:blank]
[task 2019-07-28T12:18:03.385Z] 12:18:03 INFO - GECKO(1711) | [Parent 1711, Main Thread] WARNING: IPC message discarded: actor cannot send: file /builds/worker/workspace/build/src/ipc/glue/ProtocolUtils.cpp, line 568
[task 2019-07-28T12:18:03.385Z] 12:18:03 INFO - GECKO(1711) | [Parent 1711, Main Thread] WARNING: IPC message discarded: actor cannot send: file /builds/worker/workspace/build/src/ipc/glue/ProtocolUtils.cpp, line 568
[task 2019-07-28T12:18:03.385Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.385Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.385Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.386Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.386Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.386Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.386Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.386Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.386Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.386Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.386Z] 12:18:03 INFO - GECKO(1711) | JavaScript error: resource://gre/modules/BrowserElementParent.jsm, line 214: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.isDeadWrapper]
[task 2019-07-28T12:18:03.387Z] 12:18:03 INFO - TEST-INFO | started process screencapture
[task 2019-07-28T12:18:03.521Z] 12:18:03 INFO - TEST-INFO | screencapture: exit 0

This seems to be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1568673
Daniel, could you please take a look at this?

Comment 1

[Intermittent Failures Robot]

1 failures in 4608 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-inbound: 1

Platform breakdown:

• macosx1014-64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1569475&startday=2019-07-22&endday=2019-07-28&tree=all

Comment 2

[Daniel Holbert [:dholbert]]

Thanks for the heads up, Noemi.

Some contextual lines from the log:

TEST-PASS | devtools/client/responsive.html/test/browser/browser_viewport_resizing_fixed_width.js | Meta Viewport ON return to initial size should have expected layout height. - 
INFO - Closing responsive design mode
INFO - GECKO(1711) | [Parent 1711, Main Thread] WARNING: '!CanHandleWith(sPresContext)', file /builds/worker/workspace/build/src/dom/events/IMEStateManager.cpp, line 1152
INFO - GECKO(1711) | Assertion failure: zoom > CSSToScreenScale(0.0f) (zoom factor must be positive), at /builds/worker/workspace/build/src/layout/base/MobileViewportManager.cpp:191

This test, browser_viewport_resizing_fixed_width.js, is the same test that was involved in intermittent bug 1566824 (now duped to bug 1555457), so these assertions have gotten us one step closer to getting to the bottom of solving that issue. (hooray!)

The assertion failure here is the one at the bottom of CSSToScreenScale MobileViewportManager::ClampZoom, the same assertion that was firing (after being added) in bug 1568673 comment 7. The assertion means that the caller passed in the val NaN to be clamped, and we're failing to clamp it because every comparisons with NaN fails.

In this case, the caller is MobileViewportManager::UpdateResolution, but it's got several callsites into ClampZoom and I can't tell which one is involved because the backtrace is reporting the wrong filename/line-number for that function (it says it's gfx/2d/ScaleFactor.h:34 which is probably a copy-constructor involved in the call, rather than the actual callsite).

I'm tempted to just add a mozilla::IsNaN() special-case inside of ClampZoom, which would make us pretend that 1.0 was passed in instead, and spam a non-fatal NS_ERROR to give us a hint that the caller might want to be cleaned up.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Bug 1569475: Adjust MobileViewportManager::ClampZoom to check for NaN zoom values and use 1.0 instead. r?botond

Also spam a NS_ERROR whenever this happens (in debug builds), so that we
know to fix the caller to avoid division by zero.

Comment 4

[Daniel Holbert [:dholbert]]

Try run with the same central-as-beta patch from comment 0 (mac devtools tests only, the ones that were affected here):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bbb5b8900c49142bb603add82f96381096d647bf

Comment 5

[Daniel Holbert [:dholbert]]

noemi, how do I get a Try run like the one that you reported this bug on?

Your patch (on Try) says:

staging release: 70.0b12
Pushed via mach try release

...but ./mach try release doesn't seem to be a recognized command for me.

(I imported your patch and ran ./mach try fuzzy to generate the Try run in comment 4, but that didn't give me dt-7 testruns for some reason, even though I requested "devtools" runs.
Then I tried directly pushing your patch as-is to Try, with hg push -f try, and that gave me a comprehensive run in https://treeherder.mozilla.org/#/jobs?repo=try&revision=9479815515676bc8873d78bb58dce5c7383f3884 (I was hoping it'd be auto-filtered by try_task_config.json in the patch but apparently not). But I'm not confident that this Try run was testing the same configuration that was perma-failing for you. If I was testing that same configuration, I would expect to see my patch's new non-fatal NS_ERROR being a perma-fail, instead of the fatal MOZ_ASSERT perma-fail, during the test browser_viewport_resizing_fixed_width.js (in the mac debug dt-7 job). And I do not see either assertion failing.

Comment 6

[Daniel Holbert [:dholbert]]

Ah, I may've skimmed the ./mach try release error message too quickly -- perhaps I just need to specify the version number, -v 70.0b12.

Comment 7

[Daniel Holbert [:dholbert]]

Yeah, that -v flag is was what I was missing.

Here's a Try run based on the same commit as noemi's in comment 0, with my patch here added (and one other unrelated annotation-tweaking patch that was in my queue):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b1afa00e8a7e5701fb57349c555419465b75061

Comment 8

[Daniel Holbert [:dholbert]]

Also, it looks like this wasn't quite a "perma" assertion-failure in comment 0's try run after all [--> adjusting summary]

Comment 0's try run had seven OS X 10.14 debug "dt7" runs, and only five out of those seven hit this assertion-failure. (So: the majority, but not all of them.)

So, it's not surprising that some testruns will get lucky & avoid triggering my patch's new NS_ERROR() for this NaN-passed-in situation.

Comment 9

[Daniel Holbert [:dholbert]]

Cool, here are two test runs from comment 7 that are green (woohoo) and include a report of the NS_ERROR from this patch (showing that we are indeed intervening & preventing a fatal assertion-failure):
https://treeherder.mozilla.org/logviewer.html#?job_id=259085993&repo=try
https://treeherder.mozilla.org/logviewer.html#?job_id=259086002&repo=try

You can see the NS_ERROR failure if you load those^ logs, click the "open raw log" button at the top, and then search for "ClampZoom". That shows something like this, which is the new expected non-fatal failure mode after the attached patch is applied:

INFO - TEST-PASS | devtools/client/responsive.html/test/browser/browser_viewport_resizing_fixed_width_and_zoom.js | Meta Viewport ON return to initial size should have expected layout height. - 
INFO - Closing responsive design mode
INFO - GECKO(1696) | [Parent 1696, Main Thread] WARNING: '!CanHandleWith(sPresContext)', file /builds/worker/workspace/build/src/dom/events/IMEStateManager.cpp, line 1152
INFO - GECKO(1696) | [Child 1699, Main Thread] ###!!! ASSERTION: Don't pass NaN to ClampZoom; check caller for 0/0 division: 'Error', file /builds/worker/workspace/build/src/layout/base/MobileViewportManager.cpp, line 176
INFO - GECKO(1696) | #01: MobileViewportManager::UpdateResolution(nsViewportInfo const&, mozilla::gfx::IntSizeTyped<mozilla::ScreenPixel> const&, mozilla::gfx::SizeTyped<mozilla::CSSPixel, float> const&, mozilla::Maybe<float> const&, MobileViewportManager::UpdateType) [gfx/2d/ScaleFactor.h:34]
INFO - 
INFO - GECKO(1696) | #02: MobileViewportManager::RefreshViewportSize(bool) [layout/base/MobileViewportManager.cpp:563]
INFO - 
INFO - GECKO(1696) | #03: mozilla::PresShell::UpdateViewportOverridden(bool) [layout/base/PresShell.cpp:10595]
INFO - 
INFO - GECKO(1696) | #04: non-virtual thunk to nsDocShell::SetMetaViewportOverride(nsIDocShell::MetaViewportOverride) [docshell/base/nsDocShell.cpp:0]

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b336bded7d42
Adjust MobileViewportManager::ClampZoom to check for NaN zoom values and use 1.0 instead. r=botond

Comment 11

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/b336bded7d42

Comment 12

[Andreea Pavel [:apavel]]

No longer occurs in beta sims.

Comment 13

[Intermittent Failures Robot]

2 failures in 4439 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1
• mozilla-inbound: 1

Platform breakdown:

• macosx1014-64: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1569475&startday=2019-07-29&endday=2019-08-04&tree=all