1303649 - [e10s] flicker of lists when closing or opening menu on trello.com due to interruption of reflow

Status: RESOLVED INCOMPLETE

(Core :: Layout, defect)

Description

[Karl Tomlinson (:karlt)]

The page needs an account to view.

1. Ensure "Enable multi-process Nightly" is checked.
2. Load a board with a number of lists so that they do not all fit in one
   viewport.  e.g. https://trello.com/b/qxgTGTHC/ if you have access
3. Click "Show menu", top-right.
4. After menu opens, close menu with click on X.
5. Repeat from 3 as necessary.

Don't move the mouse during the open and close animations.

Expected: lists remain visible during animation, as with e10s disabled.
Actual: lists often flicker.

Reproduces in winnt and linux, at least.

Regressed
https://hg.mozilla.org/mozilla-central/log?rev=ancestors(4de1094b41b2)-ancestors(11c3335d77fb751128c456daa204385d69678484)

Suspect changes for bug 1255968.

Comment 1

[Karl Tomlinson (:karlt)]

Based on comments in bug 1292781, I suspect SynthMouseMoveEvent may be involved.

Even without e10s (and prior to changes for bug 1255968), moving the mouse during animations would cause the lists to flicker on slower systems.  I wonder how often it is important to interrupt reflow for the sake of pointer motion.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Is this still an issue?

Comment 3

[Karl Tomlinson (:karlt)]

This became much less frequent after bug 1301194 was fixed.

Still I see this occasionally with the Linux amd64 2017-01-05 Nightly when I force CPUs to run at 933MHz.

I suspect the underlying issue is still there, but this shows up less frequently because the animation is proceeding more quickly.

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

Hi Karl, we looked at this during regression triage. When you're back can you comment on next steps?

Comment 5

[Karl Tomlinson (:karlt)]

I guess removing Msg_SynthMouseMoveEvent__ID from PuppetWidget::HasPendingInputEvent() might remove the regression.

Perhaps the author or reviewer of 
https://hg.mozilla.org/mozilla-central/rev/4de1094b41b2
can comment on why that is there but https://bugzilla.mozilla.org/show_bug.cgi?id=1292781#c2 implies there is no reason.

Comment 6

[Boris Zbarsky [:bzbarsky]]

So on other widget types (aka without e10s) what do we do?

On Linux we check for pending events including PointerMotionMask etc.

On Windows we check for QS_INPUT in addition to window move/resize.

On Mac we check for stuff including kCGEventMouseMoved.

Those are all kind of on purpose.  See bug 67752 comment 71 through bug 67752 comment 74. 

So e10s checking for mouse moves in general is consistent with that.  On the other hand, what is the difference between SynthMouseMoveEvent and RealMouseMoveEvent for PuppetWidget?

Looking at TabParent::SendRealMouseEvent it looks like that case corresponds to eMouseMove == aEvent.mMessage && aEvent.mReason == WidgetMouseEvent::eSynthesized.  That can happen in a few cases:

1) nsContentUtils::SendMouseEvent with aIsWidgetEventSynthesized.  In practice that's only true when called via the testing method in windowutils, and then only if the caller wants it to be.

2) PresShell::ProcessSynthMouseMoveEvent -- mouse move events firing as a result of layout moving things under the pointer.

Given that, I agree we should remove SynthMouseMoveEvent from the list here.

Note that the same problem might still happen if you just legitimately move the mouse...

Comment 7

[Karl Tomlinson (:karlt)]

I'll put up a patch to remove SynthMouseMoveEvent from HasPendingInputEvent() in bug 1341186, but that is not the cause of this bug.

When this bug demonstrates, HasPendingInputEvent() is finding Msg_RealMouseButtonEvent__ID, so it seems that the mouse button is triggering interruption of a reflow in progress, which is as designed.

Performance still seems to be slightly worse with e10s than without, on Linux and NT, but it is inconsistent so hard to compare.

I don't think there is much point keeping this bug open, as there are bugs with clearer problems to investigate than this.

These symptoms do sometimes happen without e10s, again probably less often but the unpredictable nature of the symptoms makes it hard to compare.