1533450 - basic_compositor_video Talos tests are failing with QuantumBar enabled

Status: RESOLVED WORKSFORME

(Firefox :: Address Bar, defect, P5)

Description

[Mark Banner (:standard8)]

I have just been doing another Talos regression test and noticed that the basic_compositor_video (bcv) tests are failing:

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

They weren't on a previous try run.

On one of the windows builds, I saw:

JavaScript error: chrome://browser/content/urlbarBindings.xml, line 1359: TypeError: this.valueFormatter is undefined

I believe that the bcv test is doing something to disable browser chrome.

So it could be that something has changed which means we're now not handling it correctly.

Comment 1

[Marco Bonardo [:mak]]

does it work if you also change the quantumbar attribute to true in browser.xul?
While the pref switch allows to switch across the implementations, it's not perfect, because of xul, switching both the pref and the attribute makes it like quantumbar is enabled by default, rather than switching it on on startup.

Comment 2

[Mark Banner (:standard8)]

(In reply to Marco Bonardo [::mak] from comment #1)

does it work if you also change the quantumbar attribute to true in browser.xul?
While the pref switch allows to switch across the implementations, it's not perfect, because of xul, switching both the pref and the attribute makes it like quantumbar is enabled by default, rather than switching it on on startup.

So I tried this here. The bcv passes.

I also ran mochitests, as a previous try run showed lots of leaks, so I think that's the same issue.

Comment 3

[Marco Bonardo [:mak]]

yes, as far as I could try, it's not possible to completely eradicate and replace the binding in a fully transparent way. That's why the attribute exists and defaults to false, so in the default case (QB off) we don't do anything additional to the status-quo (no fancy binding replacements). In the study case we'll flip the pref on the fly, and we may leak on shutdown (even if I couldn't reproduce those leaks locally).
To activate the feature in Nightly in a definite way, we'll also have to flip the attribute. I also tried to set the attribute based on the pref in onBeforeInitialXULLayout(), but that doesn't work (along with other tens of experiments).

For now I can only suggest that we should flip both to run tests properly. Unless someone wants to deep dive the underlying reasons for which the runtime replacement likes to leak in a subset of tests.

Comment 4

[Marco Bonardo [:mak]]

I'd suggest to close this as WFM, the test only fails when QB is dinamically enabled (pref flip), but not when it's statically enabled (pref and attribute flip). Our target is to enable the QB statically for everyone and the tests will go through that status.

Comment 5

[Mark Banner (:standard8)]

Yeah, I think that's the right thing to do here.