Intermittent test_moz_mouse_pixel_scroll_event.html | prepareScrollUnits: gScrollable96.wheelHorizontalLine may be illegal value, got 10

RESOLVED FIXED in Firefox 46

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: KWierso, Assigned: tnikkel)

Tracking

({intermittent-failure})

unspecified
mozilla47
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 unaffected, firefox46 fixed, firefox47 fixed, firefox-esr45 unaffected)

Details

(Whiteboard: [gfx-noted])

Attachments

(3 attachments)

Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
17 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 7
* try: 5
* fx-team: 2
* b2g-inbound: 2
* mozilla-central: 1

Platform breakdown:
* linux64: 12
* linux32: 5

For more details, see:
http://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-09-28&endday=2015-10-04&tree=all
This is probably a regression from turning on APZ on Linux. Most likely a similar issue to bug 1209942.
Blocks: 1143856
Component: DOM: Events → Panning and Zooming
27 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 16
* fx-team: 5
* mozilla-central: 4
* try: 1
* b2g-inbound: 1

Platform breakdown:
* linux64: 17
* linux32: 10

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-10-05&endday=2015-10-11&tree=all
21 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 12
* b2g-inbound: 6
* try: 2
* fx-team: 1

Platform breakdown:
* linux64: 11
* linux32: 9
* mulet-linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-10-12&endday=2015-10-18&tree=all
17 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 8
* fx-team: 5
* b2g-inbound: 3
* try: 1

Platform breakdown:
* linux32: 9
* linux64: 8

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-10-19&endday=2015-10-25&tree=all
26 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 12
* try: 6
* fx-team: 4
* mozilla-central: 2
* b2g-inbound: 2

Platform breakdown:
* linux64: 17
* linux32: 9

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-10-26&endday=2015-11-01&tree=all
24 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 8
* fx-team: 6
* b2g-inbound: 6
* mozilla-central: 3
* try: 1

Platform breakdown:
* linux32: 14
* linux64: 10

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-11-02&endday=2015-11-08&tree=all
23 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 10
* fx-team: 9
* mozilla-central: 3
* try: 1

Platform breakdown:
* linux32: 15
* linux64: 8

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-11-09&endday=2015-11-15&tree=all
20 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 10
* mozilla-central: 4
* fx-team: 4
* b2g-inbound: 2

Platform breakdown:
* linux64: 10
* linux32: 10

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-11-16&endday=2015-11-22&tree=all
20 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 9
* fx-team: 9
* mozilla-central: 1
* b2g-inbound: 1

Platform breakdown:
* linux64: 11
* linux32: 9

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-11-23&endday=2015-11-29&tree=all
19 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 13
* mozilla-central: 3
* fx-team: 2
* try: 1

Platform breakdown:
* linux32: 14
* linux64: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-11-30&endday=2015-12-06&tree=all
13 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 7
* try: 2
* mozilla-central: 2
* fx-team: 2

Platform breakdown:
* linux32: 7
* linux64: 6

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-12-07&endday=2015-12-13&tree=all
16 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 11
* fx-team: 2
* try: 1
* mozilla-central: 1
* b2g-inbound: 1

Platform breakdown:
* linux64: 9
* linux32: 7

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-12-14&endday=2015-12-20&tree=all
7 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 3
* b2g-inbound: 2
* try: 1
* fx-team: 1

Platform breakdown:
* linux32: 5
* linux64: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-12-21&endday=2015-12-27&tree=all
7 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 3
* try: 1
* mozilla-central: 1
* fx-team: 1
* b2g-inbound: 1

Platform breakdown:
* linux64: 5
* linux32: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2015-12-28&endday=2016-01-03&tree=all
21 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 8
* fx-team: 7
* try: 3
* mozilla-central: 2
* b2g-inbound: 1

Platform breakdown:
* linux32: 15
* linux64: 6

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2016-01-04&endday=2016-01-10&tree=all
Blanket gfx-noted on APZ intermittent failures so they can be found more easily on https://brasstacks.mozilla.com/orangefactor/
Whiteboard: [gfx-noted]
15 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 6
* fx-team: 4
* try: 3
* mozilla-central: 1
* b2g-inbound: 1

Platform breakdown:
* linux64: 10
* linux32: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2016-01-11&endday=2016-01-17&tree=all
(Assignee)

Comment 22

3 years ago
This appears to be caused by the mozmousepizelscroll handling getting installed after the page is fully painted, so the event regions on the layer tree do not contain the information that a wheel event shouldn't scroll the page. Here is a try run where I force the page to paint after the mozmousepizelscroll handler is added to verify my theory as correct

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

So to fix this we just need to make sure that adding a mozmousepizelscroll triggers a paint.
16 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 10
* fx-team: 3
* try: 1
* mozilla-central: 1
* b2g-inbound: 1

Platform breakdown:
* linux64: 9
* linux32: 7

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2016-01-18&endday=2016-01-24&tree=all
Are you still working on this? The bug I started to look at seems to have the same root cause (not very surprising, it's another failure mode of the same test). I can work on the patch if you are busy with image lib stuff.
Depends on: 1210312
Flags: needinfo?(tnikkel)
(Assignee)

Comment 25

3 years ago
Yeah, I was hoping to get back to this soon. All the failures in this test are likely the same issue and if they aren't probably best to just wait until the known issue is fixed.
Flags: needinfo?(tnikkel)
8 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 6
* try: 1
* mozilla-central: 1

Platform breakdown:
* linux64: 7
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2016-01-25&endday=2016-01-31&tree=all
(Assignee)

Comment 27

3 years ago
My naiva patch to fix this didn't seem to fix it

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

hmmmm
(Assignee)

Comment 28

3 years ago
Couple of things that are likely problems:
-mTarget isn't a nINode, so that chunk does nothing for the event handler added in this test
-SchedulePaint will schedule a paint, but the paint will likely not have started by the time the test starts sending events that need to be handled correctly (and so need the updated layer tree)
(Assignee)

Comment 29

3 years ago
(In reply to Timothy Nikkel (:tnikkel) from comment #28)
> Couple of things that are likely problems:
> -mTarget isn't a nINode, so that chunk does nothing for the event handler
> added in this test

That's not really a problem, we can get the presshell and ask for a paint a different way for that case, but it's still not enough because of...

> -SchedulePaint will schedule a paint, but the paint will likely not have
> started by the time the test starts sending events that need to be handled
> correctly (and so need the updated layer tree)

Not really sure what we can do here. In order for this to work we'd need to do a sync paint here, and then wait until that layer tree is transferred over to the parent process to. But that is a no-go because we install event handlers from frame construction, and we cannot paint during frame construction.

Options:
1) is it only tests (with special privileges that web content doesn't have) that can send events that are "apz aware"? I don't think so...
2) force a full paint and layer tree sync before processing an apz aware event.
Option 1 is good - web content generally cannot send events through APZ, so in the worst case they register a listener and then the *user* has to provide the input event before the next repaint in order to tickle this bug. I think triggering a repaint is good enough here, and we can modify the test to wait for the pending paint before proceeding.
20 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 7
* fx-team: 6
* mozilla-aurora: 3
* try: 2
* mozilla-central: 2

Platform breakdown:
* linux64: 12
* linux32: 8

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2016-02-01&endday=2016-02-07&tree=all
(Assignee)

Comment 32

3 years ago
Created attachment 8717362 [details] [diff] [review]
Make test wait for paint
Attachment #8717362 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 33

3 years ago
Created attachment 8717363 [details] [diff] [review]
Schedule paint on apz aware listener added
Assignee: nobody → tnikkel
Attachment #8717363 - Flags: review?(bugs)
Comment on attachment 8717362 [details] [diff] [review]
Make test wait for paint

Review of attachment 8717362 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thanks! The other patch more properly belongs on bug 1131188, but I guess you can just dupe it over.
Attachment #8717362 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8717363 [details] [diff] [review]
Schedule paint on apz aware listener added

>   if (IsApzAwareEvent(aTypeAtom)) {
>     nsCOMPtr<nsINode> node = do_QueryInterface(mTarget);
>     if (node) {
>       node->SetMayHaveApzAwareListeners();
>     }
>+    SchedulePaintForTarget();
So we QI to nsINode in this 'if'.


>+EventListenerManager::SchedulePaintForTarget()
>+{
>+  nsIDocument* doc = nullptr;
>+  if (nsCOMPtr<nsINode> node = do_QueryInterface(mTarget)) {
>+    doc = node->OwnerDoc();
>+  }
And then again here.  In general would be good to avoid extra QI calls, since those are slow virtual calls.
But this shouldn't be too hot code path.


So we don't need to reschedule paint when a node, which has apz related listener, from another document is adopted to our document?
Attachment #8717363 - Flags: review?(bugs) → review+
(Assignee)

Comment 36

3 years ago
(In reply to Olli Pettay [:smaug] (high review load) from comment #35)
> So we QI to nsINode in this 'if'.
>
> And then again here.  In general would be good to avoid extra QI calls,
> since those are slow virtual calls.
> But this shouldn't be too hot code path.

I moved the SetMayHaveApzAwareListeners call into schedule paint function (and renamed that function ProcessApzAwareEventListenerAdd) so we could just do one QI.

> So we don't need to reschedule paint when a node, which has apz related
> listener, from another document is adopted to our document?

The insertion notifications into the new document will trigger frame construction and that will trigger painting if necessary. display: none can't receive events, and nodes that aren't in the dom tree can't receive events, so we should be fine I think. ni to make sure you agree.
Flags: needinfo?(bugs)
Ok, right, and we don't care to optimize here to check for only nodes which do have or will have a layout object. We just re-paint always. Ok, fine.
Flags: needinfo?(bugs)

Comment 39

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cccdf7779f2c
https://hg.mozilla.org/mozilla-central/rev/b1a7043faedd
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
No longer depends on: 1131188
Duplicate of this bug: 1131188
10 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 5
* fx-team: 3
* try: 1
* mozilla-aurora: 1

Platform breakdown:
* linux64: 5
* linux32: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1210057&startday=2016-02-08&endday=2016-02-14&tree=all
Can you please nominate this for Aurora approval?
status-firefox45: --- → unaffected
status-firefox46: --- → affected
status-firefox-esr45: --- → unaffected
Flags: needinfo?(tnikkel)
(Assignee)

Updated

3 years ago
Depends on: 1253052
(Assignee)

Comment 43

3 years ago
I realized we should probably do this a little better, so I wrote bug 1253052. I'll ask for uplift on that too when it lands.
Flags: needinfo?(tnikkel)
(Assignee)

Comment 44

3 years ago
Comment on attachment 8717362 [details] [diff] [review]
Make test wait for paint

Approval Request Comment
[Feature/regressing bug #]: none really?
[User impact if declined]: sheriffs have to star this failure a lot
[Describe test coverage new/current, TreeHerder]: it's a test
[Risks and why]: safe, it's a test
[String/UUID change made/needed]: none
Attachment #8717362 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 45

3 years ago
Comment on attachment 8717363 [details] [diff] [review]
Schedule paint on apz aware listener added

Approval Request Comment
[Feature/regressing bug #]: turning on apz
[User impact if declined]: if a webpage registers event listeners for some events that usually cause scrolling the page won't act right
[Describe test coverage new/current, TreeHerder]: this big is about fixing a test failure
[Risks and why]: pretty safe, just asks for an extra paint sometimes
[String/UUID change made/needed]: none
Attachment #8717363 - Flags: approval-mozilla-aurora?
Comment on attachment 8717363 [details] [diff] [review]
Schedule paint on apz aware listener added

Fixing a test failure from APZ, may help scrolling. 
Please uplift to aurora.
Attachment #8717363 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Reporter)

Comment 48

3 years ago
Had to back these out of aurora for bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=2076653&repo=mozilla-aurora

https://hg.mozilla.org/releases/mozilla-aurora/rev/1ffe8313fbe3
status-firefox46: fixed → affected
Flags: needinfo?(tnikkel)
(Assignee)

Comment 49

3 years ago
Created attachment 8726970 [details] [diff] [review]
"Schedule paint on apz aware listener added" for aurora
Flags: needinfo?(tnikkel)
(Reporter)

Comment 50

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/21cf28c789ce
status-firefox46: affected → fixed
Comment on attachment 8717362 [details] [diff] [review]
Make test wait for paint

test fix, ok to land on aurora
Attachment #8717362 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.