Bug 1569475 Comment 2 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

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.

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.
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.

Back to Bug 1569475 Comment 2