Closed Bug 1756118 Opened 3 years ago Closed 3 years ago

ResizeObserver keeping the refresh driver ticking until the page is reloaded

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
99 Branch
Tracking Flags:
Tracking Status
firefox99 --- fixed

People

(Reporter: florian, Assigned: smaug)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

testcase
(deleted), text/html
Details
Bug 1756118, stop observing RefreshDriver if there are no resize observers, r=dholbert
(deleted), text/x-phabricator-request
Details
Bug 1756118 followup: add a test to be sure refresh driver stops ticking when resize observer is unregistered. r?smaug
(deleted), text/x-phabricator-request
Details

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?

Flags: needinfo?(mstange.moz)

ResizeObserver is more a layout thingie. -> layout.

Component: DOM: Core & HTML → Layout

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

Blocks: power-usage
Attached file testcase (deleted) —
Assignee: nobody → bugs
Status: NEW → ASSIGNED
No longer blocks: 1756269
Depends on: 1756269
Pushed by opettay@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/56412ea7cf9c stop observing RefreshDriver if there are no resize observers, r=dholbert

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

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox99: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 99 Branch

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

Flags: needinfo?(mstange.moz)
Pushed by dholbert@mozilla.com: 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

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

Flags: needinfo?(bugs)

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

Flags: needinfo?(bugs)
Pushed by dholbert@mozilla.com: 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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: