Closed Bug 1299846 Opened 8 years ago Closed 8 years ago

Complete the implementation of chrome.idle.queryState

Categories

(WebExtensions :: Compatibility, defect, P2)

defect

Tracking

(firefox51 fixed)

RESOLVED FIXED
mozilla51
Iteration:
52.1 - Oct 3
Tracking Status
firefox51 --- fixed

People

(Reporter: bsilverberg, Assigned: bsilverberg)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [idle] triaged)

Attachments

(1 file)

The chrome.idle.queryState [1] method has been implemented as a stub which simply returns "active" whenever `queryState` is called.

This bug is to update the implementation to report a more accurate value.

It will use nsIIdleService [2], specifically the `idleTime` property, to check how long the system has been idle, and then report either "active" or "idle".

[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/idle/queryState
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIIdleService
Comment on attachment 8787309 [details]
Bug 1299846 - Complete the implementation of chrome.idle.queryState,

https://reviewboard.mozilla.org/r/76114/#review74114

::: toolkit/components/extensions/ext-idle.js:11
(Diff revision 1)
> +
>  extensions.registerSchemaAPI("idle", "addon_parent", context => {
>    return {
>      idle: {
>        queryState: function(detectionIntervalInSeconds) {
> +        if (idleService.idleTime / 1000 < detectionIntervalInSeconds) {

Not that it matters much, but multiplication is cheaper than division, and integer comparisons are cheaper than floating point comparisons, `idleTime < detectionIntervalInSeconds * 1000` would be better.
Attachment #8787309 - Flags: review?(kmaglione+bmo) → review+
Try looks good, requesting checkin.
Keywords: checkin-needed
Adding dev-doc-needed as this changes the current behaviour of browser.idle.queryState. Prior to this patch, state was always reported as "active", whereas now state will be (somewhat) accurately reported as either "active" or "idle". "locked" remains unsupported.

[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/idle/queryState
Keywords: dev-doc-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bf2af823492c
Complete the implementation of chrome.idle.queryState, r=kmag
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bf2af823492c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
I've updated the compat data for idle.queryState():

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/idle/queryState
https://github.com/mdn/browser-compat-data/commit/8ec3d9a8c2ed47d9a574bbe2d8c6e04536d124ba

Marking as dev-doc-complete, but please let me know if anything else is needed here.
This looks good Will, but I notice there is still a note at the bottom of the page that says "Note: This function currently returns "active" in Firefox, regardless of the system idle state." Was that left in by accident, or is that note meant to be accurate for the current release version of Firefox, which it is? If the latter, what will inform us to change it when 51 goes to release?
Flags: needinfo?(wbamberg)
(In reply to Bob Silverberg [:bsilverberg] from comment #9)
> This looks good Will, but I notice there is still a note at the bottom of
> the page that says "Note: This function currently returns "active" in
> Firefox, regardless of the system idle state." 

Thanks for catching that Bob. I thought I had got it but apparently didn't. 

> Was that left in by accident,
> or is that note meant to be accurate for the current release version of
> Firefox, which it is? If the latter, what will inform us to change it when
> 51 goes to release?

As far as possible I'd like to remove random notes like this entirely, and keep all notes on browser support in the compat data.
Flags: needinfo?(wbamberg)
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.