Enable apz tests for e10s+windows

RESOLVED FIXED in Firefox 45

Status

()

Core
Graphics
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mrbkap, Assigned: kats)

Tracking

unspecified
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox45 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
In order to turn on e10s mochitests on for OSX, we have to figure out why the apz tests aren't passing.

As far as I can tell, there's a problem with scroll events on OSX, but only in iframes. When I ran the tests in this directory by themselves, they passed; it's only in the frame, while running the directory that they fail.
(Reporter)

Comment 1

3 years ago
Created attachment 8647820 [details] [diff] [review]
Disable the tests

We should use this bug to track turning the tests back on.
Attachment #8647820 - Flags: review?(bugmail.mozilla)
(Reporter)

Updated

3 years ago
Blocks: 1194550

Updated

3 years ago
Whiteboard: [gfx-noted]
Do you have the output from a run of the tests, or a try push showing the failures? When I run this locally on my OS X machine it passes:

mach mochitest -f plain --e10s gfx/layers/apz/test/

I'm not opposed to disabling the tests temporarily but I'd like to have some idea of what the problem is at least.
Flags: needinfo?(mrbkap)
Comment on attachment 8647820 [details] [diff] [review]
Disable the tests

Clearing review for now.
Attachment #8647820 - Flags: review?(bugmail.mozilla)
(Reporter)

Comment 4

3 years ago
I'll try again and debug a little. It looks like it'll be a bit before we can get these tests enabled on treeherder, so no rush.
Flags: needinfo?(mrbkap)
Ok, thanks. Also FYI I would expect test_layerization.html might fail, but that should be fixed in bug 1177018. The rest of the tests should be passing.
Hijacking this bug as per IRC discussion yesterday to investigate why the gfx/layers/apz/test/mochitest/test_bug1151667.html is failing with e10s on windows. The try push showing the failure is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fe342dcb884

I wasn't able to reproduce locally (I'll try harder) but in the meantime I did a try push with more logging: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc85eb2665b8
Summary: Enable apz tests for e10s+cocoa → Enable apz tests for e10s+windows
The try push I did above still hasn't been scheduled, and from trychooser it looks like the test queue on windows machines is climbing towards infinity... :(
The jobs finally finished; I looked at the logs and it appears to be the same problem as in bug 1203901 (i.e. the layer tree on the compositor side is not fully updated before we start sending the wheel events). I'll do a try push with a fix.
That try push has test_bug1151667.html passing, it seems. However test_layerization is suffering from the same problem. I'll fix that one too (and any others...) while I'm at it.
(Reporter)

Comment 12

3 years ago
Thanks for figuring this out!
Created attachment 8682549 [details] [diff] [review]
Patch for test_bug1151667 and test_layerization

Might as well get this landed. I'm still debugging the test_wheel_transactions failures which seem to have a different cause. (https://treeherder.mozilla.org/#/jobs?repo=try&revision=970f97a1e94e&group_state=expanded)
Attachment #8682549 - Flags: review?(botond)
Attachment #8681999 - Attachment is obsolete: true
What seems to be happening in test_wheel_transaction.html is that near the end of the test there are two loops with wheel events, and a timeout in between. The intent is that the timeout ends the transaction so the second set of wheel events end up going to the outer frame. Instead it looks like the last event from the first loop is delayed a tiny bit, and starts a new transaction. The first event from the second loop ends up getting added to that transaction, and goes to the inner APZC instead of the outer APZC, never triggers a scroll event, and causes the test to hang.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #14)
> What seems to be happening in test_wheel_transaction.html is that near the
> end of the test there are two loops with wheel events, and a timeout in
> between. The intent is that the timeout ends the transaction so the second
> set of wheel events end up going to the outer frame. Instead it looks like
> the last event from the first loop is delayed a tiny bit, and starts a new
> transaction. The first event from the second loop ends up getting added to
> that transaction, and goes to the inner APZC instead of the outer APZC,
> never triggers a scroll event, and causes the test to hang.

I wonder if the problem is that synthesizeNativeWheelAndWaitForScrollEvent() only waits for the _first_ scroll event triggered by the wheel event, while the delayed one is a subsequent one (and thus the first loop doesn't wait for it). It would be nice to wait for all scroll events triggered by a wheel event, I'm just not sure how to do that.
Created attachment 8683202 [details] [diff] [review]
Patch for test_wheel_transaction

A bit of discussion on IRC led us to conclude that this is most likely a timestamp resolution problem; otherwise it should be impossible for those two events to end up in the same transaction. Increasing the timeout seems to fix it, per https://treeherder.mozilla.org/#/jobs?repo=try&revision=506cd6278b0a&group_state=expanded
Attachment #8683202 - Flags: review?(botond)
Comment on attachment 8682549 [details] [diff] [review]
Patch for test_bug1151667 and test_layerization

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

::: gfx/layers/apz/test/mochitest/test_layerization.html
@@ +7,5 @@
>    <title>Test for layerization</title>
>    <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
> +  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
> +  <script type="application/javascript" src="/tests/SimpleTest/paint_listener.js"></script>
> +  <script type="application/javascript" src="apz_test_utils.js"></script>

Did alpha ordering go out of vogue? :p
Attachment #8682549 - Flags: review?(botond) → review+
Comment on attachment 8683202 [details] [diff] [review]
Patch for test_wheel_transaction

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

::: gfx/layers/apz/test/mochitest/test_wheel_transactions.html
@@ +82,5 @@
>    // Scroll up a bit more. It's still |outer| scrolling because
>    // |inner| is still scrolled all the way to the top.
>    yield scrollWheelOver(outer, 10);
>  
>    // Wait for the transaction timeout to elapse.

// timeout * 5 is used to make it less likely that the timeout is less
// than the system timestamp resolution
Attachment #8683202 - Flags: review?(botond) → review+
(In reply to Botond Ballo [:botond] from comment #17)
> > +  <script type="application/javascript" src="apz_test_utils.js"></script>
> 
> Did alpha ordering go out of vogue? :p

Whoops, fixed :)

(In reply to Botond Ballo [:botond] from comment #18)
> // timeout * 5 is used to make it less likely that the timeout is less
> // than the system timestamp resolution

Added (to both sites, for consistency).

Comment 21

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a2e622ecf00f
https://hg.mozilla.org/mozilla-central/rev/dc4dd8cbc003
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.