1222308 - Drop frame IDs from mFrameIDsNotYetComposited when they get too old

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: 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).

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 3

[Nicolas Silva [:nical]]

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

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1092626. Add aInterrupted parameter to ReflowFinished. r=mats

Bug 1092626. Add aInterrupted parameter to ReflowFinished. r=mats

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: 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

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d907f1abdcf6f705e2554399218fe010a6b151a8
Bug 1222308. Assume frames that are very old will never be composited. r=nical

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d907f1abdcf6

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking since this is a recent regression (from 42)

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/41043b5b5b56

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is there a reason why you didn't uplift to beta?

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/41043b5b5b56

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8c199302644e

Comment 15

[Carsten Book [:Tomcat]]

(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

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No problem --- just wanted to make sure it wasn't waiting on something from me.