952835 - screen goes to sleep while video still playing in Metro Firefox

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: General, defect, P2)

Description

[Karen Rudnitski [:kar]]

on my surface pro 2, nightly (12-20 build still).

Go to this website: http://www.doyogawithme.com/content/mamachi-flow-prenatal-yoga-class-morning-flow and the screen goes to sleep even with the video still playing. Video pauses, and resumes once the screen is woken up again.

Expected behaviour: if I'm watching a video, screen stays on.

Comment 1

[Matt Brubeck (:mbrubeck)]

Documentation for Windows Store apps:
http://msdn.microsoft.com/en-us/library/windows/apps/jj152725.aspx

Do we have logic to prevent screensaver/sleep during video playback on other platforms?

Comment 2

[Matt Brubeck (:mbrubeck)]

> Do we have logic to prevent screensaver/sleep during video playback on other
> platforms?

We have code to do this on Android (bug 739542), and open bugs for other platforms (bug 517870, bug 772347).

Comment 3

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: Feedback v1

Code is working fine. I'm not sure it's in the right place though. Also, the power manager service throws when closing the browser, due to ref count. It's destroyed before MetroAppShell gets destroyed and we remove the wakelock listener. Don't know where it should be. 

Code is hooking up a WakeListener to power management service like Android is doing at [1] and [2].
Callback states for nsIDOMWakeLockListener are documented at [3]. Android's locking logic is at [4].

[1] - http://mxr.mozilla.org/mozilla-central/source/widget/android/nsAppShell.cpp#113
[2] - http://mxr.mozilla.org/mozilla-central/source/widget/android/nsAppShell.cpp#188
[3] - http://mxr.mozilla.org/mozilla-central/source/dom/power/nsIDOMWakeLockListener.idl#9
[4] - http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java#2617

Comment 4

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: Patch v1

Fixed the ref count issue after IRC chat with Jimm.

Comment 5

[Jim Mathies [:jimm]]

Comment on attachment 8360713 [details] [diff] [review]
Patch v1

Review of attachment 8360713 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/winrt/MetroAppShell.cpp
@@ +54,5 @@
> +public:
> +  NS_DECL_ISUPPORTS;
> +
> +private:
> +  ComPtr<ABI::Windows::System::Display::IDisplayRequest> displayRequest;

nit - mDisplayRequest

@@ +56,5 @@
> +
> +private:
> +  ComPtr<ABI::Windows::System::Display::IDisplayRequest> displayRequest;
> +  
> +  NS_IMETHOD Callback(const nsAString& aTopic, const nsAString& aState) {

nit - white space above here

Comment 6

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: Patch v2

Patch with nits addressed.

Comment 7

[Rodrigo Silveira [:rsilveira] [:rodms]]

https://hg.mozilla.org/integration/fx-team/rev/f67a5c0ff459

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e5e71feaf49c

Comment 10

[Manuela Muntean [Away]]

For iteration #22, verified as fixed with latest Nightly and Aurora on Win 8.1 64-bit.