Closed Bug 1563918 Opened 5 months ago Closed 5 months ago

Massive performance issue after maximizing/windowing when pointer/mouse cursor is still at maximize/window button and blinking/flickering of maximize/window button after landing patch from bug #1551961

Categories

(Core :: DOM: UI Events & Focus Handling, defect, major)

69 Branch
x86_64
Windows 7
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox67.0.1 --- unaffected
firefox68 --- unaffected
firefox69 --- verified
firefox70 --- verified

People

(Reporter: Virtual, Assigned: mconley)

References

(Regression)

Details

(5 keywords)

Attachments

(3 files)

Attached video screencast.mp4

STR:

  1. Open Mozilla Firefox browser
  2. Use windowed mode but enlarge browser window like in maximized mode
  3. Press maximize/window button few times
    and enjoy massive performance issue when pointer/mouse cursor is still at maximize/window button and blinking/flickering of maximize/window button

Moving pointer/mouse cursor from maximize/window button fixes this issue.

Gecko Profiler profile:
https://perfht.ml/2YCEV1B

Regression caused by:
Bug #1551961

Differential Revision:
https://phabricator.services.mozilla.com/D36060

@ Mike Conley (:mconley) - Seems like your patch caused this regression, could you please look at it? Thanks!

Flags: needinfo?(mconley)
Has Regression Range: --- → yes
Has STR: --- → yes
Flags: needinfo?(mconley)

No dice reproducing this on Windows 10. Dusting off my Windows 7 VM to see if I can reproduce there.

I'm attaching Graphics section from about:support, maybe this will be some clue, like disabled WebRender etc.

Okay, I could reproduce this easily on Windows 7. Starting investigation...

I think I've figured this out. Strangely, using WindowFromPoint via WindowAtMouse here causes us to re-enter the WM_NCMOUSELEAVE message handler, so we just keep processing WM_NCMOUSELEAVE again and again and again.

We've got an alternative function, EventIsInsideWindow defined here that does much the same thing as WindowAtMouse, but does so without WindowFromPoint. Using that, I can no longer reproduce the issue. I'll have a patch posted tomorrow for testing.

No, I can't reproduce this bug anymore with build from comment #7.

Flags: needinfo?(Virtual)

In bug 1551961, a WM_NCMOUSELEAVE message handler was added to the Windows widget
backend so that we can detect when the mouse leaves a draggable region (since
draggable regions are hittest'ed as HTCAPTION, and are therefore "non-client").

Inside that message handler, we used the WindowAtMouse handler to determine whether
or not the WM_NCMOUSELEAVE was firing because the mouse was moving off of the
non-client area to client area, or moving out of the window entirely.

For reasons that are not particularly clear, on Windows Aero Glass, when clicking on
one of the min/max/close caption buttons, a WM_NCMOUSELEAVE message fires, and the
::WindowFromPoint call that WindowAtMouse uses causes another WM_NCMOUSELEAVE message
to be queued immediately after, so we end up in this situation where the message
loop is getting spammed by WM_NCMOUSELEAVE messages. Moving the mouse away from
the caption buttons seems to stop the spamming.

We have a function similar to WindowAtMouse called EventIsInsideWindow which
does not use ::WindowFromPoint, and this seems to sidestep the issue, while
being functionally equivalent.

Depends on D37421

Assignee: nobody → mconley
Status: NEW → ASSIGNED
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d303fb1c4374
Avoid spamming native message loop with WM_NCMOUSELEAVE messages. r=jmathies
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

Hey Virtual,

Can you verify that this is now fixed in the most recent Nightly? If so, I'll request uplift to beta. Thanks!

Flags: needinfo?(Virtual)

Sure. I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 70.0a1 (2019-07-11), so I'm marking this bug as VERIFIED.
Thank you very much one more time! \o/

P.S. In your free time please see also bug #1564540, which I'm suspecting that it's also caused by bug #1551961.

Status: RESOLVED → VERIFIED
Flags: needinfo?(Virtual)

Commented in bug 1564540.

Flags: needinfo?(mconley)

Comment on attachment 9076812 [details]
Bug 1563918 - Avoid spamming native message loop with WM_NCMOUSELEAVE messages. r?jmathies

Beta/Release Uplift Approval Request

  • User impact if declined: Users using Windows Aero on Windows 7 can get into situations where the browser window becomes unresponsive while their mouse cursor is over the caption (min/max/close) buttons.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This works around a weird quirk in how a win32 API (WindowFromPoint) works, by using a different method that accomplishes the same task but without hitting the quirk. (The quirk is what made us spam the message loop with WM_NCMOUSELEAVE messages).
  • String changes made/needed: None.
Attachment #9076812 - Flags: approval-mozilla-beta?

Comment on attachment 9076812 [details]
Bug 1563918 - Avoid spamming native message loop with WM_NCMOUSELEAVE messages. r?jmathies

Fixes a big perf regression in 69 for some users. Approved for 69.0b6.

Attachment #9076812 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

This issue was Verified in Beta and Nightly, I will remove the qeverify+ flags.

Flags: qe-verify+
QA Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.