Closed
Bug 1383653
Opened 7 years ago
Closed 7 years ago
[Shutdown Decoder] video can only be suspended once.
Categories
(Core :: Audio/Video: Playback, defect)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: kaku, Assigned: kaku)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
Should be a regression of bug 1274919. STR. 1. go to about:config, make sure "media.suspend-bkgnd-video.enabled" is TRUE. set "media.suspend-bkgnd-video.delay-ms" to ZERO. - it can reduce waiting time because it forces suspending video immediately once the tab goes background. 2. install about:media. 3. open a new tab, going to Youtube and playing arbitrary video. 4. put the Youtube tab on background, switch to "about:media" tab, make sure that "Video Decoder:" field is "blank media data decoder". 5. hover on the Youtube tab and then click it to bring it back to foreground. 6. switch to "about:media" again. Expect. 7. The "Video Decoder:" field is "blank media data decoder". Actual. 7. The "Video Decoder:" field is NOT "blank media data decoder", which means that we don't suspend the video decoder.
Assignee | ||
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8889757 [details] Bug 1383653 P1 - add debug messages for understanding the status of suspending video decoder; https://reviewboard.mozilla.org/r/160832/#review166080 ::: dom/media/MediaDecoder.cpp:1270 (Diff revision 1) > void > MediaDecoder::UpdateVideoDecodeMode() > { > // The MDSM may yet be set. > if (!mDecoderStateMachine) { > + LOG("UpdateVideoDecodeMode(), early return because we don't have MDSM.\n"); Remove '\n' from the message.
Attachment #8889757 -
Flags: review?(jwwang) → review+
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8889758 [details] Bug 1383653 P3 - notify the MediaDecoder::BackgroundVideoDecodingPermissionObserver when a tab is selected; https://reviewboard.mozilla.org/r/160834/#review166746 ::: browser/base/content/tabbrowser.xml:7489 (Diff revision 1) > if (val) { > + > + // Set unselectedTabHover to false since this tab has been selected. > + if (this.linkedPanel && this.linkedBrowser) { > + this.linkedBrowser.unselectedTabHover(false); > + } I don't think this belongs in the _visuallySelected setter. updateCurrentBrowser is probably the right place to do this?
Attachment #8889758 -
Flags: review?(dao+bmo) → review-
Assignee | ||
Comment 5•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8889758 [details] Bug 1383653 P3 - notify the MediaDecoder::BackgroundVideoDecodingPermissionObserver when a tab is selected; https://reviewboard.mozilla.org/r/160834/#review166746 > I don't think this belongs in the _visuallySelected setter. updateCurrentBrowser is probably the right place to do this? I'm not familiar with the front-end code, but I had a discussion with :ralin and he think updateCurrentBrowser should be the right place. After going through the updateCurrentBrowser code, I think I should put the `this.linkedBrowser.unselectedTabHover(false);` logic here: http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/browser/base/content/tabbrowser.xml#1239-1283 right?
Comment 6•7 years ago
|
||
Here: http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/browser/base/content/tabbrowser.xml#1194-1209
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8889758 [details] Bug 1383653 P3 - notify the MediaDecoder::BackgroundVideoDecodingPermissionObserver when a tab is selected; https://reviewboard.mozilla.org/r/160834/#review167366 ::: browser/base/content/tabbrowser.xml:1212 (Diff revision 2) > this.mCurrentTab.removeAttribute("titlechanged"); > this.mCurrentTab.removeAttribute("attention"); > + > + // Call the current browser's unselectedTabHover() with false > + // because the tab has been selected. > + this.mCurrentBrowser.unselectedTabHover(false); Can you move the finishUnselectedTabHoverTimer call as well?
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•7 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b06dbca9530762785424a4d6b79487dfe854ab6b
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8891166 [details] Bug 1383653 P2 - call tabbrowser-tab::finishUnselectedTabHoverTimer() at tabbrowser::updateCurrentBrowser(); https://reviewboard.mozilla.org/r/162360/#review167842
Attachment #8891166 -
Flags: review?(dao+bmo) → review+
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8889758 [details] Bug 1383653 P3 - notify the MediaDecoder::BackgroundVideoDecodingPermissionObserver when a tab is selected; https://reviewboard.mozilla.org/r/160834/#review167844
Attachment #8889758 -
Flags: review?(dao+bmo) → review+
Assignee | ||
Comment 15•7 years ago
|
||
Thanks for the review!
Comment 16•7 years ago
|
||
Pushed by tkuo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6fbab9f755a8 P1 - add debug messages for understanding the status of suspending video decoder; r=jwwang https://hg.mozilla.org/integration/autoland/rev/de7f272d1f1e P2 - call tabbrowser-tab::finishUnselectedTabHoverTimer() at tabbrowser::updateCurrentBrowser(); r=dao https://hg.mozilla.org/integration/autoland/rev/8c470a91f0ff P3 - notify the MediaDecoder::BackgroundVideoDecodingPermissionObserver when a tab is selected; r=dao
Comment 17•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6fbab9f755a8 https://hg.mozilla.org/mozilla-central/rev/de7f272d1f1e https://hg.mozilla.org/mozilla-central/rev/8c470a91f0ff
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•