+++ This bug was initially created as a clone of Bug #1710486 +++

Similar to bug 1710486, there's another test failure (after applying that fix) which looks like a hang with my patch for bug 1708735 (which only tweaks frontend stuff and causes most panels to be considered non-opaque):

https://treeherder.mozilla.org/jobs?repo=try&revision=bb7b4afe31ae79d2b3ba8712af8fd4d88d2223e5&selectedTaskRun=MrC0jFZBQBiYHeNyzRA-8g.0

However, it isn't a real hang like bug 1710486. After looking a bit into it, we just get stuck doing infinite repaint messages (unless you move the window manually or something).

There are two tests which show this issue and at first I thought were intermittents, but they reproduce locally 100% of the time:

• toolkit/content/tests/chrome/test_panel.xhtml
• layout/xul/test/test_windowminmaxsize.xhtml

I worked around the issue in the second test in the try run above by using an opaque background in the problematic panel: https://hg.mozilla.org/try/diff/82457fcdb0ee421f469ff93714dce346c99da9bc/layout/xul/test/titledpanelwindow.xhtml

But of course that's lame and we should probably figure out the root cause. This looks a bit like bug 1709998.

Sotaro, Chris, I plan to look into this tomorrow, but any chance you have any ideas of how to debug this?

(And to be clear, I confirmed that the patch in bug 1709998 doesn't help, as expected, since these are 1proc mochitests and don't even use e10s).

It might be related to Bug 1709864. I am going to look into the bug today.

nsWindow::OnPaint() flooding could happen like Bug 1709864 comment 5.

I could reproduce the problem locally with toolkit/content/tests/chrome/test_panel.xhtml.

When the problem happened, size was changed from (150, 50) to (150, 73) and re-trigger invalidation

• https://searchfox.org/mozilla-central/rev/aec7c53cdbbff65305d41c9d805a70efc0e902ed/widget/windows/nsWindow.cpp#2106

Size min was set by nsContainerFrame::SetSizeConstraints().

Hmm, current D114501 at Bug 1709864 did not address the problem.

When the D114501 applied, repaint was triggered from nsView::DoResetWidgetBounds() since curBounds(250, 262, 150, 73) and newBounds(250, 262, 150, 50) happened.

It seems that current widget size constraint needs to be applied to newBounds. But nsBaseWidget::ResizeClient() looks a bit tricky.

I am not sure why desktopDelta exists in nsBaseWidget::ResizeClient().

It is zero in normal situation on Windows. I saw non-zero value with STR of Bug 1701451. In this case, window size was updated unexpectedly by ::UpdateLayeredWindow(). And nsView::DoResetWidgetBounds() does not care the desktopDelta for comparing curBounds.TopLeft() and newBounds for triggering nsBaseWidget::ResizeClient().

(In reply to Sotaro Ikeda [:sotaro] from comment #7)

I am not sure why desktopDelta exists in nsBaseWidget::ResizeClient().

:jfkthame, do you know why desktopDelta is necessary?

D114814 addressed Infinite repaint loop with test_panel.xhtml for me.

Thanks for looking into this Sotaro!

(In reply to Sotaro Ikeda [:sotaro] from comment #8)

(In reply to Sotaro Ikeda [:sotaro] from comment #7)

I am not sure why desktopDelta exists in nsBaseWidget::ResizeClient().

:jfkthame, do you know why desktopDelta is necessary?

I'm not sure of the details offhand but I think this may relate to how window management is different across platforms -- the "desktop" coordinate system corresponds to device pixels on Windows IIRC, but to Cocoa's "points" (logical pixels) on macOS (and Linux? I don't remember...), and the mapping to device pixels varies depending which screen the widget is on. In particular, in a mixed-DPI configuration it doesn't work to simply convert "desktop" coordinates (the things the OS APIs use to manage windows) to device pixels (which a lot of Gecko wants to use) by multiplying by the display's scale factor, because this can result in overlapping device-pixel coordinates and confusion about where the widget belongs.

Basically... it's complicated, and platform-dependent. And so when touching this stuff, it's good to test across multiple platforms, and with various multi-monitor/mixed-resolution configurations. Unfortunately, we don't have any automated test coverage for situations like that.

Hmm, thank you for detailed explanation. Then it is not simple to remove it.

What happens with nsView::DoResetWidgetBounds() is like the following.

-[1] nsView::DoResetWidgetBounds() resizes widget to nsView::CalcWidgetBounds(nsWindowType aType) size if the size is different than widget->GetClientBounds().
-[2] nsView::DoResetWidgetBounds() calls widget->ResizeClient() to resize widget
-[3] nsBaseWidget::ResizeClient() calls Resize() to resize widget. But the ResizeClient() might change the resize value.
-[4] The Resize() might change the resize value by using nsBaseWidget::ConstrainSize()
+ For example on Windows, nsWindow::Resize()
-[5] Final resized value could be different than nsView::CalcWidgetBounds(), nsWindow::Invalidate() triggers repaint and DoResetWidgetBounds() on Windows.

Then nsView::CalcWidgetBounds() size might be applied to widget with modification. And next widget->GetClientBounds() could be different than nsView::CalcWidgetBounds() again
with several reasons. But it seems OK to apply widget->ConstrainSize() in nsView::DoResetWidgetBounds(). It could remove repaint because of widget->ConstrainSize() call in the Resize() as in [4].

Adding haik and mstange because they dealt with widget size constraints issues on mac recently.

(In reply to Sotaro Ikeda [:sotaro] from comment #14)

But it seems OK to apply widget->ConstrainSize() in nsView::DoResetWidgetBounds(). It could remove repaint because of widget->ConstrainSize() call in the Resize() as in [4].

I agree that that should at best be harmless. Do you plan to send that change for review? It'd unblock me :)

Backed out for causing failures at test_panel.xhtml.

Backout link: https://hg.mozilla.org/integration/autoland/rev/453466931a6c57d416d4b8697f7491b11fdaa0d4

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=windows10-32%2Cdebug%2Cmochitests%2Cwithout%2Ce10s%2Ctest-windows10-32%2Fdebug-mochitest-chrome-1proc%2Cc1&revision=64abec51f1cb5e876da8adb3a475384e147fdc31&selectedTaskRun=dnG-iXrzSH-OA-oYBH_WiQ.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=339532132&repo=autoland&lineNumber=13631

When the test failed,requested height in nsView::DoResetWidgetBounds() was less than mMinSize.height and desktopDelta was positive(16.0, 39.0). In this case, requested final widget size in nsWindow::Resize() was "mMinSize.height + desktopDelta.height". It happened because nsView::DoResetWidgetBounds() does not care about desktopDelta. This problem is ouf of scope for this bug. Then I am going to relax the size check.

https://treeherder.mozilla.org/jobs?repo=try&revision=da7ba94664bdcf16bd5f7ef0774a108193448b27

https://hg.mozilla.org/mozilla-central/rev/3f007ac3eea6