709585 - Add API to shut down and reboot device

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Something like navigator.reboot(), navigator.shutDown().  We want to use this to power off and restart gonk devices.

This is obviously a privileged API.

Comment 1

[Mounir Lamouri (:mounir)]

Writing the API should be only some DOM/HAL boilerplate. Do we already have the Gonk code to do that?

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Yes, this should be pretty straightforward.  To my knowledge, we don't have that code yet.

Comment 3

[Justin Lebar (not reading bugmail)]

Kan-Ru's API in bug 708964 includes shutdown/reboot.

Comment 4

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 1, Makefile boilerplate

Comment 5

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 2, PowerManager interface

Comment 6

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 3, Plug mozPower into navigator object

Comment 7

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 4, Dummy PowerManager DOM code

Comment 8

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 5, hal code for the Power API

Comment 9

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 6, Connect DOM and hal

Comment 10

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 6, Connect DOM and hal

--
Attachment #581185 [details] [diff] - Attachment is obsolete: true

Comment 11

[Mounir Lamouri (:mounir)]

Kan-Ru, I think you should ask a DOM peer [1] to review the changes inside dom/. :sicking or :smaug might be two good choices (Justin or I could help to give feedbacks but we are not peers).
Some people who know DOM APIs should probably have a look at the API. You should ask a super-review for that. Again, :sicking or :smaug might be appropriate but you should not ask the review and the super-review to the same person.

[1] https://wiki.mozilla.org/Modules/All#Document_Object_Model

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 581181 [details] [diff] [review]
Part 2, PowerManager interface


>+[scriptable, uuid(6ec16abc-2fe8-4ab3-99b0-0f08405be81b)]
>+interface nsIDOMMozPowerManager : nsISupports {
{ should be in the next line

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 581184 [details] [diff] [review]
Part 5, hal code for the Power API

>diff --git a/widget/src/gonk/nsWindow.h b/widget/src/gonk/nsWindow.h

>+extern nsIntRect gScreenBounds;
>+

This is so ugly.  Please file a followup to get rid of it.

>diff --git a/widget/src/xpwidgets/nsBaseWidget.h b/widget/src/xpwidgets/nsBaseWidget.h

>+  NS_IMETHOD_(void) SetInputContext(const InputContext& aContext,
>+                                    const InputContextAction& aAction) { return; }
>+  NS_IMETHOD_(InputContext) GetInputContext()
>+  {
>+    InputContext ctx;
>+    return ctx;
>+  }

As far as I can tell, this change is unnecessary.  Please remove it.

r=me with that fix.

Comment 14

[Michael Wu [:mwu]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #13)
> Comment on attachment 581184 [details] [diff] [review]
> Part 5, hal code for the Power API
> 
> >diff --git a/widget/src/gonk/nsWindow.h b/widget/src/gonk/nsWindow.h
> 
> >+extern nsIntRect gScreenBounds;
> >+
> 
> This is so ugly.  Please file a followup to get rid of it.
> 
> >diff --git a/widget/src/xpwidgets/nsBaseWidget.h b/widget/src/xpwidgets/nsBaseWidget.h
> 
> >+  NS_IMETHOD_(void) SetInputContext(const InputContext& aContext,
> >+                                    const InputContextAction& aAction) { return; }
> >+  NS_IMETHOD_(InputContext) GetInputContext()
> >+  {
> >+    InputContext ctx;
> >+    return ctx;
> >+  }
> 
> As far as I can tell, this change is unnecessary.  Please remove it.
> 
> r=me with that fix.

Uh, wrong patch?

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 581183 [details] [diff] [review]
Part 4, Dummy PowerManager DOM code

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

::: dom/base/Navigator.cpp
@@ +144,5 @@
>      mBatteryManager = nsnull;
>    }
>  
> +  if (mPowerManager) {
> +    mPowerManager->Shutdown();

Given that this function is empty, I'd say just nuke it and remove all this code.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 581197 [details] [diff] [review]
Part 6, Connect DOM and hal

Actually, you need to add security checks here. We don't want just any webpage to be able to shut off your device :)

Comment 17

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Jonas Sicking (:sicking) from comment #16)
> Comment on attachment 581197 [details] [diff] [review]
> Part 6, Connect DOM and hal
> 
> Actually, you need to add security checks here. We don't want just any
> webpage to be able to shut off your device :)

May I ask how to add security checks? I thought the mozPower object is already only accessible by privileged pages.

Comment 18

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Looking at part 4, it always returns a non-null object from GetMozPower, so there doesn't seem to be any security checks there either.

Comment 19

[Andreas Gal :gal]

You can use nsContentUtils::IsCallerChrome()

Comment 20

[Andreas Gal :gal]

Kan-Ru, can you please add a PowerManager service that calls HAL, and the DOM calls the service? This way extensions can override this, and more importantly we can call this from chrome JS code. Thanks!

Comment 21

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 2, PowerManager interface, v2

--
Attachment #581181 [details] [diff] - Attachment is obsolete: true

Comment 22

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 3, Plug mozPower into navigator object, v2

--
Attachment #581182 [details] [diff] - Attachment is obsolete: true

Comment 23

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 4, Dummy PowerManager DOM code, v2

--
Attachment #581183 [details] [diff] - Attachment is obsolete: true

Comment 24

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 6, Connect DOM and hal, v2

--
Attachment #581197 [details] [diff] - Attachment is obsolete: true

Comment 25

[Andreas Gal :gal]

What happened to the security checks?

Comment 26

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 5, hal code for the Power API, v2

--
Attachment #581184 [details] [diff] - Attachment is obsolete: true

Comment 27

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 2, PowerManager interface, v3

--
Attachment #584363 [details] [diff] - Attachment is obsolete: true

Comment 28

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 3, Plug mozPower into navigator object, v3

--
Attachment #584364 [details] [diff] - Attachment is obsolete: true

Comment 29

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 4, Dummy PowerManager DOM code, v3

--
Attachment #584365 [details] [diff] - Attachment is obsolete: true

Comment 30

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 6, Connect DOM and hal, v3

--
Attachment #584366 [details] [diff] - Attachment is obsolete: true

Comment 31

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 584374 [details] [diff] [review]
Part 5, hal code for the Power API, v2

>diff --git a/hal/gonk/GonkHal.cpp b/hal/gonk/GonkHal.cpp

>+#pragma GCC visibility push(default)
>+#include <unistd.h>
>+#include <sys/reboot.h>
>+#pragma GCC visibility pop
>+

You need to add sys/reboot.h to config/system-headers.  Not one of the
prettier parts of our build system, sorry :(.

>+void
>+Reboot()
>+{
>+  reboot(RB_AUTOBOOT);
>+}
>+
>+void
>+PowerOff()
>+{
>+  reboot(RB_POWER_OFF);
>+}
>+

This implementation is the same for gonk and linux.  Let's move it
into a linux/Power.cpp file and then build it for both gonk and
other-non-android-linux.

Looks good otherwise!  I would like to see a v2 patch with the changes
I requested.

Comment 32

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 5, hal code for the Power API, v2

--
Attachment #584374 [details] [diff] - Attachment is obsolete: true

Comment 33

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 584682 [details] [diff] [review]
Part 5, hal code for the Power API, v2

Thanks!

Comment 34

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Jonas, any ETA on review here?  Pulling the battery out of devices shut them down is getting tiresome :).

Comment 35

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 584376 [details] [diff] [review]
Part 3, Plug mozPower into navigator object, v3

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

Just add the mozPower property to nsIDOMNavigator directly. We've been way too interface-happy in the past which is something we're trying to stop.

r=me with that changed.

Comment 36

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 584377 [details] [diff] [review]
Part 4, Dummy PowerManager DOM code, v3

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

I'd just nuke the service thing here. It seems only useful for C++ callers, and they might as well call into hal directly.

r=me with PowerManagerService removed.

::: dom/power/PowerManager.cpp
@@ +55,5 @@
> +NS_IMPL_RELEASE(PowerManager)
> +
> +PowerManager::PowerManager()
> +{
> +}

Remove the ctor and dtor since they're empty anyway. That way they'll be inlined away.

Comment 37

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 584378 [details] [diff] [review]
Part 6, Connect DOM and hal, v3

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

I'd just nuke the service thing here. It seems only useful for C++ callers, and they might as well call into hal directly.

r=me with PowerManagerService removed.

Comment 38

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Jonas Sicking (:sicking) from comment #37)
> Comment on attachment 584378 [details] [diff] [review]
> Part 6, Connect DOM and hal, v3
> 
> Review of attachment 584378 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'd just nuke the service thing here. It seems only useful for C++ callers,
> and they might as well call into hal directly.
> 
> r=me with PowerManagerService removed.

PowerManagerService was added to address the issue said in comment 20. It can also be used to hold global wakelock information.

Comment 39

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Andreas: Is there a reason chrome code can't call navigator.mozPower.* ? Though that doesn't work from JS-components (since there is no navigator object) which might be an issue.

I'm ok with keeping the service if that's JS-components is something we care about.

Comment 40

[Andreas Gal :gal]

comment #39: there might be no content window around

Comment 41

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 1, Makefile boilerplate.

Comment 42

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 2, PowerManager interface, v3

Comment 43

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 3, Plug mozPower into navigator object, v3

Comment 44

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 4, Dummy PowerManager DOM code, v3

Comment 45

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 5, hal code for the Power API, v2

Comment 46

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 6, Connect DOM and hal, v3

Comment 47

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 3, Plug mozPower into navigator object, v3

Comment 48

[[:fabrice] Fabrice Desré]

Do we want to also add an "Airplane mode" to this API? It's more mobile-specific than just powerOff/reboot but we'll need it for b2g.

Comment 49

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I think "airplane mode" should be under control of the content power manager.  "Airplane mode" isn't clearly defined, and might include leaving wifi on (or reserving the ability to enable it later).  Reboot/shutdown are very clearly defined.

Comment 50

[Dão Gottwald [:dao]]

applying https://bug709585.bugzilla.mozilla.org/attachment.cgi?id=588338
patching file dom/Makefile.in
Hunk #1 FAILED at 66
1 out of 1 hunks FAILED -- saving rejects to file dom/Makefile.in.rej
patching file dom/dom-config.mk
Hunk #1 FAILED at 0
1 out of 1 hunks FAILED -- saving rejects to file dom/dom-config.mk.rej
abort: patch failed to apply

Comment 51

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 1, Makefile boilerplate.

Comment 52

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 2, PowerManager interface, v3

Comment 53

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 3, Plug mozPower into navigator object, v3

Comment 54

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 4, Dummy PowerManager DOM code, v3

Comment 55

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 5, hal code for the Power API, v2

Comment 56

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 6, Connect DOM and hal, v3

Comment 57

[Mozilla RelEng Bot]

Try run for 679052bd4045 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=679052bd4045
Results (out of 212 total builds):
    exception: 1
    success: 185
    warnings: 22
    failure: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-679052bd4045

Comment 58

[Scott Johnson (:jwir3)]

Pushed to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/561e1013aba0
https://hg.mozilla.org/integration/mozilla-inbound/rev/828713f89601
https://hg.mozilla.org/integration/mozilla-inbound/rev/648e5e5b9dd3
https://hg.mozilla.org/integration/mozilla-inbound/rev/656500fc5e79
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a4c4cb828a8
https://hg.mozilla.org/integration/mozilla-inbound/rev/10b455909e94

Comment 59

[Scott Johnson (:jwir3)]

We had to back these patches out because they caused the windows builds to break. 

https://hg.mozilla.org/integration/mozilla-inbound/rev/15ca5d162a81
https://hg.mozilla.org/integration/mozilla-inbound/rev/de33a82cee1f
https://hg.mozilla.org/integration/mozilla-inbound/rev/f64266a2463a
https://hg.mozilla.org/integration/mozilla-inbound/rev/c89909dee64f
https://hg.mozilla.org/integration/mozilla-inbound/rev/068243a96679
https://hg.mozilla.org/integration/mozilla-inbound/rev/543a3ea8cbc8

Comment 60

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Scott Johnson (:jwir3) from comment #59)
> We had to back these patches out because they caused the windows builds to
> break. 
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/15ca5d162a81
> https://hg.mozilla.org/integration/mozilla-inbound/rev/de33a82cee1f
> https://hg.mozilla.org/integration/mozilla-inbound/rev/f64266a2463a
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c89909dee64f
> https://hg.mozilla.org/integration/mozilla-inbound/rev/068243a96679
> https://hg.mozilla.org/integration/mozilla-inbound/rev/543a3ea8cbc8

Yeah, I found it from last try push. I will update shortly.

Comment 61

[Mozilla RelEng Bot]

Try run for a673abe29aba is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=a673abe29aba
Results (out of 62 total builds):
    exception: 1
    success: 56
    warnings: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-a673abe29aba

Comment 62

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Part 5, hal code for the Power API, v3

Added windows stub

Comment 63

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8c649c8e47ef
https://hg.mozilla.org/integration/mozilla-inbound/rev/be91138c1a84
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d097d1f741e
https://hg.mozilla.org/integration/mozilla-inbound/rev/52147d521bbc
https://hg.mozilla.org/integration/mozilla-inbound/rev/acc23575da69
https://hg.mozilla.org/integration/mozilla-inbound/rev/629978209c61

Comment 64

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8c649c8e47ef
https://hg.mozilla.org/mozilla-central/rev/be91138c1a84
https://hg.mozilla.org/mozilla-central/rev/0d097d1f741e
https://hg.mozilla.org/mozilla-central/rev/52147d521bbc
https://hg.mozilla.org/mozilla-central/rev/acc23575da69
https://hg.mozilla.org/mozilla-central/rev/629978209c61