Note: There are a few cases of duplicates in user autocompletion which are being worked on.

NS_MOUSE_MOZHITTEST is fired several times a second on idly hovering the browser window

RESOLVED DUPLICATE of bug 506815

Status

()

Core
Event Handling
RESOLVED DUPLICATE of bug 506815
3 years ago
2 years ago

People

(Reporter: gfritzsche, Assigned: chutten)

Tracking

Trunk
All
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

NS_MOUSE_MOZHITTEST is fired several times a second on idly hovering the browser window.

* trace events e.g. here: http://hg.mozilla.org/mozilla-central/annotate/29d086b32a26/dom/events/EventStateManager.cpp#l516
* hover over the browser window, e.g. over content
* observe that the event tracing is not stopping even though the mouse is idly hovering

We checked this back to August 04, but first noticed it in recent builds.
This seems too frequent, so we should investigate why we do this.

Updated

3 years ago
Keywords: regressionwindow-wanted

Comment 2

3 years ago
What is higher up in the stack? Where are we dispatching the event?

Comment 3

3 years ago
I guess somehow http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp#5690
which means http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp#4737.


(But I think we should filter out NS_MOUSE_MOZHITTEST in this case so that it doesn't trigger "user-interaction-active". NS_MOUSE_MOZHITTEST isn't really about user being active.)
More details should be upcoming (later or after the workweek) from avih or me.
We think the hit test firing constantly at this rate at idle user input cant be right anyway.
Do you happen to know the goal of the hittest messages here?

Comment 5

3 years ago
I think https://bugzilla.mozilla.org/show_bug.cgi?id=555081 added the relevant code.

Comment 6

3 years ago
Do we know if this a bug? maybe it's by design to work this way?

One result of this behavior which is clearly a bug is that user-interaction-active keeps firing as long as the mouse hovers the browser even when the user is idle (bug 1107782).

Maybe Felipe known? (as he was assigned to bug 555081).
Flags: needinfo?(felipc)

Updated

3 years ago
Blocks: 1107782
It's possible that Windows just fires WM_NCHITTEST messages very often. It's also possible that we're doing something that results in a higher frequency than usual, but I don't know what that would be or how we would find out.

We should probably use the nsIWidget::UpdateWindowDraggingRegion infrastructure that I added for Mac in bug 944836 on Windows. Then we'd respond to WM_NCHITTEST messages by simply testing whether the point is inside a region that's stored on the widget, and we wouldn't need to send any  NS_MOUSE_MOZHITTEST messages any more. For that to work properly we need to make sure that -moz-window-dragging is set on the browser chrome XUL elements in a way that matches the current behavior.
Flags: needinfo?(twalker)
Flags: needinfo?(anthony.s.hughes)
I'll take this
Flags: needinfo?(anthony.s.hughes)
Where do you *watch* the event tracing reported here?
needInfo me again when event tracing process is explained.  thanks.

Updated

3 years ago
Flags: needinfo?(twalker)
I pinged smaug on IRC about this:

smaug: RyanVM: so in principle adding event listener for chrome window: window.addEventListener("MozMouseHittest", function() {/* do something */}, true);
smaug: dump("foo"); should work, if enabled in prefs
smaug: (prints to stdout)
Flags: needinfo?(twalker)
Sorry, this appears to be a over my head. I won't be able to nail down a regression range.
Flags: needinfo?(twalker)
Assignee: nobody → chutten
(Assignee)

Comment 13

2 years ago
I have myself nearly convinced that this is MouseTrailer's fault (which ought to be fixed by its removal in bug 506815).

MouseTrailer will, every 200ms, ask what window's under the mouse pointer (see http://mzl.la/2100y8C). This asks Windows to give us all a WM_NCHITTEST which we then have to deal with.

+ni jimm to comment on the likelihood that my supposition is correct and on the likelihood that the solution to bug 506815 will take care of this as well.
Flags: needinfo?(jmathies)
That rings a bell! I remember looking into this a long time ago while doing some Windows 7 work and IIRC you're spot on. Bug 506815 would certainly take care of the problem as described in this bug (messages being sent every 200ms when mouse is idle), but not when the mouse is moved.


The question is whether this is actually causing any problems. These msgs used to be more expensive, but a cache was introduced in bug 898126. Although the cache is only for 50ms, so it doesn't overlap with the interval here, but presumably it helps deal with most of the messages anyway? If not maybe we can increase the interval of that cache to be higher than 200ms (that is, if bug 506815 doesn't get fixed).

Another way to make these msgs less expensive is what mstange suggested here in comment 7.
Flags: needinfo?(felipc)
(Assignee)

Updated

2 years ago
Depends on: 506815
Keywords: regressionwindow-wanted
(Assignee)

Comment 15

2 years ago
Why not do both? We could remove time-sensitivity from the cache (since it's scrubbed on window geometry changes anyway and is keyed to a point) _and_ rid ourselves of unnecessary kernel hiccoughs (and device wakeups) from MT's 200ms timer.
(In reply to Chris H-C :chutten from comment #15)
> Why not do both? We could remove time-sensitivity from the cache (since it's
> scrubbed on window geometry changes anyway and is keyed to a point)

I'm not seeing any code that scrubs the cache. What are you referring to?

(In reply to :Felipe Gomes from comment #14)
> Another way to make these msgs less expensive is what mstange suggested here
> in comment 7.

It looks like bug 1163113 already started implementing this. It should be possible to remove the code that sends the eMouseHitTest events now, and to sprinkle -moz-window-dragging:drag over the chrome XUL such that the current draggable areas stay draggable.
(Assignee)

Comment 17

2 years ago
Oh goodness, I must've read that somewhere else during my investigation. You're right, there is no such mechanism.

The most important work I feel is to stop the 200ms wakeup timer. It's bad for battery, and it's on main thread.

Updated

2 years ago
Duplicate of this bug: 899243

Comment 19

2 years ago
(In reply to :Gijs Kruitbosch from bug 899243 comment #2)
> Note that all this intersects with other effects the current mousehittest
> stuff has, mostly because mouseleave and so on go missing on empty bits of
> the tabstrip and stuff like that. Felipe knows more.

Specifically, see bug 1220486 and bug 1195067.

Comment 20

2 years ago
(In reply to Markus Stange [:mstange] from comment #7)
> It's possible that Windows just fires WM_NCHITTEST messages very often. It's
> also possible that we're doing something that results in a higher frequency
> than usual, but I don't know what that would be or how we would find out.

It does, but only when the mouse is moving. If idle it shouldn't.

> MouseTrailer will, every 200ms, ask what window's under the mouse pointer
> (see http://mzl.la/2100y8C). This asks Windows to give us all a WM_NCHITTEST
> which we then have to deal with.
> 
> +ni jimm to comment on the likelihood that my supposition is correct and on
> the likelihood that the solution to bug 506815 will take care of this as
> well.

Seems too, I commented out the WindowFromPoint call and the WM_NCHITTEST events that triggered while the mouse was idle went away.

(In reply to :Gijs Kruitbosch from comment #19)
> (In reply to :Gijs Kruitbosch from bug 899243 comment #2)
> > Note that all this intersects with other effects the current mousehittest
> > stuff has, mostly because mouseleave and so on go missing on empty bits of
> > the tabstrip and stuff like that. Felipe knows more.
> 
> Specifically, see bug 1220486 and bug 1195067.

Yeah MouseTrailer has been removed before, it always seems to regress something and we have to bring it back. :(
Flags: needinfo?(jmathies)
(Assignee)

Comment 21

2 years ago
Hopefully MouseTrailer's gone for good, this time: bug 506815.

506815 seems to cover the original intent of this bug, but I'm intrigued by :mstange's comment 7. I can't read the cocoa impl's Obj-C well enough to quite understand what would be needed of Windows. Quite a bit of bug 944836's patches went into cross-platform code, which suggests to me that the load might be light enough to take care of here.

Otherwise, maybe we should find a new bug for this work.
Flags: needinfo?(mstange)
(In reply to Chris H-C :chutten from comment #21)
> Hopefully MouseTrailer's gone for good, this time: bug 506815.

Nice!

I think we should close this bug once MouseTrailer is really gone, because it fixes what was originally reported here. I've filed bug 1227562 for getting rid of the hit test events, with more detailed instructions.
Flags: needinfo?(mstange)
(Assignee)

Comment 23

2 years ago
I'll go ahead and dupe this against MouseTrailer, then, as it's now in m-c. Here's hoping it doesn't resurrect.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 506815
(Assignee)

Updated

2 years ago
Blocks: 1228368
You need to log in before you can comment on or make changes to this bug.