I'm thinking we can have bool attribute navigator.mozScreen.enabled I'd also want to put screen brightness in here. I guess if it's just brightness and enabled/disabled, we could put them straight on the navigator object, but maybe there are other things we want to expose? (We could, for example, expose details about the screen, such as its subpixel layout.) We don't have a permissions API yet, but once we do, we probably want to hide mozScreen entirely; unprivileged pages can use the visibility API to tell whether they're visible.

This needs to tie into the RIL backend too, as there's a RIL_REQUEST_SCREEN_STATE variable we need to update when the screen is turned on/off.

What does that do in rild?

Mostly it's a power saving mechanism. From the ril.h header: Indicates the current state of the screen. When the screen is off, the RIL should notify the baseband to suppress certain notifications (eg, signal strength and changes in LAC/CID or BID/SID/NID/latitude/longitude) in an effort to conserve power. These notifications should resume when the screen is on.

But that doesn't apply to when the screen is turned off based on the proximity sensor while a call is in progress, right? We'll probably need some hackery dackery doo to know when to notify rild that the screen is really for srs off.

I haven't actually tested to see whether that happens or not, but I wouldn't be surprised if it still does. We don't care about the listed changes when on a call with the screen off, I don't think it'll effect the call status.

I have a lot of code written for this, but I'm currently putting out some fires. I'll get back to this in a few days.

This adds bool attribute window.screen.mozEnabled double attribute window.screen.mozBrightness and hooks screen.mozEnabled up to the power button. We don't currently fire the correct visibility change events when we turn off the screen, but I figure we can do that in a follow-up. I tested screen.mozBrightness by setting it to random values before turning on the screen, but I've taken that out in the final patch.

Comment on attachment 575294 [details] [diff] [review] Patch v1 Review of attachment 575294 [details] [diff] [review]: ----------------------------------------------------------------- I don't think I have enough background to review changes in those files: b2g/chrome/content/shell.js hal/gonk/GonkHal.cpp widget/src/gonk/nsAppShell.cpp r=me for the other files (with the comments addressed). ::: dom/base/nsScreen.cpp @@ +54,5 @@ > + > +/* static */ void > +nsScreen::Initialize() > +{ > + sInitialized = true; Add a NS_ASSERTION checking that sInitialized isn't true. @@ +58,5 @@ > + sInitialized = true; > + Preferences::AddBoolVarCache(&sAllowScreenEnabledProperty, > + "dom.screenEnabledProperty.enabled"); > + Preferences::AddBoolVarCache(&sAllowScreenBrightnessProperty, > + "dom.screenBrightnessProperty.enabled"); Is it really useful to have a VarCache? We could just check the pref everytime. I don't know the costs of using VarCache but if there is one, it seems unworthy. @@ +261,5 @@ > +nsresult > +nsScreen::GetMozEnabled(bool *aEnabled) > +{ > + NS_ENSURE_TRUE(sAllowScreenEnabledProperty, NS_ERROR_NOT_AVAILABLE); > + NS_ENSURE_ARG_POINTER(aEnabled); I think if sAllowScreen... isn't true, we could just return NS_OK and a default value. Throwing an exception seems a bad idea to me. Also, you don't need NS_ENSURE_ARG_POINTER. @@ +269,5 @@ > + > +nsresult > +nsScreen::SetMozEnabled(bool aEnabled) > +{ > + NS_ENSURE_TRUE(sAllowScreenEnabledProperty, NS_ERROR_NOT_AVAILABLE); Ditto for the exception. @@ +272,5 @@ > +{ > + NS_ENSURE_TRUE(sAllowScreenEnabledProperty, NS_ERROR_NOT_AVAILABLE); > + > + // TODO: When the screen's state changes, all visible windows should fire a > + // visibility change event. File a bug and add the bug number before landing please :) @@ +282,5 @@ > +nsresult > +nsScreen::GetMozBrightness(double *aBrightness) > +{ > + NS_ENSURE_TRUE(sAllowScreenBrightnessProperty, NS_ERROR_NOT_AVAILABLE); > + NS_ENSURE_ARG_POINTER(aBrightness); Ditto for the exception and the NS_ENSURE_ARG_POINTER. @@ +290,5 @@ > + > +nsresult > +nsScreen::SetMozBrightness(double aBrightness) > +{ > + NS_ENSURE_TRUE(sAllowScreenBrightnessProperty, NS_ERROR_NOT_AVAILABLE); Ditto for the exception. ::: hal/Hal.cpp @@ +405,5 @@ > + > + if (brightness < 0) > + brightness = 0; > + if (brightness > 1) > + brightness = 1; You could use |clamped| from Algorithm.h. That would make this more readable and you wouldn't violate the coding style ;)

Comment on attachment 575294 [details] [diff] [review] Patch v1 Chris, would you mind looking at the remaining files?

> Is it really useful to have a VarCache? We could just check the pref everytime. I really have no idea when it's appropriate to use a cache or not, so I've been using caches everywhere. I have similar code in the navigator for whether the vibrator is enabled. Anyone have guidance on when we should prefer caches versus going to the pref service? I suspect it doesn't matter much either way. > I think if sAllowScreen... isn't true, we could just return NS_OK and a default value. Throwing an > exception seems a bad idea to me. I'm concerned about lying to the page if we don't have to. A page might, for example, try to determine whether Firefox is in the background by examining |screen.enabled && !document.mozVisible|. If we don't throw an exception, do we just drop writes to the properties? The page does screen.enabled = false, but the next time we read screen.enabled, it's true? I guess throwing an exception is also lame. I'd rather the properties just not be there if we don't support them.

Comment on attachment 575294 [details] [diff] [review] Patch v1 Jonas, what do you think we should do about the API when turning the screen on/off and modifying brightness isn't available?

Is the plan to allow any page to modify the on-state and brightness? That seems a bit scary to me. Especially since there is no provision for resetting these modifications if the user wants the screen back (like pressing the escape key, or touching the screen on a touch device). I could easily see this leading to dataloss if the user thinks that the device has crashed and does a force-reboot. Or is the idea that this will only be available to privileged apps?

> Or is the idea that this will only be available to privileged apps? Yes; I should have made that clear. I don't think either property should be available to anything except the most privileged app. We don't have the privilege API done yet, though.

Ok, please call into the privilege manager, or add a pref which controls which sites are able to use this feature. Other than that it looks ok.

Comment on attachment 575294 [details] [diff] [review] Patch v1 >diff --git a/b2g/app/mobile.js b/b2g/app/mobile.js > // deep within the bowels of the widgetry system. Remove me when GL > // compositing isn't default disabled in widget/src/android. > pref("layers.acceleration.force-enabled", true); >+ >+pref("dom.screenEnabledProperty.enabled", true); >+pref("dom.screenBrightnessProperty.enabled", true); Nit: People usually add a small comment line to separate block of preferences. For this I guess '// Screen WebAPI' should be enough. >diff --git a/b2g/chrome/content/shell.js b/b2g/chrome/content/shell.js > if (e.keyCode == e.DOM_VK_HOME) { > shell.doCommand("cmd_close"); > } >+ else if (e.keyCode == e.DOM_VK_SLEEP) { >+ screen.mozEnabled = !screen.mozEnabled; >+ } > } > }; Nit: } else if {

> Ok, please call into the privilege manager Does this code exist in the tree atm? I can't find it, if so. > or add a pref which controls which sites are able to use this feature. Other than that it looks ok. Is a pref disabling it everywhere except B2G sufficient?

(In reply to Justin Lebar [:jlebar] from comment #16) > > or add a pref which controls which sites are able to use this feature. Other than that it looks ok. > > Is a pref disabling it everywhere except B2G sufficient? There is an implementation for something else than Gonk?

> There is an implementation for something else than Gonk? No. So the API is twice-disabled outside gonk; once by a pref, and the other by there being no code. :)

Then, except if we want to implement a system for B2G we could just not handle that for the moment in my opinion. Given that B2G can hardly run non-pre-installed content, it seems to be quite safe to have it available. We could also add a whitelist preference like I did for WebSMS.

Comment on attachment 575294 [details] [diff] [review] Patch v1 >diff --git a/b2g/app/mobile.js b/b2g/app/mobile.js >+pref("dom.screenEnabledProperty.enabled", true); >+pref("dom.screenBrightnessProperty.enabled", true); I assume there's another mechanism preventing general web content from seeing this API? If not, we need that :). >diff --git a/hal/Hal.cpp b/hal/Hal.cpp >+#define PROXY_IF_SANDBOXED_RV(_call) \ >+ do { \ >+ if (InSandbox()) { \ >+ return hal_sandbox::_call; \ >+ } else { \ >+ return hal_impl::_call; \ >+ } \ >+ } while (0) >+ Yuck :). My kingdom for gcc expression-statements ... Anywho, please rename this to RETURN_PROXY_IF_SANDBOXED() so the effect on control flow is a bit clearer. >+void SetScreenBrightness(double brightness) >+{ >+ AssertMainThread(); >+ >+ if (brightness < 0) >+ brightness = 0; >+ if (brightness > 1) else if >diff --git a/hal/gonk/GonkHal.cpp b/hal/gonk/GonkHal.cpp >+class AutoFd See ScopedClose in xpcom/glue/FileUtils.h >+const char *screenEnabledFilename = "/sys/power/state"; >+const char *screenBrightnessFilename = "/sys/class/backlight/pwm-backlight/brightness"; >+ >+template<ssize_t n> size_t >+// We can write to screenEnabledFilename to enable/disable the screen, but when >+// we read, we always get "mem"! So we have to keep track ourselves whether >+// the screen is on or not. Is that what android does? >+double >+GetScreenBrightness() >+{ >+ char buf[32]; >+ ReadFromFile(screenBrightnessFilename, buf); >+ >+ unsigned int val; >+ if (sscanf(buf, "%u", &val) != 1) { strtoul() please. sscanf() is massive overkill. Yeah doesn't really matter here, but good habit to be in ... >+ return val / 256.0; Shouldn't this be / 255.0? >+} >+ >+void >+SetScreenBrightness(double brightness) >+{ >+ // Don't use De Morgan's law to push the ! into this expression; we want to >+ // catch NaN too. >+ if (!(brightness >= 0 && brightness <= 1)) { Nit: I like the inequalities to point the same direction for easier scanning, so 0 <= brightness && brightness <= 1 >+ // Convert the value in [0, 1] to an int between 0 and 255, then write to a >+ // string. >+ int val = static_cast<int>(round(brightness * 255)); No need to round, the float->int conversion will do that incidentally. >+ char str[4]; >+ if (snprintf(str, sizeof(str), "%d", val) > static_cast<int>(sizeof(str))) { This should be an assertion. char str[4]; DebugOnly<int> bytesWritten = snprintf(str, sizeof(str), "%d", val); MOZ_ASSERT(bytesWritten < sizeof(str)); This is just nit-y stuff, r=me with those fixed.

> I assume there's another mechanism preventing general web content from > seeing this API? If not, we need that :). You mean unprivileged pages shouldn't be aware that screen.mozEnabled exists? We don't have a mechanism for this that I'm aware of... >> +template<ssize_t n> > size_t This is ssize_t because we compare it with a signed number later on: > ssize_t numRead = read(...); > + buf[PR_MIN(numRead, n - 1)] = '\0'; > No need to round, the float->int conversion will do that incidentally. Really? http://codepad.org/SHSVR262 int main() { float f = 1.9; printf("%d\n", static_cast<int>(f)); } outputs 1. > >+// We can write to screenEnabledFilename to enable/disable the screen, but when > >+// we read, we always get "mem"! So we have to keep track ourselves whether > >+// the screen is on or not. > Is that what android does? I didn't find where Android keeps track of whether the screen is on or off, but as far as I can tell, Android never reads from this file.

(In reply to Justin Lebar [:jlebar] from comment #21) > > I assume there's another mechanism preventing general web content from > > seeing this API? If not, we need that :). > > You mean unprivileged pages shouldn't be aware that screen.mozEnabled > exists? We don't have a mechanism for this that I'm aware of... > My understanding is that we make the sms and telephony APIs disappear when there's no available backend. I don't know how that's done. What happens if an unprivileged page frobs the screen brightness, e.g.? You have an explicit security check? > > No need to round, the float->int conversion will do that incidentally. > > Really? http://codepad.org/SHSVR262 > > int main() { > float f = 1.9; > printf("%d\n", static_cast<int>(f)); > } > > outputs 1. > That's rounding for you ;). But yeah you're right, best to keep that extra bit of precision. > > >+// We can write to screenEnabledFilename to enable/disable the screen, but when > > >+// we read, we always get "mem"! So we have to keep track ourselves whether > > >+// the screen is on or not. > > > Is that what android does? > > I didn't find where Android keeps track of whether the screen is on or off, > but as far as I can tell, Android never reads from this file. I would be interested to know what android does. I'm not against tracking that state in gecko a priori, but if something else already tracks it, closer to the HW, I would prefer to use that. We can do that in a followup though.

> What happens if an unprivileged page frobs the screen brightness, e.g.? You have an explicit > security check? Yes, that's how it works right now. > I would be interested to know what android does. I just had another look. PowerManagerService.java keeps track of mPowerState itself. It calls into the Power JNI wrapper class to turn off the screen; that file doesn't provide any way to read the screen state. So it looks like Android does basically what we're doing in this patch.

Sounds good. Thanks for checking.

BTW, on the b2g call today, there were some questions about this API raised by mwu. As far as I understood, his points were - do away with enabled/disabled in favor of brightness==0 -> disabled. I think the counter-objection is that it's convenient to turn off the display while having the backend remember the preferred brightness, so that the "screen-off" code doesn't have to remember that itself. - there might already something exposed on |window| on which this new interface could have been stuck. The idea of making the screen brightness a CSS property was also mooted, so that it could be animated by CSS animations etc., but I'm not sure if there's any precedent for this kind of CSS property.

I'm sorry I wasn't on the call to talk about this. I'm wary of overloading an existing DOM property to control the screen's brightness or on/off state, for three reasons: 1. New DOM properties are cheap compared to the cost of fishy API. 2. If we overload an existing property, then content has to detect which of the two overloads it's calling. 3. window.screen already has relevant properties like the screen's height and width, and we'll probably add others, such as the screen's subpixel layout. It feels like a natural place for the knobs to adjust the screen's properties. But I defer to Jonas and the WebAPI folks on this issue. There may well be a nook or cranny that this API fits well into. I don't think we should make backlight == 0 mean "turn the screen off". Again, three points: a. On the SGII, we can't dim the backlight smoothly down to black. hw-backlight==255 is very bright, and hw-backlight==0 is dim but most definitely not "no backlight". So if in the DOM backlight == 0 means "screen off", you'd be able to dim the screen smoothly from very bright to kind of dim, and then there'd be a discontinuity when the screen finally turned off at backlight == 0. b. Even if this weren't an issue (on traditional rather than organic LEDs, you can certainly turn off the backlight while leaving the screen on), there's a big difference between "backlight off" and "screen off". In the former case, the screen still receives touch events. It's perfectly reasonable to ask for the backlight to be off with the screen still on, so you can "wake" the phone by touching it. c. Last, the backlight responds very quickly when frobbed, but turning the screen on and off has a visible lag. So the screen will respond very quickly as you move from backlight == 1 to backlight == 0 + epsilon, but then there will be about a second's delay before the screen goes to black. We could hack this in Hal (black the screen immediately, but pretend that it's off) but that hack doesn't translate to non-OLED screens, where a black screen isn't the same as backlight off.

I like the idea of making screen brightness a CSS property a lot. The samsung galaxy s2 firmware turns down the screen brightness before turning it off, so making screen brightness a css property and animating it would fit that case perfectly. On the other hand, it is in fact weird. It would only affect the window where as most of the css properties I know also affect the children, and children overriding them usually have a meaning. Combining brightness and enabled/disabled is another option though I don't feel particularly strong one way or another on combining or keeping separate.

As for borrowing existing dom properties, one option I was looking at was: https://developer.mozilla.org/en/DOM/window.minimize https://developer.mozilla.org/en/DOM/window.restore since in B2G, the window is the screen, and trying to hide your screen should mean we stop displaying things/turn off the screen. I don't remember anything in the dom or css that would make sense for brightness though.

> I like the idea of making screen brightness a CSS property a lot. It'd be cute to use a CSS animation here, but CSS is all about applying your effects to just some elements on the page. The backlight is a global property. It's not hard to animate the backlight using JS. > As for borrowing existing dom properties, one option I was looking at was: What happens when content (say, a web app) calls window.minimize? Does it want to turn off the screen, or does it want to minimize itself and return to the home screen? Using window.minimize/restore for task switching seems much more reasonable... Unless you'd want window.minimize to do something different for apps and the top-level window? This feels like a hack to me.

(In reply to Justin Lebar [:jlebar] from comment #29) > > I like the idea of making screen brightness a CSS property a lot. > > It'd be cute to use a CSS animation here, but CSS is all about applying your > effects to just some elements on the page. The backlight is a global > property. > > It's not hard to animate the backlight using JS. > Well, you could say that of css animation in general. Also I already raised the same objections in my own comments regarding specific elements vs. global. > > As for borrowing existing dom properties, one option I was looking at was: > > What happens when content (say, a web app) calls window.minimize? Does it > want to turn off the screen, or does it want to minimize itself and return > to the home screen? Using window.minimize/restore for task switching seems > much more reasonable... > > Unless you'd want window.minimize to do something different for apps and the > top-level window? This feels like a hack to me. Why? Minimize/restore only applies to the top level "chrome" html, and content html would have minimize/restore mean another thing, as the policy by the content html dictates. window.minimize is a sufficiently nasty api for content that it shouldn't even be allowed in most cases, really.

> Why [is it a hack]? 1. A window.minimize function which does two things, one of which isn't "minimize", is confusing. 2. If we overload an this property, then pages have to be aware of which context they're running in. 3. New DOM properties are cheap compared to the cost of fishy API. 4. It harms the abstraction (illusion?) that the only difference between the B2G homescreen and any other app is privilege. Now the B2G homescreen is a privileged app running *in a special context* which modifies the definition of "minimize" and "restore".

(In reply to Justin Lebar [:jlebar] from comment #31) > > Why [is it a hack]? > > 1. A window.minimize function which does two things, one of which isn't > "minimize", is confusing. > 2. If we overload an this property, then pages have to be aware of which > context they're running in. > 3. New DOM properties are cheap compared to the cost of fishy API. > 4. It harms the abstraction (illusion?) that the only difference between > the B2G homescreen and any other app is privilege. Now the B2G homescreen > is a privileged app running *in a special context* which modifies the > definition of "minimize" and "restore". Very well. Carry on then.

(In reply to Chris Jones [:cjones] [:warhammer] from comment #22) > (In reply to Justin Lebar [:jlebar] from comment #21) > > > I assume there's another mechanism preventing general web content from > > > seeing this API? If not, we need that :). > > > > You mean unprivileged pages shouldn't be aware that screen.mozEnabled > > exists? We don't have a mechanism for this that I'm aware of... > > > > My understanding is that we make the sms and telephony APIs disappear when > there's no available backend. I don't know how that's done. navigator.mozSms is null if the security constraints are not fulfilled or when the current platform doesn't handle SMS's. mozSms never disappears from the navigator object. However, this would be doable by conditionally adding nsIDOMNavigotarWhateverPartialInterface to the navigator object in nsDOMClassInfo. It's not done for WebSMS because the temporary security model consists of mozSms being null if the domain isn't whitelisted. If you want to do that for screen.mozEnabled and screen.mozBrightness you could probably create a new interface (nsIDOMMozScreenSomething that have those attributes) and conditionally add it to Screen class info.

Chris / Jonas: Are we good to go here with the current API and current API security measures, or would you like some changes?

The API is ok with me. No better ideas. It seems suboptimal that the incantation needed to enable the screen properties is different than the one for sms etc., but the check here is also fine with me.

(In reply to Chris Jones [:cjones] [:warhammer] from comment #35) > It seems suboptimal that the incantation needed to enable the screen > properties is different than the one for sms etc., but the check here is > also fine with me. Actually wait, no, this is not suboptimal but bad: when the pref is flipped, *all* web content, privileged or not, will be able to set these properties, right? How about we copy the SMS check, where there's an on/off switch and then prefs to enable access for particular domains? Should be able to copy/paste the code, or turn it into common helpers.

But I would also be ok with fixing that up in a followup bug.

> when the pref is flipped, *all* web content, privileged or not, will be able to set these > properties, right? Yes, all web content running on Gonk, privileged or not, would be able to set the properties, once the pref is flipped. And when the patch is checked in, all web content running under Gecko will be able to see that the properties exist, regardless of what the pref is set to. > How about we copy the SMS check, where there's an on/off switch and then prefs to enable access for > particular domains? Should be able to copy/paste the code, or turn it into common helpers. My understanding was that a privilege API was being worked on, which would subsume this. The SMS whitelist is a hack, right? I can add the same hack, but like Mounir said in comment 19: > Then, except if we want to implement a system for B2G we could just not handle that for the moment > in my opinion. Given that B2G can hardly run non-pre-installed content, it seems to be quite safe > to have it available.

Jonas, outstanding questions for you: 1. >> Is it really useful to have a VarCache? We could just check the pref everytime. > I really have no idea when it's appropriate to use a cache or not, so I've been using caches > everywhere. I have similar code in the navigator for whether the vibrator is enabled. 2. >> I think if sAllowScreen... isn't true, we could just return NS_OK and a default value. Throwing an >> exception seems a bad idea to me. > I'm concerned about lying to the page if we don't have to. A page might, for example, try to > determine whether Firefox is in the background by examining |screen.enabled && > !document.mozVisible|. > If we don't throw an exception, do we just drop writes to the properties? The page does > screen.enabled = false, but the next time we read screen.enabled, it's true? I guess throwing an > exception is also lame. 3. >> Ok, please call into the privilege manager > Does this code exist in the tree atm? I can't find it, if so.

(In reply to Justin Lebar [:jlebar] from comment #38) > > How about we copy the SMS check, where there's an on/off switch and then prefs to enable access for > > particular domains? Should be able to copy/paste the code, or turn it into common helpers. > > My understanding was that a privilege API was being worked on, which would > subsume this. The SMS whitelist is a hack, right? I can add the same hack, > but like Mounir said in comment 19: > > > Then, except if we want to implement a system for B2G we could just not handle that for the moment > > in my opinion. Given that B2G can hardly run non-pre-installed content, it seems to be quite safe > > to have it available. B2G can load arbitrary web content. But like I said, I'm fine with this landing as-is and us sorting it out in a followup, soon-ish. (Which might be the full permissions impl.)

Jonas, ping re comment 39?

Comment on attachment 575294 [details] [diff] [review] Patch v1 Looks good to me. Stealing from jonas.

(In reply to Justin Lebar [:jlebar] from comment #39) > Jonas, outstanding questions for you: > > 1. > > >> Is it really useful to have a VarCache? We could just check the pref everytime. > > > I really have no idea when it's appropriate to use a cache or not, so I've been using caches > > everywhere. I have similar code in the navigator for whether the vibrator is enabled. Anywhere perf sensitive should definitely have them, but this doesn't look like it'll get called a lot. I don't really think it matters either way, it doesn't take a lot of code and I doubt having the pref observer there costs us a lot (if it does we need to fix that anyway). > 2. > > >> I think if sAllowScreen... isn't true, we could just return NS_OK and a default value. Throwing an > >> exception seems a bad idea to me. > > > I'm concerned about lying to the page if we don't have to. A page might, for example, try to > > determine whether Firefox is in the background by examining |screen.enabled && > > !document.mozVisible|. > > > If we don't throw an exception, do we just drop writes to the properties? The page does > > screen.enabled = false, but the next time we read screen.enabled, it's true? I guess throwing an > > exception is also lame. Exceptions are kind'a lame. document.mozVisible should return false if the screen is off so there should be no need to check screen.enabled. > 3. > > >> Ok, please call into the privilege manager > > Does this code exist in the tree atm? I can't find it, if so. It does. See: http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#2479 http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#5643

> document.mozVisible should return false if the screen is off so there should be no need to check > screen.enabled. My concern is about naive code which is written expecting screen.enabled to be truthful and observing things which don't make much sense. What do you think screen.enabled and screen.brightness should return when the page doesn't have permissions? [true, 1], [true, -1]?

I'm just going to default enabled to true and brightness to 1. We can change this later if need be.

(In reply to Justin Lebar [:jlebar] from comment #45) > I'm just going to default enabled to true and brightness to 1. We can > change this later if need be. FWIW, that seems reasonable to me.

WRT the permission manager: Right now, the screen state is frobbed from Chrome: chrome://browser/content/shell.xul I can allow chrome to frob the property and check the permission manager for content. That sound OK?

I heard we're not supposed to touch the permission manager, it's dying. But just enabling the API for chrome code for now, and filing a followup to enable for content that blocks on The New Permissions Model, sounds fine to me.

rm some debugging code.

Pushed to m-i. Also need to push gonk parts to b2g repo.

backed out for build failures on windows Hal.obj : error LNK2019: unresolved external symbol "bool __cdecl mozilla::hal_impl::GetScreenEnabled(void)" (?GetScreenEnabled@hal_impl@mozilla@@YA_NXZ) referenced in function "bool __cdecl mozilla::hal::GetScreenEnabled(void)" (?GetScreenEnabled@hal@mozilla@@YA_NXZ) Hal.obj : error LNK2019: unresolved external symbol "void __cdecl mozilla::hal_impl::SetScreenEnabled(bool)" (?SetScreenEnabled@hal_impl@mozilla@@YAX_N@Z) referenced in function "void __cdecl mozilla::hal::SetScreenEnabled(bool)" (?SetScreenEnabled@hal@mozilla@@YAX_N@Z) Hal.obj : error LNK2019: unresolved external symbol "double __cdecl mozilla::hal_impl::GetScreenBrightness(void)" (?GetScreenBrightness@hal_impl@mozilla@@YANXZ) referenced in function "double __cdecl mozilla::hal::GetScreenBrightness(void)" (?GetScreenBrightness@hal@mozilla@@YANXZ) Hal.obj : error LNK2019: unresolved external symbol "void __cdecl mozilla::hal_impl::SetScreenBrightness(double)" (?SetScreenBrightness@hal_impl@mozilla@@YAXN@Z) referenced in function "void __cdecl mozilla::hal::SetScreenBrightness(double)" (?SetScreenBrightness@hal@mozilla@@YAXN@Z) xul.dll : fatal error LNK1120: 4 unresolved externals

Ah, there's Windows hal now. That slipped by me.

(In reply to Justin Lebar [:jlebar] from comment #54) > Ah, there's Windows hal now. That slipped by me. It has been created a few hours ago so you just ran out of luck ;)

https://hg.mozilla.org/integration/mozilla-inbound/rev/da074baa9f05 I'll land on the B2G clone once this sticks on m-i. (There's gonk-only code I can't land on m-c.)

Landed in B2G: https://github.com/cgjones/mozilla-central/commit/fe06ae146b7df4d1d339cea2848f8f3d25ab5328 https://github.com/andreasgal/B2G/commit/62af95c50cf40d5a2d4a2ff4b1f85a9e71267dca I landed the whole patch v2.1, which is what I landed on m-c minus the WindowsHal stubs (because WindowsHal isn't in B2G yet.)

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e861cc550d4 for the gonkhal bits

Mentioned on https://developer.mozilla.org/en-US/docs/DOM/window.screen https://developer.mozilla.org/en-US/docs/Firefox_12_for_developers#New_WebAPIs New pages: https://developer.mozilla.org/en-US/docs/DOM/window.screen.mozEnabled https://developer.mozilla.org/en-US/docs/DOM/window.screen.mozBrightness