1859484 - Chrome-compatible about:blank patch causes unwanted changed rectangles on browser/base/content/test/performance/browser_windowopen.js

Status: NEW

(Toolkit :: Performance Monitoring, defect, P2)

Description

[Henri Sivonen (:hsivonen)]

1. Compile with the patch from bug 543435
2. Run ./mach test browser/base/content/test/performance/browser_windowopen.js

This causes a bunch of failures with "unexpected changed rect".

I could use some guidance for where to even look for the cause.

Does the test rely on some painting or layout suppression mechanism that my patch unsuppresses too early for some reason?

Visually, when running the test locally, at least the search field on the home page first shows up as search engine-unspecific and then updates to show Google as the selected search engine.

In order to make the about:blank change tractable, my patch creates the initial about:blank eagerly, which might affect things if previously this test managed to not to trigger the lazy creation of the initial about:blank. However, when the initial about:blank is navigated away from right away, the initial about:blank is at least not supposed to report itself via onLocationChange to the UI.

Comment 1

[Florian Quèze [:florian]]

(In reply to Henri Sivonen (:hsivonen) from comment #0)

1. Compile with the patch from bug 543435
2. Run ./mach test browser/base/content/test/performance/browser_windowopen.js

Instead of these steps, I looked at the log in https://treeherder.mozilla.org/logviewer?job_id=432779746&repo=try&lineNumber=7177 from one of the try runs you linked in bug 543435.

The unexpected change is for the entire browser window. Going from an unexpected data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVQIW2P4DwQACfsD/Z8fLAAAAAAASUVORK5CYII= 1px image to an image of the full browser window.

Does the test rely on some painting or layout suppression mechanism that my patch unsuppresses too early for some reason?

The test uses a canvas to grab a screenshot of the entire window for every MozAfterPaint DOM event. It's likely that one of these events is fired too early with your patches.

Comment 2

[Henri Sivonen (:hsivonen)]

An update:

1. The 1x1 pixel frame wasn't due to an actual MozAfterPaint but due to the harness also taking a snapshot if the the harness attaches when readyState == "complete", and the initial about:blank's readyState is always "complete" (terrible if considered as platform design but Chrome-compatible).
2. I implemented paint suppression for the initial about:blank past its load event to keep painting suppressed during the pervasive front end pattern of first creating a XUL browser and only then navigating it away from about:blank (as opposed to giving it a URL to navigate to as part of creation). This test still fails with this paint suppression addition in place.

Comment 3

[Molly Howell (she/her) (mostly inactive)]

(this bug showed up in my triage list this week but I'm not entirely sure of the right way to handle it; please do feel free to change these flags as needed)

Comment 4

[Henri Sivonen (:hsivonen)]

Florian, do you have more advice on what I should try next given that item 2 from comment 6 was not enough?

Comment 5

[Florian Quèze [:florian]]

(In reply to Henri Sivonen (:hsivonen) from comment #4)

Florian, do you have more advice on what I should try next given that item 2 from comment 6 was not enough?

Is the test still catching a 1x1px window, or is it something else now? (A link to your latest try run might help)

Comment 6

[Henri Sivonen (:hsivonen)]

The failure on try is now being masked by other failures. I'll get back to this once I've dealt with the failures that mask this.

Comment 7

[:Gijs (he/him)]

Going to clear this needinfo per comment #6. But also, fwiw, reading this:

(In reply to Florian Quèze [:florian] from comment #1)

The unexpected change is for the entire browser window. Going from an unexpected data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVQIW2P4DwQACfsD/Z8fLAAAAAAASUVORK5CYII= 1px image to an image of the full browser window.

Does the test rely on some painting or layout suppression mechanism that my patch unsuppresses too early for some reason?

The test uses a canvas to grab a screenshot of the entire window for every MozAfterPaint DOM event. It's likely that one of these events is fired too early with your patches.

I expect a potential fix would be to refuse to paint the initial about:blank in cases where that docshell represents a toplevel (chrome, rather than content) window. I can't see anyone wanting that to appear on screen.

Comment 8

[Vincent Hilla [:vhilla]]

This test is a known intermittent (bug 1872332). If I run the test locally, I always get a bunch of failures both on central and with the patch. On try (https://treeherder.mozilla.org/jobs?repo=try&revision=956ea36672777c8b74cd4f27ba894c2eeb0290d5), I lately saw no failure. Maybe Henri's paint suppression was enough to fix bug 543435 specific issues?

Comment 9

[Florian Quèze [:florian]]

(In reply to Vincent Hilla [:vhilla] from comment #8)

This test is a known intermittent (bug 1872332).

The known relatively frequent intermittent failure of this test happens only on Mac. I think the failures discussed here were also on other platforms. But great if you don't see failures anymore in newer try runs :-).

Comment 10

[Vincent Hilla [:vhilla]]

Thanks for pointing this out, then the test still fails: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=RwO1ks8TSHikpYU2s67xrQ.0&revision=2d8188a74b910a7aeb4ba661894bdf134c7a3c96 (nvm, it's Mac)