Closed Bug 1383610 Opened 7 years ago Closed 7 years ago

[ShutdownDecoder] Turn off preference on FF55 desktop version.

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox55 blocking fixed
firefox56 --- fixed

People

(Reporter: kaku, Assigned: kaku)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

According to PM and EM's decision, let's turn off the feature on FF55 desktop version and keep the Feneec ON.
Assignee: nobody → kaku
Status: NEW → ASSIGNED
Comment on attachment 8889270 [details]
Bug 1383610 - turn off preference of ShutdownDecoder feature on desktop version release and beta channel but keep the Fennec on;

https://reviewboard.mozilla.org/r/160328/#review166114

Per bug 1380584 comment 4.
Attachment #8889270 - Flags: review?(bwu) → review+
Thanks!
Pushed by tkuo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c48bb95530b1
turn off preference on desktop version release channel but keep the Fennec on; r=bwu
https://hg.mozilla.org/mozilla-central/rev/c48bb95530b1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
This is a very unfortunate decision... How is that for desktop?

as far as I can tell, mobile PC/Mac device will now be affected.

Why this change when it will have serious consequence on battery life of affected device?

We should back this out IMHO
Status: RESOLVED → REOPENED
Flags: needinfo?(bwu)
Resolution: FIXED → ---
Flags: needinfo?(ajones)
(In reply to Jean-Yves Avenard [:jya] from comment #6)
> as far as I can tell, mobile PC/Mac device will now be affected.

In this context desktop means Windows, Mac, Linux, etc.
Flags: needinfo?(ajones)
(In reply to Jean-Yves Avenard [:jya] from comment #6)
> This is a very unfortunate decision... How is that for desktop?
> 
> as far as I can tell, mobile PC/Mac device will now be affected.
> 
> Why this change when it will have serious consequence on battery life of
> affected device?
> 
> We should back this out IMHO
I love this feature and really like to back out or re-enable it soon. Per the discussions in the mail, we just need to see some shield studies to make sure if there is any thing we should be aware of. Besides, we have a great enhancement on bug 1274919 on the way.
Flags: needinfo?(bwu)
Blocks: 1380584
Blake and Jean-Yves. Please help to work on this. This is the dependence of starting the shield study. Many thanks.
Flags: needinfo?(jyavenard)
Flags: needinfo?(bwu)
Bobby, 
We already landed the patch to turn the pref off in comment 5.
Flags: needinfo?(jyavenard)
Flags: needinfo?(bwu)
Please make sure your commit message specific enough so it will make sense to others. Thanks.
Kaku and Blake, 

As Julien's response in intent-to-ship mail, please help to make sure this change apply to mozilla-beta. So we could ride to release. Thanks.
Flags: needinfo?(kaku)
Flags: needinfo?(bwu)
FWIW the patch looks wrong to me, AFAIK we don't have a RELEASE macro defined anywhere.  You probably want RELEASE_OR_BETA instead.
(In reply to Julien Cristau [:jcristau] from comment #13)
> FWIW the patch looks wrong to me, AFAIK we don't have a RELEASE macro
> defined anywhere.  You probably want RELEASE_OR_BETA instead.

Neither do I know where is the definition of RELEASE, but looks like I am not the only occurrence.
http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/modules/libpref/init/all.js#5741-5751

I will confirm the usage is correct or not and uplift/modify accordingly.
Flags: needinfo?(kaku)
Flags: needinfo?(bwu)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 6fd0bb2d8bec -d ac1b19c47aac: rebasing 409710:6fd0bb2d8bec "Bug 1383610 - turn off preference of ShutdownDecoder feature on desktop version release and beta channel but keep the Fennec on; r=bwu" (tip)
merging modules/libpref/init/all.js
warning: conflicts while merging modules/libpref/init/all.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by tkuo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c95cc38d6065
turn off preference of ShutdownDecoder feature on desktop version release and beta channel but keep the Fennec on; r=bwu
Approval Request Comment
[Feature/Bug causing the regression]: bug 1383610
[User impact if declined]: we'll release the ShutdownDecoder feature which is not ready to be released.
[Is this code covered by automated tests?]: this patch only turn off the preference, nothing to do with the feature's code. (The feature is well tested.)
[Has the fix been verified in Nightly?]: we want to keep the preference ON at Nightly.
[Needs manual test from QE? If yes, steps to reproduce]: NO.
[List of other uplifts needed for the feature/fix]: NO.
[Is the change risky?]: NO.
[Why is the change risky/not risky?]: It's not because we're turning off the feature.
[String changes made/needed]: NO.
Attachment #8890766 - Flags: review?(bwu)
Attachment #8890766 - Flags: approval-mozilla-beta?
Attachment #8890766 - Flags: review?(bwu) → review+
Comment on attachment 8890766 [details] [diff] [review]
bug-1383610-beta-uplift.patch

pref off "shutdown decoder" feature on desktop for 55 release, beta55+
Attachment #8890766 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/mozilla-central/rev/c95cc38d6065
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
(In reply to Tzuhao Kuo [:kaku] (PTO 7/26) from comment #19)
> [Is this code covered by automated tests?]: this patch only turn off the
> preference, nothing to do with the feature's code. (The feature is well
> tested.)
> [Has the fix been verified in Nightly?]: we want to keep the preference ON
> at Nightly.
> [Needs manual test from QE? If yes, steps to reproduce]: NO.

Setting qe-verify- based on Tzuhao Kuo's assessment on manual testing needs.
Flags: qe-verify-
FWIW I checked that media.suspend-bkgnd-video.enabled is false in 55.0b13 (linux64).
You need to log in before you can comment on or make changes to this bug.