Drop frame IDs from mFrameIDsNotYetComposited when they get too old

RESOLVED FIXED in Firefox 43, Firefox OS v2.5

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
mozilla45
Points:
---

Firefox Tracking Flags

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

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

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.
Created attachment 8684014 [details]
MozReview Request: Bug 1222308. Assume frames that are very old will never be composited. r=nical

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.

Updated

3 years ago
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
Created attachment 8684511 [details]
MozReview Request: Bug 1092626. Add aInterrupted parameter to ReflowFinished. r=mats

Bug 1092626. Add aInterrupted parameter to ReflowFinished. r=mats
Attachment #8684511 - Flags: review?(mats)
Created attachment 8684512 [details]
MozReview Request: 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

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?

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d907f1abdcf6
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
See Also: → bug 1222821
Tracking since this is a recent regression (from 42)
status-firefox42: --- → affected
status-firefox43: --- → affected
status-firefox44: --- → affected
tracking-firefox43: --- → +
tracking-firefox44: --- → +
tracking-firefox45: --- → +
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+

Comment 11

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/41043b5b5b56
status-firefox44: affected → fixed
Is there a reason why you didn't uplift to beta?
Flags: needinfo?(cbook)

Comment 14

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/8c199302644e
status-firefox43: affected → fixed
(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: → bug 1245409
You need to log in before you can comment on or make changes to this bug.