1619836 - High Frequency devtools/client/shared/test/browser_layoutHelpers_getBoxQuads2.js | C-div quad x position is as expected. - 80 == 130 -

Status: RESOLVED FIXED

(DevTools :: General, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: rmaries [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=291548834&repo=autoland
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/IPzzeanoQISXs2ASqkbgBg/runs/0/artifacts/public/logs/live_backing.log
Reftest URL: https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/IPzzeanoQISXs2ASqkbgBg/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

---

[task 2020-03-04T02:32:09.563Z] 02:32:09 INFO - TEST-START | devtools/client/shared/test/browser_layoutHelpers_getBoxQuads2.js
[task 2020-03-04T02:32:09.728Z] 02:32:09 INFO - TEST-INFO | started process screencapture
[task 2020-03-04T02:32:09.950Z] 02:32:09 INFO - TEST-INFO | screencapture: exit 0
[task 2020-03-04T02:32:09.950Z] 02:32:09 INFO - Buffered messages logged at 02:32:09
[task 2020-03-04T02:32:09.951Z] 02:32:09 INFO - Entering test bound
[task 2020-03-04T02:32:09.951Z] 02:32:09 INFO - Adding a new tab with URL: http://example.com/browser/devtools/client/shared/test/doc_layoutHelpers_getBoxQuads2-a.html
[task 2020-03-04T02:32:09.952Z] 02:32:09 INFO - Tab added and finished loading
[task 2020-03-04T02:32:09.954Z] 02:32:09 INFO - Running tests
[task 2020-03-04T02:32:09.954Z] 02:32:09 INFO - Document is offset 0, 4440 from window.
[task 2020-03-04T02:32:09.954Z] 02:32:09 INFO - Checking C-div.
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Buffered messages finished
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - TEST-UNEXPECTED-FAIL | devtools/client/shared/test/browser_layoutHelpers_getBoxQuads2.js | C-div quad x position is as expected. - 80 == 130 -
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Stack trace:
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - resource://testing-common/content-task.js line 110 > eval:null:38
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - TEST-UNEXPECTED-FAIL | devtools/client/shared/test/browser_layoutHelpers_getBoxQuads2.js | C-div quad y position is as expected. - 180 == 230 -
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Stack trace:
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - resource://testing-common/content-task.js line 110 > eval:null:43
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Checking E-div.
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - TEST-UNEXPECTED-FAIL | devtools/client/shared/test/browser_layoutHelpers_getBoxQuads2.js | E-div quad x position is as expected. - 380 == 430 -
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Stack trace:
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - resource://testing-common/content-task.js line 110 > eval:null:38
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - TEST-UNEXPECTED-FAIL | devtools/client/shared/test/browser_layoutHelpers_getBoxQuads2.js | E-div quad y position is as expected. - 210 == 260 -
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Stack trace:
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - resource://testing-common/content-task.js line 110 > eval:null:43
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - Leaving test bound
[task 2020-03-04T02:32:09.955Z] 02:32:09 INFO - TEST-PASS | devtools/client/shared/test/browser_layoutHelpers_getBoxQuads2.js | The main process DevToolsServer has no pending connection when the test ends -

Comment 1

[Razvan Maries]

Brad, could you take a look?

Comment 3

[Natalia Csoregi [:nataliaCs]]

This has started having a high frequency, possibly starting with Bug 1593756 as Razvan mentioned.
Closed the other bug created as duplicate.

Comment 4

[Brad Werth [:bradwerth]]

This test was just added to test a new API. I don't want to rollback the API so I'll have to disable this test while I figure it out. I'll push a patch that does so, and keep this bug open.

Comment 5

[Brad Werth [:bradwerth]]

Attachment: Bug 1619836 Part 1: Disable intermittent test of new getBoxQuadsFromWindow API.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8b8a1c04574
Part 1: Disable intermittent test of new getBoxQuadsFromWindow API. r=emilio

Comment 7

[Brad Werth [:bradwerth]]

I think the issue is that GetBoxQuadsFromWindowOrigin needs to use the parent app units per dev pixel after transforming quads by the ChildToParent matrix. PresShell::ScrollFrameRectIntoView seems to show the correct math. The challenge is that the call from the content process has no way to get this information. Perhaps the paired calls to GetBoxQuadsFromWindowOrigin will need to be augmented by an additional call to send this information to the content process.

Comment 8

[Brad Werth [:bradwerth]]

Attachment: Bug 1619836 Part 3: Make the test browser_layoutHelpers_getBoxQuads2.js more informative and forgiving.

Depends on D65338

Comment 9

[Brad Werth [:bradwerth]]

Or the issue is that the scaling factor is wrong: it's not considering AppUnitsPerCSSPixel, and that's clearly wrong.

Comment 10

[Brad Werth [:bradwerth]]

Attachment: Bug 1619836 Part 2: Change GetBoxQuadsFromWindowOrigin to include necessary factor of AppUnitsPerCSSPixel().

Depends on D65411

Comment 11

[Brad Werth [:bradwerth]]

Attachment: Bug 1619836 Part 4: Re-enable the test.

Depends on D65412

Comment 12

[Brad Werth [:bradwerth]]

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

Comment 13

[Brad Werth [:bradwerth]]

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

Comment 14

[Hiroyuki Ikezoe (:hiro)]

(In reply to Brad Werth [:bradwerth] from comment #7)

I think the issue is that GetBoxQuadsFromWindowOrigin needs to use the parent app units per dev pixel after transforming quads by the ChildToParent matrix. PresShell::ScrollFrameRectIntoView seems to show the correct math. The challenge is that the call from the content process has no way to get this information. Perhaps the paired calls to GetBoxQuadsFromWindowOrigin will need to be augmented by an additional call to send this information to the content process.

I don't quite understand why the test intermittently fails if it's caused by the units inconsistency? My guess is that the transform matrix hasn't arrived to the OOP iframe's process. We probably need to use waitUntilApzState before processing the test.

Comment 15

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/d8b8a1c04574

Comment 16

[Intermittent Failures Robot]

30 failures in 1032 pushes (0.029 failures/push) were associated with this bug yesterday.

Repository breakdown:

• autoland: 17
• mozilla-central: 11
• try: 2

Platform breakdown:

• macosx1014-64-shippable: 4
• macosx1014-64: 9
• linux1804-64: 9
• linux1804-64-shippable: 3
• windows10-64: 4
• windows10-64-devedition: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1619836&startday=2020-03-04&endday=2020-03-04&tree=all

Comment 17

[Brad Werth [:bradwerth]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #14)

I don't quite understand why the test intermittently fails if it's caused by the units inconsistency? My guess is that the transform matrix hasn't arrived to the OOP iframe's process. We probably need to use waitUntilApzState before processing the test.

I'm not sure if it is intermittent -- it might have been perma-fail on some platforms. The math seems wrong given that it doesn't consider AppUnitsPerCSSPixel, and only uses AppUnitsPerDevPixel. https://phabricator.services.mozilla.com/D65412 addresses that mistake in the original implementation. The try run at https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1b01caada7edd0f4a1a64b43b700aaa4237a46c doesn't show any failures, and I've triggered the macOS job multiple times there.

Comment 18

[Hiroyuki Ikezoe (:hiro)]

It's definitely intermittent. See https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=291585020&revision=2220f62fbd3bf974919c1e86c4f1212cc510d87c

Comment 19

[Brad Werth [:bradwerth]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #18)

It's definitely intermittent.

Okay, I'll update the patch to add a call to waitUntilApzStable and make a new repeated try run on macOS.

Comment 20

[Intermittent Failures Robot]

30 failures in 5483 pushes (0.005 failures/push) were associated with this bug in the last 7 days.

This is the #41 most frequent failure this week.

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **

Repository breakdown:

• autoland: 17
• mozilla-central: 11
• try: 2

Platform breakdown:

• macosx1014-64-shippable: 4
• linux1804-64: 9
• macosx1014-64: 9
• linux1804-64-shippable: 3
• windows10-64: 4
• windows10-64-devedition: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1619836&startday=2020-03-02&endday=2020-03-08&tree=all

Comment 21

[Brad Werth [:bradwerth]]

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

Comment 22

[Hiroyuki Ikezoe (:hiro)]

Brad, I'd suggest to open a new browser window with enabling fission just like this test, so that we don't need to care about the (intermittent) failure and when we switch to use the new API all places in devtools, we can test it in non fission browser.

Comment 23

[Brad Werth [:bradwerth]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #22)

Brad, I'd suggest to open a new browser window with enabling fission just like this test, so that we don't need to care about the (intermittent) failure and when we switch to use the new API all places in devtools, we can test it in non fission browser.

I'll attempt to make that change to the test. With the most recent patches, we no longer have intermittents because the call to waitUntilApzStable seems to work. What we have is very consistent precision errors on macOS above our tolerance level of 1 pixel. I'll attempt to recreate those problems locally so I can inspect the transforms.

Comment 24

[Brad Werth [:bradwerth]]

Per conversation with Hiro, the test has been made to force a Fission window. The API is designed for a Fission environment, and the precision errors on macOS do not occur in a Fission window.

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

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f8f03c49e42
Part 2: Change GetBoxQuadsFromWindowOrigin to include necessary factor of AppUnitsPerCSSPixel(). r=hiro
https://hg.mozilla.org/integration/autoland/rev/42e2ca98079f
Part 3: Make the test browser_layoutHelpers_getBoxQuads2.js more informative and forgiving. r=hiro
https://hg.mozilla.org/integration/autoland/rev/ca9276ce741d
Part 4: Re-enable the test. r=hiro

Comment 26

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/0f8f03c49e42
https://hg.mozilla.org/mozilla-central/rev/42e2ca98079f
https://hg.mozilla.org/mozilla-central/rev/ca9276ce741d