Have default settings be energy star 5.0 compliant

Content ought to be able to keep the device's screen from turning off.

I don't think we should extend this privilege to all pages. Users coming from other platforms expect that the phone will manage the display, within most applications.

OTOH, it's kind of a bummer that a movie-watching app would need to explicitly ask for permission to keep the display on. If we made this facility available to all pages, it wouldn't need to ask.

We have [1]; I'm not sure what the status of this API is. But I also don't like this API. It should just be lock() and unlock() with no args.

We'll need a separate, more privileged API for Gaia itself to manage the current brightness.

[1] https://mxr.mozilla.org/mozilla-central/source/widget/public/nsIScreen.idl

I definitely agree we need this API. On phones it would prevent the phone from turning off, and on desktop it would prevent the screensaver from appearing.

For installed webapps I definitely think we could enable the API without any warnings etc.

Likewise I think it would be ok to do for pages that had gone into fullscreen mode.

For plain (non-fullscreen'ed) webpages I feel like we could almost do the same. We could do it if we at the same time enabled a mechanism which allowed the user turn off the disable-screensaver mode. Or we could simply show a permission doorhanger, that might be fine too :)

> For plain (non-fullscreen'ed) webpages I feel like we could almost do the same. We could do it if
> we at the same time enabled a mechanism which allowed the user turn off the disable-screensaver
> mode.

Could you rephrase this?

Oh, I think I understand. We can let normal pages disable screensaver without a prompt, so long as we give the user a way to re-enable the screensaver without leaving the page. How would we do that without being...annoying?

Also, Chris, can you give me an idea of this bug's priority relative to other current B2G work?

Being able to turn the display on/off and lock/unlock input is higher priority for B2G than API to allow content to prevent sleeping/screen-off. I don't 100% understand how the display on/off works in android yet, so that'd be the best place to start. I assume you want to do this in bug 681009?

> I assume you want to do this in bug 681009?

Yes. :)

As far as API goes here. I think something like this would work:

interface Navigator {

  Request disableScreenSaver(Boolean);

}

interface Request : EventTarget {
  Function onsuccess;
}

This makes for a clean API for most use cases where the page doesn't really care if the request succeeded or not. All you'd do is:

navigator.disableScreenSaver(true);
or
navigator.disableScreenSaver(false);

For pages which do want to display a warning to users where it's not able to disable the screensaver it can use the success event handler.

Note that for our standard doorhanger UI we can't tell when the user denies the request (since simply ignoring the doorhanger or clicking outside it isn't equivalent to "no", but rather "I'll decide later").

I don't know for sure that we'll ever end up using a doorhanger UI here. But with the above API we keep our options open.

Android has a nice (conceptual) solution for this, which is the notion of a "wake lock" that prevents the display from turning off (~screensaver). What's nice about this compared to a binary on/off API is that it composes well across multiple callers, and is extensible to locking other kinds of power-saving mechanisms (like low-power CPU idle). I would be in favor of an API like that, but I'm not sure what a good DOM-y solution would look like.

Can we do

  disableScreenSaver,
  enableScreenSaver

? Functions which take magic booleans are a pet peeve of mine. (I guess this boolean isn't so magic, but I don't like disableScreenSaver(false) is still unfortunate.)

I'm not sure we need a reentrant solution like the Android wake lock -- an author can easily code that up around the API if necessary, right?

I'm also not sure pages need to be able to warn the user if they can't disable the screensaver; do you have a use-case, Jonas? I'm afraid of "you must disable your screensaver in order to use this app" kind of BS.

(In reply to Justin Lebar [:jlebar] from comment #8)
> I'm not sure we need a reentrant solution like the Android wake lock -- an
> author can easily code that up around the API if necessary, right?
>

Not with nested iframes, etc, or if something like the status bar and a game are trying to change power-save state. Note that the "lock" API (or something like it) is also extensible, which is nice to have and reduces API surface area.

Created attachment 599965
Implement wake lock interface v2

Changed wake lock implementation to use hal directly. PowerManagerService now exports only minimum interfaces required to use wake lock. The leaks should be gone now..

Try run for 20e43fdd7711 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=20e43fdd7711
Results (out of 128 total builds):
    exception: 51
    success: 66
    warnings: 4
    failure: 7
Builds (or logs if builds failed) available at:
http://<email address hidden>

Created attachment 600050
Implement wake lock interface v2.1

Update test cases

Mounir might want to look at the Hal.cpp changes (I think they're fine).

Try run for 14cc0af010a8 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=14cc0af010a8
Results (out of 279 total builds):
    exception: 3
    success: 225
    warnings: 27
    failure: 24
Builds (or logs if builds failed) available at:
http://<email address hidden>

Comment on attachment 600050
Implement wake lock interface v2.1

>@@ -941,24 +946,45 @@ Navigator::GetMozBattery(nsIDOMMozBatter
>
> return NS_OK;
> }
>
> NS_IMETHODIMP
> Navigator::GetMozPower(nsIDOMMozPowerManager** aPower)
> {
> if (!mPowerManager) {
>+ *aPower = nsnull;

It's convention to do *aPower = nsnull unconditionally, just so someone doesn't
have to scan the code to see that it's correct.

>+ * Request a wake lock for a resource.

This comment looks great; thanks.

>+nsRefPtr<PowerManagerService> PowerManagerService::sSingleton;

Please mark this as static.

> /* static */ already_AddRefed<nsIPowerManagerService>
> PowerManagerService::GetInstance()
> {
>- nsCOMPtr<nsIPowerManagerService> pmService;
>+ if (!sSingleton) {
>+ sSingleton = new PowerManagerService();
>+ sSingleton->Init();
>+ ClearOnShutdown(&sSingleton);
>+ }
>
>- pmService = new PowerManagerService();
>+ NS_ADDREF(sSingleton);
>+ return sSingleton.get();

This works, but raw addref/release is scary, so we prefer

  nsCOMPtr<nsIPowerManagerService> service = sSingleton;
  return service.forget();

>+void
>+PowerManagerService::ComputeWakeLockState(const hal::WakeLockInformation& aWakeLockInfo,
>+ nsAString &aState)
>+{
>+ if (aWakeLockInfo.locks() == 0) {
>+ aState.AssignLiteral("notheld");
>+ } else if (aWakeLockInfo.locks() == aWakeLockInfo.hidden()) {
>+ aState.AssignLiteral("background");
>+ } else {
>+ aState.AssignLiteral("foreground");
>+ }

Maybe "unlocked/locked-background/locked-foreground" would be clearer?

>+void
>+PowerManagerService::Notify(const hal::WakeLockInformation& aWakeLockInfo)
>+{
>+ nsAutoString state;
>+ ComputeWakeLockState(aWakeLockInfo, state);
>+
>+ nsTArray<nsCOMPtr<nsIDOMMozWakeLockListener> > listeners(mListeners);

Please add a comment explaining why we're copying this list, and why copying is OK (we expect no more than one listener per window, so it shouldn't be too long).

>+NS_IMETHODIMP
>+PowerManagerService::NewWakeLock(const nsAString &aTopic,
>+ nsIDOMWindow *aWindow,
>+ nsIDOMMozWakeLock **aWakeLock)
>+{
>+ nsRefPtr<WakeLock> wakelock = new WakeLock();
>+ nsresult rv = wakelock->Init(aTopic, aWindow);
>+ NS_ENSURE_SUCCESS(rv, rv);
>+
>+ NS_ADDREF(*aWakeLock = wakelock);

Out of general fear of raw addref/release, how about

  *aWakeLock = wakelock.forget()

>diff --git a/dom/power/PowerManagerService.h b/dom/power/PowerManagerService.h
>--- a/dom/power/PowerManagerService.h
>+++ b/dom/power/PowerManagerService.h
> class PowerManagerService
> : public nsIPowerManagerService
>+ , public WakeLockObserver
> {
> public:
> NS_DECL_ISUPPORTS
> NS_DECL_NSIPOWERMANAGERSERVICE
>
> static already_AddRefed<nsIPowerManagerService> GetInstance();
>+
>+ void Init();
>+
>+ void Notify(const hal::WakeLockInformation& aWakeLockInfo);

NotifyWakeLockState, NotifyWakeLockStateChange, or something?

>+ void ComputeWakeLockState(const hal::WakeLockInformation& aWakeLockInfo,
>+ nsAString &aState);

Does this need to be public?

>+
>+private:
>+
>+ ~PowerM...

> Would you mind changing these bools to
>
> enum WakeLockState {
> WAKE_LOCK_STATE_UNLOCKED,
> WAKE_LOCK_STATE_HIDDEN,
> WAKE_LOCK_STATE_VISIBLE
> };

Actually, this is kind of confusing, because UNLOCKED is not a valid state for either function. Maybe we should just go with ModifyWakeLock everywhere. Or you can just have HIDDEN and VISIBLE.

Interdiff v2 --> v2.1

>--- b/browser/installer/package-manifest.in
>+++ a/browser/installer/package-manifest.in
>@@ -156,17 +156,16 @@
> @BINPATH@/components/dom_events.xpt
> @BINPATH@/components/dom_geolocation.xpt
> @BINPATH@/components/dom_network.xpt
> @BINPATH@/components/dom_notification.xpt
> @BINPATH@/components/dom_html.xpt
> @BINPATH@/components/dom_indexeddb.xpt
> @BINPATH@/components/dom_offline.xpt
> @BINPATH@/components/dom_json.xpt
>-@BINPATH@/components/dom_power.xpt
> @BINPATH@/components/dom_range.xpt
> @BINPATH@/components/dom_sidebar.xpt
> @BINPATH@/components/dom_sms.xpt
> @BINPATH@/components/dom_storage.xpt
> @BINPATH@/components/dom_stylesheets.xpt
> @BINPATH@/components/dom_traversal.xpt
> @BINPATH@/components/dom_xbl.xpt
> @BINPATH@/components/dom_xpath.xpt

This was checked in, right?

diff --git a/dom/power/test/power_wakelock_child.html b/dom/power/test/power_wakelock_child.html
new file mode 100644
--- /dev/null
+++ b/dom/power/test/power_wakelock_child.html
@@ -0,0 +1,11 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <title>Test for Power API</title>
+ <script type="application/javascript">
+ lock = navigator.requestWakeLock("test");
+ </script>
+</head>
+<body>
+</body>
+</html>
diff --git a/dom/power/test/test_power_basics.html b/dom/power/test/test_power_basics.html
--- a/dom/power/test/test_power_basics.html
+++ b/dom/power/test/test_power_basics.html
@@ -9,14 +9,102 @@
 <p id="display"></p>
 <div id="content" style="display: none">
 </div>
 <pre id="test">
 <script type="application/javascript">

 /** Test for Power API **/

+function checkWakeLock() {
+ var count = 0;
+ var callback = function wakeLockListener(topic, state) {
+ is(topic, "test", "WakeLockListener should receive the lock topic");
+ switch (state) {
+ case "foreground":
+ count += 1;
+ break;
+ case "background":
+ count += 1;
+ break;
+ case "notheld":
+ count -= 1;
+ break;
+ }
+ }
+ navigator.mozPower.addWakeLockListener(callback);
+
+ var lock1, lock2;
+ ok(lock1 = navigator.requestWakeLock("test"), "navigator.requestWakeLock should return a wakelock");
+ ok(lock2 = navigator.requestWakeLock("test"), "navigator.requestWakeLock should return a wakelock");
+ setTimeout(function() {
+ is(count, 1, "WakeLockListener should only fire once");

+ checkProperty(lock1, "topic");
+ checkProperty(lock2, "topic");

We don't need these tests; if lock1.topic doesn't exist, then reading it below will throw an exception and break the test.

+ is(lock1.topic, "test", "wakelock should remember the locked topic");
+ is(lock2.topic, "test", "wakelock should remember the locked topic");
+ ok(navigator.mozPower.getWakeLockState("test") == "foreground", "mozPower should remember the locked topic");
+ lock1.unlock();
+ lock2.unlock();

I'd like a test that when you unlock one of two held locks, we don't get a topic unlock notification.

+ setTimeout(function() {
+ ok(navigator.mozPower.getWakeLockState("test") == "notheld", "all locks should have been unlocked");
+ is(count, 0, "WakeLockListener should only fire once");
+ navigator.mozPower.removeWakeLockListener(callback);
+ ok(lock1 = navigator.requestWakeLock("test"), "navigator.requestWakeLock should return a wakelock");
+ setTimeout(function() {
+ isnot(count, 1, "navigator.mozPower.removeWakeLockListener should have removed the listener");
+ lock1.unlock();
+ checkWakeLockNavigation();
+ }, 1000);
+ }, 1000);
+ }, 1000);
+}
+
+function checkWakeLockNavigation() {
+ childWindow = window.open("power_wakelock_child.html");
+ SimpleTest.waitForFocus(function () {

Need to wait for the child window's onload event to fire.

+ ok(navigator.mozPower.getWakeLockState("test") == "foreground", "lock should be held by child window");
+ childWindow.location = "about:blank";
+ // XXX how to wait page transition?

Add an onload listener and you'll hear it.

+ setTimeout(fun...

Try run for b82684542fa7 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b82684542fa7
Results (out of 12 total builds):
    success: 11
    failure: 1
Builds (or logs if builds failed) available at:
http://<email address hidden>

Created attachment 602836
Implement wake lock interface v3

Update tests and fixed some counting bugs. Pushed to try.

Unfortunately interdiff bails on these two patches (presumably because they're not based off the same hg revision?), so I can't tell what changed.

Would you mind trying to get an interdiff? What I'd try is make v2.1 and v3 apply to the same base revision of m-c. Then the interdiff command should be able to give meaningful output.

Comment on attachment 602836
Implement wake lock interface v3

Clearing the review flag until we can figure out the interdiff situation.

Try run for 5eb01c6a9f25 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5eb01c6a9f25
Results (out of 228 total builds):
    exception: 2
    success: 192
    warnings: 31
    failure: 3
Builds (or logs if builds failed) available at:
http://<email address hidden>
 Timed out after 06 hours without completing.

Created attachment 603136
v2.1 and v3 interdiff

Thanks a lot for the interdiff.

>@@ -92,8 +99,13 @@
> {
> nsAutoString state;
> ComputeWakeLockState(aWakeLockInfo, state);
>-
>- nsTArray<nsCOMPtr<nsIDOMMozWakeLockListener> > listeners(mListeners);
>+
>+ /**
>+ * Copy the listeners list before we walk through the callbacks
>+ * because the callbacks may install new listeners. We expect no
>+ * more than one listener per window, so it shouldn't be too long.
>+ */
>+ nsTArray<nsCOMPtr<nsIDOMMozWakeLockListener> > listeners(mWakeLockListeners);

Nit: Use nsAutoTArray here.

>diff -u b/hal/Hal.cpp b/hal/Hal.cpp
>--- b/hal/Hal.cpp
>+++ b/hal/Hal.cpp
>@@ -184,10 +184,11 @@
> if (mObservers->Length() == 0) {
> DisableNotifications();
>
>+ // XXX CachingObserversManager invalidate it's cache here, but why?
>+ OnNotificationsDisabled();

AIUI caching observers invalidate their cache when notifications are disabled because as soon as they stop listening e.g. to the battery state, their cached battery state is invalid.

Does that answer your question here? We should get rid of the XXX before we check in.

>- * Increase the wake lock count of aTopic
>- * @param aTopic the resource name
>- * @param aHidden the visibility of the wake lock
>+ * Adjust the internal wake lock counts.
>+ * @param aTopic lock topic
>+ * @param aLockAdjust to increase or decrease active locks
>+ * @param aHiddenAdjust to increase or decrease hidden locks
> */
>-void AcquireWakeLock(const nsAString &aTopic, bool aHidden);
>+void ModifyWakeLock(const nsAString &aTopic, int aLockAdjust, int aHiddenAdjust);

We generally shun raw types (e.g. "int") in our code, since "int" can mean different things on different platforms.

Since you're passing WakeLockControl objects, can you just use that type here and elsewhere? (You may need to define the WakeLockControl enum in the ipdl.) If someone needs to call ModifyWakeLock(aTopic, 2), we can revisit this. You can then add the enum value directly to count.numLocks and count.numHidden, bypassing the whole enum to int conversion altogether.

>--- /dev/null
>+++ b/dom/power/test/browser_bug697132.js
>@@ -0,0 +1,197 @@
>+"use strict";
>+
>+waitForExplicitFinish();
>+
>+let pageSource1 = "data:text/html,1";
>+let pageSource2 = "data:text/html,2";
>+
>+let win, win1, win2;
>+let tab, tab1, tab2;
>+let lock, lock1, lock2;
>+let gCurStepIndex = -1;
>+let gSteps = [
>+ function basicWakeLock() {
>+ tab = gBrowser.addTab(pageSource1);
>+ win = gBrowser.getBrowserForTab(tab).contentWindow;
>+ let browser = gBrowser.getBrowserForTab(tab);
>+
>+ browser.addEventListener("load", function onLoad(e) {
>+ browser.removeEventListener("load", onLoad, true);
>+ let nav = win.navigator;
>+ let power = nav.mozPower;
>+ lock = nav.requestWakeLock("test");
>+
>+ ok(lock != null,
>+ "navigator.requestWakeLock should return a wake lock");
>+ is(lock.topic, "test",
>+ "wake lock should remember the locked topic");
>+ isnot(power.getWakeLockState("test"), "unlocked",
>+ "topic is locked");
>+
>+ lock.unlock();
>+
>+ is(lock.topic, "test",
>+ "wake lock should remember the l...

Comment on attachment 602836
Implement wake lock interface v3

lgtm!

(In reply to Justin Lebar [:jlebar] from comment #130)
> Thanks a lot for the interdiff.
>
> >@@ -92,8 +99,13 @@
> > {
> > nsAutoString state;
> > ComputeWakeLockState(aWakeLockInfo, state);
> >-
> >- nsTArray<nsCOMPtr<nsIDOMMozWakeLockListener> > listeners(mListeners);
> >+
> >+ /**
> >+ * Copy the listeners list before we walk through the callbacks
> >+ * because the callbacks may install new listeners. We expect no
> >+ * more than one listener per window, so it shouldn't be too long.
> >+ */
> >+ nsTArray<nsCOMPtr<nsIDOMMozWakeLockListener> > listeners(mWakeLockListeners);
>
> Nit: Use nsAutoTArray here.

How much do you think the nsAutoTArray should be? I think nsAutoTArray<2> is enough.

> >diff -u b/hal/Hal.cpp b/hal/Hal.cpp
> >--- b/hal/Hal.cpp
> >+++ b/hal/Hal.cpp
> >@@ -184,10 +184,11 @@
> > if (mObservers->Length() == 0) {
> > DisableNotifications();
> >
> >+ // XXX CachingObserversManager invalidate it's cache here, but why?
> >+ OnNotificationsDisabled();
>
> AIUI caching observers invalidate their cache when notifications are
> disabled because as soon as they stop listening e.g. to the battery state,
> their cached battery state is invalid.

I see. The cache is not updated anymore when notifications are disabled.

But it only caches the first call to GetCurrentInformation. I suspect the subsequent calls will get old value here.

> >+function test() {
> >+ // data url inherits its parent's principal, which is |about:| here.
> >+ Services.prefs.setCharPref("dom.power.whitelist", "about:");
> >+ runNextStep();
> >+}
>
> Is this necessary? We're chrome here, so the data URLs should have a chrome
> principal, I'd expect.

Yes, because I use the API from the tab content so it is necessary. The test script itself does not have associated document.

> In any case, please reset the pref to its original value when you're done.
>
> This test looks great, but are you confident that you don't need a test
> where you go back and forward in a background tab? Up to you.

Hum.. not even considered. I will add one.

> >+void
> >+ModifyWakeLock(const nsAString &aTopic, int aLockAdjust, int aHiddenAdjust)
> >+{
> >+ if (!sInitialized) {
> >+ Init();
> >+ }
> >+
> >+ LockCount count;
> >+ count.numLocks = 0;
> >+ count.numHidden = 0;
> >+ sLockTable->Get(aTopic, &count);
> >+ MOZ_ASSERT(count.numLocks >= count.numHidden);
> >+ MOZ_ASSERT(aLockAdjust >= 0 || count.numLocks > 0);
> >+ MOZ_ASSERT(aHiddenAdjust >= 0 || count.numHidden > 0);
> >+
> >+ WakeLockState oldState = ComputeWakeLockState(count.numLocks, count.numHidden);
> >+
> >+ count.numLocks += aLockAdjust;
> >+ count.numHidden += aHiddenAdjust;
> >+ MOZ_ASSERT(count.numLocks >= count.numHidden);
> >+
> >+ sLockTable->Put(aTopic, count);
>
> If count.numLocks goes to 0, should we remove the wake lock from the table?

Yes.. that make sense.

> >--- /dev/null
> >+++ b/hal/HalWakeLock.h
> >@@ -0,0 +1,32 @@
> >+/* -*- Mode: C++; tab-width: 40; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> >+/* This Source Code Form is subject to the terms of the Mozilla Public
> >+ * License, v. 2.0. If a copy of the MPL was not distributed...

> How much do you think the nsAutoTArray should be? I think nsAutoTArray<2> is enough.

Yes, that's fine.

> I see. The cache is not updated anymore when notifications are disabled.
>
> But it only caches the first call to GetCurrentInformation. I suspect the subsequent calls will get
> old value here.

That's what the CacheInformation() call is for. I might be misunderstanding, but I think CacheInformation is called whenever the underlying value changes, so long as notifications are enabled.

But maybe there is a bug, which is that if we call GetCurrentInformation and notifications are disabled, we'll set mHasValidCache to true, which doesn't seem right. If notifications are disabled, the cache is invalid the moment GetCurrentInformationInternal returns.

If you think that's right, would you mind filing a bug and cc'ing mounir?

> One is for compute WakeLockState and one is for compute the output string. I think we might want
> the numbers in WakeLockInformation in the future so I didn't pass the WakeLockState directly to
> the listeners.

Oh, I see. The numeric overload is used in PowerManagerService. Cool, it just needs that comment filled in. :)

Created attachment 603628
602836: Implement wake lock interface v3.1

Fixed the addressed issues and add new test case for background tab. Also converted two static_cast to NS_ISUPPORTS_CAST macro.

http://hg.mozilla.org/integration/mozilla-inbound/rev/72bc6f12d1cc

Comment on attachment 603628
602836: Implement wake lock interface v3.1

Review of attachment 603628:
-----------------------------------------------------------------

::: dom/base/Navigator.cpp
@@ +974,4 @@
> }
>
> + nsCOMPtr<nsIDOMMozPowerManager> power =
> + do_QueryInterface(NS_ISUPPORTS_CAST(nsIDOMMozPowerManager*, mPowerManager));

What kind of nonsense is this?

nsCOMPtr<nsIDOMMozPowerManager> power = mPowerManager;

will do.

::: dom/power/PowerManagerService.cpp
@@ +163,5 @@
> + nsresult rv = wakelock->Init(aTopic, aWindow);
> + NS_ENSURE_SUCCESS(rv, rv);
> +
> + nsCOMPtr<nsIDOMMozWakeLock> wl =
> + do_QueryInterface(NS_ISUPPORTS_CAST(nsIDOMMozWakeLock*, wakelock));

As it would here.

https://hg.mozilla.org/mozilla-central/rev/72bc6f12d1cc

Leaving open for unresolved comment 136 nits.

Created attachment 604291
Use nsCOMPtr copy constructor

Comment on attachment 604291
Use nsCOMPtr copy constructor

Use assignment, as in

nsCOMPtr<nsIDOMMozPowerManager> power = mPowerManager;

r=me with that

(In reply to Ms2ger from comment #139)
> Comment on attachment 604291
> Use nsCOMPtr copy constructor
>
> Use assignment, as in
>
> nsCOMPtr<nsIDOMMozPowerManager> power = mPowerManager;
>
> r=me with that

OK, I lied. It's not copy constructor but a normal constructor from a raw pointer.

I cannot assign mPowerManager to a nsCOMPtr directly because it's a nsRefPtr.

Do you want me to use

  nsCOMPtr<nsIDOMMozPowerManager> power = mPowerManager.get();

instead? It looks uglier then the normal constructor.

Created attachment 604826
Use do_QueryObject

I cannot use assignment directly as the assignee is a RefPtr object not a raw pointer nor COMPtr. This version use do_QueryObject to wrap it.

Comment on attachment 604826
Use do_QueryObject

No, that's as inefficient. Just use the constructor, then.

Created attachment 604868
Use nsCOMPtr constructor.

https://hg.mozilla.org/integration/mozilla-inbound/rev/25af1ffbabce

https://hg.mozilla.org/mozilla-central/rev/25af1ffbabce

assinged to me for sec action to schedule a meeting

:jlebar any chance we can get a review of this scheduled and done?

Sure, I was confused by comment 146, which I took to mean that I didn't need to do anything here.

But at this point, I am not convinced there's actually something to review at the moment. We have an API in this bug which doesn't do anything. Site can request wake locks, but they do nothing.

I think it would be more appropriate to do a security review when we expose a way for the user to give permission to sites to use this API. I suspect that any security review we do before then will focus on this hypothetical API anyway.

SecReview is 2012.05.04: More info here
https://<email address hidden>/Security%20Review.html?view=month&action=view&invId=110490-110489&pstat=AC&exInvId=110490-171831&useInstance=1&instStartTime=1336150800000&instDuration=3600000

Release Notes on firefox 14.0.1 say there is a api for prevent the display from sleeping could someone please tell me what the api (i was not able to find a documented api)
It would be nice to be able to stop a screensaver from coming on when playing video

(In reply to pqwoerituytrueiwoq from comment #150)
> Release Notes on firefox 14.0.1 say there is a api for prevent the display
> from sleeping could someone please tell me what the api (i was not able to
> find a documented api)
> It would be nice to be able to stop a screensaver from coming on when
> playing video

The release notes are wrong. The API exists, but it's not implemented in desktop Firefox.

Ok, thanks
Guess I won't be making that Greasemonkey script...

(In reply to Justin Lebar (not reading bugmail) from comment #151)
> The release notes are wrong. The API exists, but it's not implemented in
> desktop Firefox.

And would it be possible to link to the description of this API? Is it documented somewhere on MDN? I am interested in the implementation on FxOS/FxAndroid so lack of implementation on Desktop doesn’t bother me.

(In reply to Matěj Cepl from comment #153)
> (In reply to Justin Lebar (not reading bugmail) from comment #151)
> > The release notes are wrong. The API exists, but it's not implemented in
> > desktop Firefox.
>
> And would it be possible to link to the description of this API? Is it
> documented somewhere on MDN? I am interested in the implementation on
> FxOS/FxAndroid so lack of implementation on Desktop doesn’t bother me.

https://developer.mozilla.org/en-US/docs/Web/API/Wake_Lock_API