Closed Bug 1713547 Opened 6 months ago Closed 5 months ago

Handle zero display port scroll frame gracefully in the checkerboard telemetry

Categories

(Core :: Panning and Zooming, defect, P2)

defect

Tracking

()

RESOLVED FIXED
91 Branch
Fission Milestone M7a
Tracking Status
firefox91 --- fixed

People

(Reporter: hiro, Assigned: hiro)

References

Details

Attachments

(3 files)

During understanding how our checkerboard severity machinery works, I noticed if a frame hasn't set the display port we report it as a checkerboard severity record. The situation where a frame hasn't set the display port is, I believe, commonly happening with Fission where OOP iframes have overflow:hidden property in its root document.

Botond suggested to use the viewport instead of the display port if the display port hasn't set.

Assignee: nobody → hikezoe.birchill
Status: NEW → ASSIGNED
Summary: Handle zero display port scroll frame gracefully in the checkerborad telemetry → Handle zero display port scroll frame gracefully in the checkerboard telemetry
Fission Milestone: --- → M7a

Hiro, just want to be sure this is still on your radar -- looks like there's an r+ patch that might be ready-to-land modulo a review comment or two.

(This is on fission radar since it's tracked for the M7a milestone now.)

Flags: needinfo?(hikezoe.birchill)

I believe he wanted to land bug 1709460 first.

Yep, both bug 1709460 and this will affect checkerboard severity telemetry data, I (We) will want to see how these changes affect the telemetry results individually. I will land this a week later.

Flags: needinfo?(hikezoe.birchill)
Depends on: 1709460

I did track down where the empty displayport (base) comes from on my Linux box that I saw in bug 1713360 comment 9 both with/without Fission. (The empty displayport leads to one checkerboard severity record in the comment). The background why I tracked down is that I tried to write a browser mochitest for the case, and actually the test I originally wrote for this bug no longer fails without the patch for this bug since bug 1709460. because the test assumes that an OOP iframe doesn't have displayport base initially. (That said, I believe it's worth having the original test in our tree, because there is no other automated test to check checkerboard severity data)

The empty displayport happens for the first browser window (browser.xhtml) we show up when we launch a Firefox process, on a browser mochitest, it's browser-harness.xhtml. When the browser window gets initialized, we set displayport base for the browser window via ChromeProcessController::InitializeRoot, but unfortunately at the moment the document is still "about:blank" so that we set the displayport base to the root element for the "about:blank" instead of browser.xhtml (or browser-harness.xhtml).

Anyways I can observe the empty displayport only in the case of the first window, the reason is probably once after "browser.xhml" gets loaded it doesn't take long time to load it again for newly created windows, thus ChromeProcessController::InitializeRoot works as expected. Also the reason why we don't observe this empty displayport base on Windows is, I presume, on Windows we create a fake browser window (I don't recall the proper name), then we don't show the browser.xhtml window until the browser.xhml gets loaded. I have no theory why we don't observer the empty displayport on Mac, but probably there's some reasons.

Severity: -- → S3
Priority: -- → P2
Pushed by hikezoe.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0887c869133a
Use the viewport rect instead of the display port if the display port hasn't been set in the checkerboard severity check. r=botond
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch
Attached image A comparison on Linux

The change for this bug gave us interesting results you can see on GLAM.

Here is an image captured on GLAM, the left histogram is 2021-06-21 build (before landing the change), the right histogram is 2021-06-26.

Obviously there are three peaks on the left, but the right histogram has a single peak, it looks more like a Gaussian distribution, which is good I believe.

I am pretty sure the upper most peak on the left histogram is what I was seeing (comment 5) on my Linux box, but I have no idea about the lower most peak, it presumably implies there is another case where we don't set displayport properly.

At this moment I am totally unsure whether this result looks a regression on telemetry.mozilla.org since on TMO since it looks like the data aggregation on TMO has stopped since June 15. Anyways, even if the median value became greater than before, it's not a regression, it's definitely an improvement.

I am going to attach a Windows result.

Attached image A comparison on Windows

Here is the Windows result, as you can see on Windows there was a lower most peak too. I don't see the peak on results on Mac, so the case where we don't set displayport does happens both on Linux and Windows, but not on Mac.

Thanks. I think those are consistent with our current understanding, so that is good evidence that we understand this.

You need to log in before you can comment on or make changes to this bug.