If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Send a notification once a day when the user goes idle for some time

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
Widget
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Mardak, Assigned: Mardak)

Tracking

({dev-doc-complete, fixed1.9.1})

Trunk
mozilla1.9.2a1
dev-doc-complete, fixed1.9.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.93 KB, patch
beltzner
: approval1.9.1+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
This would be useful for making backups or processing/updating precomputed values.

One existing use of the idle service is for backing up bookmarks. A new use would be to avoid problems like bug 449124 and bug 458801 where we do a batch frecency update instead of repeatedly calculate a small check every few minutes of idle.

The notification would be something like "once-a-day-on-idle" and would probably use a pref like "idle.lastDailyNotification" to check if it was already sent in the last 24 hours.
(Assignee)

Updated

9 years ago
Depends on: 476253
(Assignee)

Comment 1

9 years ago
Created attachment 359891 [details] [diff] [review]
v1

Patch is on top of that of bug 476253.

I was thinking about caching the pref value and adding an observer to check if the user modifies it, but I figured this was on idle anyway, so it's not super bad that we keep GetIntPref-ing.

Went with "idle-daily" as the topic.
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #359891 - Flags: review?(roc)
+            PRUint64 now;
+            LL_DIV(now, PR_Now(), PR_USEC_PER_SEC);
+            PRUint32 nowSec;
+            LL_L2I(nowSec, now);

Use real operators.

Why do we actually need the once-a-day timer? Couldn't we just check whether to run idle-daily every time we go idle? Presumably if the user remains idle for an entire day we don't really need to fire idle-daily anyway.
(Assignee)

Comment 3

9 years ago
(In reply to comment #2)
> Why do we actually need the once-a-day timer?
Instead of having each place add itself as a listener? Sure, but it seems like if multiple places are just doing the same logic, might as well consolidate. It would be simpler to just listen for idle-daily topic and do something.

> Couldn't we just check whether to run idle-daily every time we go idle?
You mean as opposed to having a separate idle check? Sure, but that would require at least one listener.

> Presumably if the user remains idle for
> an entire day we don't really need to fire idle-daily anyway.
Why not? This notification would be used for things that want to do periodic updates. Backing up data, checking for updates, recalculating values, expiring data.
(In reply to comment #3)
> (In reply to comment #2)
> > Why do we actually need the once-a-day timer?
> Instead of having each place add itself as a listener? Sure, but it seems like
> if multiple places are just doing the same logic, might as well consolidate. It
> would be simpler to just listen for idle-daily topic and do something.
> 
> > Couldn't we just check whether to run idle-daily every time we go idle?
> You mean as opposed to having a separate idle check? Sure, but that would
> require at least one listener.

No, I mean having the idle service fire idle-daily, but not actually forcing a timer to run, just assuming that we don't need to fire idle-daily if there's no transition from active->idle during an entire day.

I guess if there are no listeners then that doesn't work though.

> > Presumably if the user remains idle for
> > an entire day we don't really need to fire idle-daily anyway.
> Why not? This notification would be used for things that want to do periodic
> updates. Backing up data, checking for updates, recalculating values, expiring
> data.

OK.
Comment on attachment 359891 [details] [diff] [review]
v1

With the math fix.
Attachment #359891 - Flags: superreview+
Attachment #359891 - Flags: review?(roc)
Attachment #359891 - Flags: review+
(Assignee)

Comment 6

9 years ago
Created attachment 359901 [details] [diff] [review]
v1.1

(In reply to comment #5)
> With the math fix.
Changed to just |PRUint32 nowSec = PR_Now() / PR_USEC_PER_SEC;|
Attachment #359891 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Blocks: 476297
(Assignee)

Comment 7

9 years ago
http://hg.mozilla.org/mozilla-central/rev/fd042e69eb9f
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1

Updated

9 years ago
Keywords: dev-doc-needed
https://developer.mozilla.org/en/nsIIdleService#addIdleObserver()
Keywords: dev-doc-needed → dev-doc-complete
Also: https://developer.mozilla.org/en/Observer_Notifications#Idle_Service
(Assignee)

Comment 10

9 years ago
Comment on attachment 359901 [details] [diff] [review]
v1.1

Provides a useful notification for doing things at most once a day. Needed for bug 482069's fix which has a test using the notification. Also needs bug 476253.

This just adds a new topic notification, but doesn't change any idls.
Attachment #359901 - Flags: approval1.9.1?
Comment on attachment 359901 [details] [diff] [review]
v1.1

a191=beltzner
Attachment #359901 - Flags: approval1.9.1? → approval1.9.1+
(Assignee)

Comment 12

9 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ac1562d96d16
Keywords: fixed1.9.1
You need to log in before you can comment on or make changes to this bug.