1756118 - ResizeObserver keeping the refresh driver ticking until the page is reloaded

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Florian Quèze [:florian]]

I don't have steps to reproduce, but I have a profile showing how this started. I reloaded the https://sql.telemetry.mozilla.org/dashboard/carbon-footprint-desktop-indicators page, and somehow vsync remained enabled permanently. I had the profiler running in marker-only mode, so I captured a profile: https://share.firefox.dev/3JCVKR8

Here's another profile of the same process with stack sampling: https://share.firefox.dev/3LUqC1N

This behavior stopped when I reloaded the tab, and I haven't found steps to reproduce.

Markus, any idea about how this could be made actionable?

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

ResizeObserver is more a layout thingie. -> layout.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

https://searchfox.org/mozilla-central/rev/68a5327697ec43aa55b458c504c4b313c9c80528/dom/base/ResizeObserverController.cpp#97,99,139 is at least suspicious. I couldn't immediately see how the RefreshDriver is stopped in that case

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1756118, stop observing RefreshDriver if there are no resize observers, r=dholbert

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56412ea7cf9c
stop observing RefreshDriver if there are no resize observers, r=dholbert

Comment 6

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/56412ea7cf9c

Comment 7

[Daniel Holbert [:dholbert]]

I've got a regression test here that seems to fail locally without the patch, using the API from bug 1756269.

(I'm currently rebuilding off of up-to-date mozilla-central to be sure it passes with the patch, and then will request review.)

Here's a Try build with the test and without the fix, to validate that it fails i.e. that it's sensitive enough to detect a regression:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c02d10dfa6816732e4341a0fd2b79ff6d44c4a49

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: Bug 1756118 followup: add a test to be sure refresh driver stops ticking when resize observer is unregistered. r?smaug

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2dbff9da48a
followup: add a test to be sure refresh driver stops ticking when resize observer is unregistered. r=smaug

Comment 10

[Marian-Vasile Laza]

Backed out 2 changesets (Bug 1756118, Bug 1756269) for causing linting failures.
Backout link
Push with failures
Failure Log

Comment 11

[Daniel Holbert [:dholbert]]

I fixed the linting failure & am re-landing, as noted in bug 1756269 comment 7.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9922ecce0e8c
followup: add a test to be sure refresh driver stops ticking when resize observer is unregistered. r=smaug

Comment 13

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/9922ecce0e8c