Closed
Bug 476297
Opened 16 years ago
Closed 16 years ago
Switch NavHistory idle listener to idle-daily
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: Mardak, Assigned: Mardak)
References
Details
(Keywords: fixed1.9.1)
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
dietrich
:
review+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
This is part 1 of 3 to address bug 458801.
We can use the new notification from bug 476254 to do some work once a day when the user isn't there.
|
Assignee | |
Comment 1•16 years ago
|
||
Switch over to idle-daily. Reordered the behavior in the idle handler... (Expiring pages first should make fewer things to work on for recalc, yeah?)
Delete delete delete!
|
|
Assignee | |
Comment 2•16 years ago
|
||
Would we want to up the number of pages to expire as we'll only expire once a day (and when the user quits)?
|
||
Comment 3•16 years ago
|
||
I think for expiration we want to change our algorithm entirely.
|
||
Comment 4•16 years ago
|
||
Comment on attachment 359942 [details] [diff] [review]
v1
currently, expiration can happen N times daily, when the user is idle. this switches to a single expiration run per day, right? i don't think this is feasible without some mechanism to ensure expiration doesn't fall too far behind.
|
|
Assignee | |
Comment 5•16 years ago
|
||
Right. This makes it expire once a day, so that's why I suggested perhaps bumping up the number of pages to expire.
But as Shawn pointed out, we'll probably switch to a different/async/continuous method of expiration instead of the current idle timer or this once-a-day-idle event.
|
|
Assignee | |
Comment 6•16 years ago
|
||
Just switch frecency recalculations to idle-daily and leave expiration for now.
Attachment #359942 -
Attachment is obsolete: true
Attachment #360944 -
Flags: review?(dietrich)
Attachment #359942 -
Flags: review?(dietrich)
|
|
||
Comment 7•16 years ago
|
||
Comment on attachment 360944 [details] [diff] [review]
v2
>@@ -5380,6 +5374,10 @@
> if (oldDaysMin != mExpireDaysMin || oldDaysMax != mExpireDaysMax ||
> oldVisits != mExpireSites)
> mExpire.OnExpirationChanged();
>+ } else if (nsCRT::strcmp(aTopic, gIdleDaily) == 0) {
>+ // Recalculate some frecency values (zero time means don't recalculate)
>+ if (mFrecencyUpdateIdleTime)
>+ (void)RecalculateFrecencies(mNumCalculateFrecencyOnIdle, PR_TRUE);
> } else if (nsCRT::strcmp(aTopic, NS_PRIVATE_BROWSING_SWITCH_TOPIC) == 0) {
> if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_ENTER).Equals(aData)) {
> mInPrivateBrowsing = PR_TRUE;
hm, mFrecencyUpdateIdleTime is now just basically acting the bool for whether or not to recalculate frecency at all, right?
can you update the pref comments in firefox js, as well as the handling and name of mFrecencyUpdateIdleTime?
r=me w/ those changes.
Attachment #360944 -
Flags: review?(dietrich) → review+
|
|
Assignee | |
Comment 8•16 years ago
|
||
(In reply to comment #7)
> hm, mFrecencyUpdateIdleTime is now just basically acting the bool for whether
> or not to recalculate frecency at all, right?
Yeah. Is it okay if I just leave the comment there as I'll be removing the pref in the next part (bug 476298) of these patches:
>Bug 476298 - Switch RecalculateFrecencies to just fix invalid frecencies
>Bug 476300 - Calculate all page frecencies when upgrading/migrating
>+++ b/browser/app/profile/firefox.js
>-// Perform frecency recalculation after this amount of idle, repeating.
>-// A value of zero disables updating of frecency on idle.
>-// Default is 1 minute (60000ms).
>-pref("places.frecency.updateIdleTime", 60000);
>+++ b/toolkit/components/places/src/nsNavHistory.cpp
>@@ -5318,9 +5300,8 @@
>- // Recalculate some frecency values (zero time means don't recalculate)
>- if (mFrecencyUpdateIdleTime)
>- (void)RecalculateFrecencies(mNumCalculateFrecencyOnIdle, PR_TRUE);
>+ // Update frecency values
>+ (void)FixInvalidFrecencies();
|
|
||
Comment 9•16 years ago
|
||
ah ok, sure.
|
|
Assignee | |
Comment 10•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
|
||
Comment 12•16 years ago
|
||
Comment on attachment 360944 [details] [diff] [review]
v2
a191=beltzner
Attachment #360944 -
Flags: approval1.9.1+
|
|
Assignee | |
Comment 13•16 years ago
|
||
Keywords: fixed1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•