285000 - MouseTrailer handling needs fixing up a bit more

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Boris Zbarsky [:bzbarsky]]

See bug 125386 comment 57.

In brief, we should not be filing the TimerProc() unless the window on the mouse
trailer is changing.  This suggests that what should really happen here is that
we should have MouseTrailer call TimerProc() itself when a new (different!)
window is set.  One exception may be if the setting is because the old window
has died -- in that case we probably want to add a corresponding argument to
SetMouseTrailerWindow to indicate this.

I suspect that fixing this may well fix bug 284951...

Comment 1

[Ere Maijala (slow)]

nsWindow doesn't remove itself from the mouseTrailer in destructor, it just
kills the timer. That's why we might end up calling a dead window.

Comment 2

[Ere Maijala (slow)]

Taking.

Comment 3

[Ere Maijala (slow)]

Attachment: Patch v1

bz, is this along the lines you wanted?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 177035 [details] [diff] [review]
Patch v1

Sure, but why not move the CreateTimer() calls into SetMouseTrailerWindow too? 
And remove the GetMouseTrailer(0) thing from those two nsWindow callsites?

r+sr=bzbarsky with that.

Comment 5

[Ere Maijala (slow)]

Attachment: Patch v2

Patch v2. I rewrote the MouseTrailer as a proper singleton. Much cleaner in my
opinion (and actually works without leaking), but needs a new review.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 177048 [details] [diff] [review]
Patch v2

Looks great!  r+sr=bzbarsky

Comment 7

[Ere Maijala (slow)]

Patch checked in. Hoping for the best..

Comment 8

[Ere Maijala (slow)]

I had to make the destructor of MouseTrailer public to satisfy VC++ 6 (a bug I
guess).