Comment 118 for bug 604635

Revision history for this message
In , Justin-lebar+bug (justin-lebar+bug) wrote :

>+ /**
>+ * Request a wakelock for specified resource.
>+ *
>+ * @param aTopic resource name
>+ */
>+ nsIDOMMozWakeLock requestWakeLock(in DOMString aTopic);

Please describe what requesting a wakelock does and what the return value is
good for.

What happens if I request a wake lock on an unrecognized topic? Does the
method throw an exception, return null, or return a useless lock object? (I
think we probably should throw, but I can see the reasoning behind returning a
useless lock.)

Jonas, what do you think?

Nit: We need to decide how "wakelock" is spelled in code and English. It looks
like you're doing "WakeLock" in code, so perhaps it should be "wake lock" in
English.

>diff --git a/dom/power/nsIDOMPowerManager.idl b/dom/power/nsIDOMPowerManager.idl
>--- a/dom/power/nsIDOMPowerManager.idl
>+++ b/dom/power/nsIDOMPowerManager.idl
>@@ -32,14 +32,19 @@
> * and other provisions required by the GPL or the LGPL. If you do not delete
> * the provisions above, a recipient may use your version of this file under
> * the terms of any one of the MPL, the GPL or the LGPL.
> *
> * ***** END LICENSE BLOCK ***** */
>
> #include "nsISupports.idl"
>
>-[scriptable, uuid(6ec16abc-2fe8-4ab3-99b0-0f08405be81b)]
>+interface nsIDOMMozWakeLockListener;
>+
>+[scriptable, uuid(abf4b2b1-139d-4eff-998d-8f24616910ae)]
> interface nsIDOMMozPowerManager : nsISupports
> {
>- void powerOff();
>- void reboot();
>+ void powerOff();
>+ void reboot();
>+ void addWakeLockListener(in nsIDOMMozWakeLockListener aListener);
>+ void removeWakeLockListener(in nsIDOMMozWakeLockListener aListener);
>+ boolean wakeLockIsHeld(in DOMString aTopic);
> };

This needs a comment explaining that it implements navigator.mozPower.

addWakeLockListener needs a comment explaining when the callback is fired.

>diff --git a/dom/power/nsIDOMWakeLock.idl b/dom/power/nsIDOMWakeLock.idl
>new file mode 100644
>--- /dev/null
>+++ b/dom/power/nsIDOMWakeLock.idl
>@@ -0,0 +1,14 @@
>+/* -*- 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 with this file,
>+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
>+
>+#include "nsISupports.idl"
>+
>+[scriptable, uuid(2e61eed1-5983-4562-8f26-fd361ab4a00d)]
>+interface nsIDOMMozWakeLock : nsISupports
>+{
>+ readonly attribute DOMString topic;
>+
>+ void unlock();

Need to document what happens if I call |unlock| more than once.

>diff --git a/dom/power/nsIDOMWakeLockListener.idl b/dom/power/nsIDOMWakeLockListener.idl
>new file mode 100644
>--- /dev/null
>+++ b/dom/power/nsIDOMWakeLockListener.idl
>@@ -0,0 +1,12 @@
>+/* -*- 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 with this file,
>+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
>+
>+#include "nsISupports.idl"
>+
>+[scriptable, function, uuid(4e258af8-cffb-47bc-b16d-e8241243426e)]
>+interface nsIDOMMozWakeLockListener : nsISupports
>+{
>+ void callback(in DOMString aType, in DOMString aTopic);
>+};

This needs documentation; in particular, where is this callback registered, and
what is aType?

>diff --git a/dom/power/nsIPowerManagerService.idl b/dom/power/nsIPowerManagerService.idl
>--- a/dom/power/nsIPowerManagerService.idl
>+++ b/dom/power/nsIPowerManagerService.idl
>-[scriptable, builtinclass, uuid(38919539-4641-4f0b-9f11-6b6294a9386f)]
>+interface nsIDOMMozWakeLock;
>+interface nsIDOMMozWakeLockListener;
>+
>+[scriptable, builtinclass, uuid(235ca1a1-d0c8-41f3-9b4a-dbaa4437d69c)]
> interface nsIPowerManagerService : nsISupports

This needs a comment explaining how it's different from nsIDOMMozPowerManager.

But reading through part 2, I'm thinking we may want to roll this whole
interface into hal.

Suppose we have a browser app running in a separate process from Gaia. The
browser requests a wake lock, and we want Gaia to be able to act upon it. As
is, that won't work, because the power manager service does not cross process
boundaries. Instead, the power manager service needs to run in the top-level
process and be queried by lower-level processes.

The infrastructure for doing this IPC is in hal, so calling into hal directly
from the relevant Navigator code seems like it may be the way to go.

(Btw, I don't mind if you post one big patch rather than three smaller ones;
whatever is easier for you.)