Closed Bug 1255022 Opened 4 years ago Closed 4 years ago
_transformed _scrolling _repaints* to mochitest-plain
From https://bugzilla.mozilla.org/show_bug.cgi?id=1254806#c30 I think these tests should be converted to mochitest-plain: layout/base/tests/chrome/test_transformed_scrolling_repaints.html layout/base/tests/chrome/test_transformed_scrolling_repaints_2.html layout/base/tests/chrome/test_transformed_scrolling_repaints_3.html
Seems to work locally. I'll do a try push
Try push had two types of failures. On Windows there was a windows-specific codepath in one test that tried using Components.classes. I think that codepath can just be taken out so I tried doing that, have another try push going. https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b09160ff081 The other failure was in the test_transformed_scrolling_repaints_3.html test on Windows and Linux only. I can repro easily on Linux but having trouble figuring out the problem. I know that it's caused by subpixel offsets on the layer which is somehow caused by APZ being enabled. If I disable APZ the test passes. I chased it a bit further and it seems like with APZ disabled, some of the ScrollByLines(1) calls scroll by 1102 app units with APZ disabled, and 1103 app units with APZ enabled. I chased it a bit more and it seems like with APZ disabled, mScrollPosForLayerPixelAlignment remains 1103 each time through ScrollToImpl but with APZ enabled it advances - 1103, 2206, 3309, and so on. This value gets used in ClampAndAlignWithLayerPixels, and seems to be implicated in the failure. I don't really understand the layer pixel alignment code, at least not at this level, so it would be better to hand this off to somebody else. I'll verify that my Components.classes removal works first though.
Good news, after rebasing my patch to master the failure went away. Love it when that happens. https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9a05f81d498
Try seems green, although "Add Jobs" is broken so I'll need to do a new push for android :(
Comment on attachment 8736939 [details] [diff] [review] Convert to mochitest-plain Review of attachment 8736939 [details] [diff] [review]: ----------------------------------------------------------------- nice
Attachment #8736939 - Flags: review?(mstange) → review+
Well that was pointless, the entire layout/base/tests directory is skipped on android. So this is good to land, I think.
(In reply to Kartikaya Gupta (email:email@example.com) from comment #4) > Good news, after rebasing my patch to master the failure went away. Love it > when that happens. > Indeed! \o/
Marking 47 as affected since according to Johnny's email I'm assuming the goal is to have all testing stuff uplifted to 47. I'll need to check that the failure doesn't happen on 47 though, before requesting uplift.
(In reply to Kartikaya Gupta (email:firstname.lastname@example.org) from comment #4) > Good news, after rebasing my patch to master the failure went away. Love it > when that happens. Whatever fixed this isn't on aurora yet. https://treeherder.mozilla.org/#/jobs?repo=try&revision=5770844a7cd0&group_state=expanded We can just let this ride the trains.
You need to log in before you can comment on or make changes to this bug.