The default bug view has changed. See this FAQ.

Add API to shut down and reboot device

RESOLVED FIXED in mozilla13

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: cjones, Assigned: kanru)

Tracking

(Depends on: 1 bug)

Trunk
mozilla13
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 25 obsolete attachments)

5.49 KB, patch
Details | Diff | Splinter Review
5.10 KB, patch
Details | Diff | Splinter Review
7.47 KB, patch
Details | Diff | Splinter Review
20.73 KB, patch
Details | Diff | Splinter Review
3.48 KB, patch
Details | Diff | Splinter Review
7.93 KB, patch
Details | Diff | Splinter Review
Something like navigator.reboot(), navigator.shutDown().  We want to use this to power off and restart gonk devices.

This is obviously a privileged API.
Version: unspecified → Trunk
Writing the API should be only some DOM/HAL boilerplate. Do we already have the Gonk code to do that?
Yes, this should be pretty straightforward.  To my knowledge, we don't have that code yet.
Kan-Ru's API in bug 708964 includes shutdown/reboot.
(Assignee)

Updated

5 years ago
Blocks: 708964
(Assignee)

Comment 4

5 years ago
Created attachment 581180 [details] [diff] [review]
Part 1, Makefile boilerplate
(Assignee)

Comment 5

5 years ago
Created attachment 581181 [details] [diff] [review]
Part 2, PowerManager interface
(Assignee)

Comment 6

5 years ago
Created attachment 581182 [details] [diff] [review]
Part 3, Plug mozPower into navigator object
(Assignee)

Comment 7

5 years ago
Created attachment 581183 [details] [diff] [review]
Part 4, Dummy PowerManager DOM code
(Assignee)

Comment 8

5 years ago
Created attachment 581184 [details] [diff] [review]
Part 5, hal code for the Power API
(Assignee)

Comment 9

5 years ago
Created attachment 581185 [details] [diff] [review]
Part 6, Connect DOM and hal
(Assignee)

Comment 10

5 years ago
Created attachment 581197 [details] [diff] [review]
Part 6, Connect DOM and hal

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

Updated

5 years ago
Attachment #581185 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #581180 - Flags: review?(jones.chris.g)
(Assignee)

Updated

5 years ago
Attachment #581181 - Flags: review?(jones.chris.g)
(Assignee)

Updated

5 years ago
Attachment #581182 - Flags: review?(jones.chris.g)
(Assignee)

Updated

5 years ago
Attachment #581183 - Flags: review?(jones.chris.g)
(Assignee)

Updated

5 years ago
Attachment #581184 - Flags: review?(jones.chris.g)
(Assignee)

Updated

5 years ago
Attachment #581197 - Flags: review?(jones.chris.g)
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
Assignee: nobody → kchen
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Attachment #581180 - Flags: superreview?
Attachment #581180 - Flags: review?(jones.chris.g)
Attachment #581180 - Flags: review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #581181 - Flags: review?(jones.chris.g) → review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #581181 - Flags: superreview?(bugs)
(Assignee)

Updated

5 years ago
Attachment #581182 - Flags: review?(jones.chris.g) → review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #581183 - Flags: review?(jones.chris.g) → review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #581197 - Flags: review?(jones.chris.g) → review?(jonas)
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
Attachment #581181 - Flags: superreview?(bugs) → superreview+
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.
Attachment #581184 - Flags: review?(jones.chris.g) → review+

Comment 14

5 years ago
(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?
Attachment #581180 - Flags: review?(jonas) → review+
Attachment #581181 - Flags: review?(jonas) → review+
Attachment #581182 - Flags: review?(jonas) → review+
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.
Attachment #581183 - Flags: review?(jonas) → review-
Attachment #581197 - Flags: review?(jonas) → review+
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 :)
Attachment #581197 - Flags: review+ → review-
(Assignee)

Comment 17

5 years ago
(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.
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

5 years ago
You can use nsContentUtils::IsCallerChrome()

Comment 20

5 years ago
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!
(Assignee)

Comment 21

5 years ago
Created attachment 584363 [details] [diff] [review]
Part 2, PowerManager interface, v2

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

Comment 22

5 years ago
Created attachment 584364 [details] [diff] [review]
Part 3, Plug mozPower into navigator object, v2

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

Comment 23

5 years ago
Created attachment 584365 [details] [diff] [review]
Part 4, Dummy PowerManager DOM code, v2

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

Comment 24

5 years ago
Created attachment 584366 [details] [diff] [review]
Part 6, Connect DOM and hal, v2

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

Updated

5 years ago
Attachment #581181 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #581182 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #581183 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #581197 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #581184 - Flags: review+ → review?(jones.chris.g)
(Assignee)

Updated

5 years ago
Attachment #584363 - Flags: review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #584364 - Flags: review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #584365 - Flags: review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #584366 - Flags: review?(jonas)

Comment 25

5 years ago
What happened to the security checks?
(Assignee)

Comment 26

5 years ago
Created attachment 584374 [details] [diff] [review]
Part 5, hal code for the Power API, v2

--
Attachment #581184 [details] [diff] - Attachment is obsolete: true
Attachment #584374 - Flags: review?(jones.chris.g)
(Assignee)

Comment 27

5 years ago
Created attachment 584375 [details] [diff] [review]
Part 2, PowerManager interface, v3

--
Attachment #584363 [details] [diff] - Attachment is obsolete: true
Attachment #584375 - Flags: review?(jonas)
(Assignee)

Comment 28

5 years ago
Created attachment 584376 [details] [diff] [review]
Part 3, Plug mozPower into navigator object, v3

--
Attachment #584364 [details] [diff] - Attachment is obsolete: true
Attachment #584376 - Flags: review?(jonas)
(Assignee)

Comment 29

5 years ago
Created attachment 584377 [details] [diff] [review]
Part 4, Dummy PowerManager DOM code, v3

--
Attachment #584365 [details] [diff] - Attachment is obsolete: true
Attachment #584377 - Flags: review?(jonas)
(Assignee)

Comment 30

5 years ago
Created attachment 584378 [details] [diff] [review]
Part 6, Connect DOM and hal, v3

--
Attachment #584366 [details] [diff] - Attachment is obsolete: true
Attachment #584378 - Flags: review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #581184 - Attachment is obsolete: true
Attachment #581184 - Flags: review?(jones.chris.g)
(Assignee)

Updated

5 years ago
Attachment #584363 - Attachment is obsolete: true
Attachment #584363 - Flags: review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #584364 - Attachment is obsolete: true
Attachment #584364 - Flags: review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #584365 - Attachment is obsolete: true
Attachment #584365 - Flags: review?(jonas)
(Assignee)

Updated

5 years ago
Attachment #584366 - Attachment is obsolete: true
Attachment #584366 - Flags: review?(jonas)
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.
Attachment #584374 - Flags: review?(jones.chris.g)
(Assignee)

Comment 32

5 years ago
Created attachment 584682 [details] [diff] [review]
Part 5, hal code for the Power API, v2

--
Attachment #584374 [details] [diff] - Attachment is obsolete: true
Attachment #584682 - Flags: review?(jones.chris.g)
(Assignee)

Updated

5 years ago
Attachment #584374 - Attachment is obsolete: true
Comment on attachment 584682 [details] [diff] [review]
Part 5, hal code for the Power API, v2

Thanks!
Attachment #584682 - Flags: review?(jones.chris.g) → review+
(Assignee)

Updated

5 years ago
Blocks: 697132
Jonas, any ETA on review here?  Pulling the battery out of devices shut them down is getting tiresome :).
Attachment #584375 - Flags: review?(jonas) → review+
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.
Attachment #584376 - Flags: review?(jonas) → review+
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.
Attachment #584377 - Flags: review?(jonas) → review+
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.
Attachment #584378 - Flags: review?(jonas) → review+
(Assignee)

Comment 38

5 years ago
(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.
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.

Updated

5 years ago
Attachment #581180 - Flags: superreview? → superreview+

Comment 40

5 years ago
comment #39: there might be no content window around
(Assignee)

Comment 41

5 years ago
Created attachment 588338 [details] [diff] [review]
Part 1, Makefile boilerplate.
Attachment #581180 - Attachment is obsolete: true
(Assignee)

Comment 42

5 years ago
Created attachment 588339 [details] [diff] [review]
Part 2, PowerManager interface, v3
Attachment #584375 - Attachment is obsolete: true
(Assignee)

Comment 43

5 years ago
Created attachment 588340 [details] [diff] [review]
Part 3, Plug mozPower into navigator object, v3
Attachment #584376 - Attachment is obsolete: true
(Assignee)

Comment 44

5 years ago
Created attachment 588341 [details] [diff] [review]
Part 4, Dummy PowerManager DOM code, v3
Attachment #584377 - Attachment is obsolete: true
(Assignee)

Comment 45

5 years ago
Created attachment 588342 [details] [diff] [review]
Part 5, hal code for the Power API, v2
Attachment #584682 - Attachment is obsolete: true
(Assignee)

Comment 46

5 years ago
Created attachment 588343 [details] [diff] [review]
Part 6, Connect DOM and hal, v3
Attachment #584378 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Comment 47

5 years ago
Created attachment 588353 [details] [diff] [review]
Part 3, Plug mozPower into navigator object, v3
Attachment #588340 - Attachment is obsolete: true
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.
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.
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
Keywords: checkin-needed
(Assignee)

Comment 51

5 years ago
Created attachment 590108 [details] [diff] [review]
Part 1, Makefile boilerplate.
Attachment #588338 - Attachment is obsolete: true
(Assignee)

Comment 52

5 years ago
Created attachment 590109 [details] [diff] [review]
Part 2, PowerManager interface, v3
Attachment #588339 - Attachment is obsolete: true
(Assignee)

Comment 53

5 years ago
Created attachment 590110 [details] [diff] [review]
Part 3, Plug mozPower into navigator object, v3
Attachment #588353 - Attachment is obsolete: true
(Assignee)

Comment 54

5 years ago
Created attachment 590111 [details] [diff] [review]
Part 4, Dummy PowerManager DOM code, v3
Attachment #588341 - Attachment is obsolete: true
(Assignee)

Comment 55

5 years ago
Created attachment 590112 [details] [diff] [review]
Part 5, hal code for the Power API, v2
Attachment #588342 - Attachment is obsolete: true
(Assignee)

Comment 56

5 years ago
Created attachment 590113 [details] [diff] [review]
Part 6, Connect DOM and hal, v3
Attachment #588343 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed

Comment 57

5 years ago
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/kchen@mozilla.com-679052bd4045
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
Keywords: checkin-needed

Updated

5 years ago
Target Milestone: --- → mozilla12
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
Target Milestone: mozilla12 → ---
(Assignee)

Comment 60

5 years ago
(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

5 years ago
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/kchen@mozilla.com-a673abe29aba
(Assignee)

Comment 62

5 years ago
Created attachment 593019 [details] [diff] [review]
Part 5, hal code for the Power API, v3

Added windows stub
Attachment #590112 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
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
Keywords: checkin-needed
Target Milestone: --- → mozilla13
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
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Depends on: 729767

Updated

4 years ago
Depends on: 819795

Updated

3 years ago
Depends on: 963366
You need to log in before you can comment on or make changes to this bug.