1299846 - Complete the implementation of chrome.idle.queryState

Status: RESOLVED FIXED

(WebExtensions :: Compatibility, defect, P2)

Description

[Bob Silverberg [:bsilverberg]]

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 1

[Bob Silverberg [:bsilverberg]]

Attachment: Bug 1299846 - Complete the implementation of chrome.idle.queryState,

Review commit: https://reviewboard.mozilla.org/r/76114/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/76114/

Comment 2

[Kris Maglione [:kmag]]

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.

Comment 3

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8787309 [details]
Bug 1299846 - Complete the implementation of chrome.idle.queryState,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/76114/diff/1-2/

Comment 4

[Bob Silverberg [:bsilverberg]]

Try looks good, requesting checkin.

Comment 5

[Bob Silverberg [:bsilverberg]]

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf2af823492c
Complete the implementation of chrome.idle.queryState, r=kmag

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/bf2af823492c

Comment 8

[Will Bamberg [:wbamberg]]

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.

Comment 9

[Bob Silverberg [:bsilverberg]]

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?

Comment 10

[Will Bamberg [: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.