Drop frame IDs from mFrameIDsNotYetComposited when they get too old

RESOLVED FIXED in Firefox 43

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 affected, firefox43+ fixed, firefox44+ fixed, firefox45+ fixed, b2g-v2.5 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Randell found this. If you leave a video hidden for a long time, ImageContainer::mFrameIDsNotYetComposited grows and grows because it contains the IDs of all frames that were sent to the compositor but have not yet been composited. Normally NotifyCompositeInternal would detect that those frames have been skipped and clean up the array, but if the tab remains hidden then that code will never run. When it *does* run, it takes O(N^2) time, which can hang everything.
Bug 1222308. Assume frames that are very old will never be composited. r=nical

This also makes NotifyCompositeInternal take O(N) time in the length of
mFrameIDsNotYetComposited instead of O(N^2).
Attachment #8684014 - Flags: review?(nical.bugzilla)
STR: go to https://mozilla.github.com/webrtc-landing/pc_test.html, and Start.  Scroll it so the videos are off-screen.  Walk away for several hours.  come back, scroll up.
Attachment #8684014 - Flags: review?(nical.bugzilla) → review+
Comment on attachment 8684014 [details]
MozReview Request: Bug 1222308. Assume frames that are very old will never be composited. r=nical

https://reviewboard.mozilla.org/r/24473/#review22001
Bug 1092626. Add aInterrupted parameter to ReflowFinished. r=mats
Attachment #8684511 - Flags: review?(mats)
Bug 1092626. When a reflow is interrupted and a scroll position is clamped, try to restore to the old scroll position when the reflow completes. r=mats
Attachment #8684512 - Flags: review?(mats)
Attachment #8684511 - Attachment is obsolete: true
Attachment #8684511 - Flags: review?(mats)
Attachment #8684512 - Attachment is obsolete: true
Attachment #8684512 - Flags: review?(mats)
Assignee: nobody → roc
Comment on attachment 8684014 [details]
MozReview Request: Bug 1222308. Assume frames that are very old will never be composited. r=nical

Approval Request Comment
[Feature/regressing bug #]: 1143575
[User impact if declined]: OOM/hang if a video played for a long time while not visible
[Describe test coverage new/current, TreeHerder]: video playback code is pretty well tested, although playback of hidden videos is not
[Risks and why]: low risk; simple change with small scope
[String/UUID change made/needed]: none
Attachment #8684014 - Flags: approval-mozilla-beta?
Attachment #8684014 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/d907f1abdcf6
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
See Also: → 1222821
Tracking since this is a recent regression (from 42)
Comment on attachment 8684014 [details]
MozReview Request: Bug 1222308. Assume frames that are very old will never be composited. r=nical

Fix for a video playback hang, OK to uplift to aurora and beta.
Attachment #8684014 - Flags: approval-mozilla-beta?
Attachment #8684014 - Flags: approval-mozilla-beta+
Attachment #8684014 - Flags: approval-mozilla-aurora?
Attachment #8684014 - Flags: approval-mozilla-aurora+
Is there a reason why you didn't uplift to beta?
Flags: needinfo?(cbook)
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #12)
> Is there a reason why you didn't uplift to beta?

sorry was baking that patch back one day on aurora to make sure nothing brakes
Flags: needinfo?(cbook)
No problem --- just wanted to make sure it wasn't waiting on something from me.
See Also: → 1245409
You need to log in before you can comment on or make changes to this bug.