Closed
Bug 407373
Opened 17 years ago
Closed 17 years ago
if ExpireItems() fails, don't fire the timer again.
Categories
(Firefox :: Bookmarks & History, defect)
Firefox
Bookmarks & History
Tracking
()
RESOLVED
FIXED
Firefox 3 beta2
People
(Reporter: moco, Assigned: moco)
References
Details
Attachments
(1 file)
(deleted),
patch
|
dietrich
:
review+
mtschrep
:
approvalM10+
|
Details | Diff | Splinter Review |
if ExpireItems() fails, don't keep going
after running over night, in my console, I was seeing:
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8000FFFF: file c:/builds
/trunk/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp, line 483
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8000FFFF: file c:/builds
/trunk/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp, line 335
WARNING: ExpireItems failed.: file c:/builds/trunk/mozilla/toolkit/components/pl
aces/src/nsNavHistoryExpire.cpp, line 292
even when no longer idle, every 3.5 seconds, I'd get those messages.
Dietrich's fix for bug #407018 should fix the error, as the problem is caused by using selectStatement instead of selectMinStatement.
But I think we should consider either changing the logic in ExpireItems (which sets keepGoing), or fix DoPartialExpiration.
Dietrich, what do you think about this:
nsresult
nsNavHistoryExpire::DoPartialExpiration()
{
// expire history items
PRBool keepGoing;
nsresult rv = ExpireItems(EXPIRATION_COUNT_PER_RUN, &keepGoing);
if (NS_FAILED(rv))
NS_WARNING("ExpireItems failed.");
else if (keepGoing)
StartTimer(SUBSEQUENT_EXPIRATION_TIMEOUT);
return NS_OK;
}
Assignee | ||
Comment 1•17 years ago
|
||
Attachment #292100 -
Flags: review?(dietrich)
Comment 2•17 years ago
|
||
Comment on attachment 292100 [details] [diff] [review]
patch
Thanks seth, i had a similar fix in my local tree for bug 407018. This should go in regardless of whether i figure out the allocations problem. r=me.
Attachment #292100 -
Flags: review?(dietrich) → review+
Assignee | ||
Updated•17 years ago
|
Attachment #292100 -
Flags: approvalM10?
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → sspitzer
Target Milestone: --- → Firefox 3 M11
Comment 3•17 years ago
|
||
So you want this for b2?
Assignee | ||
Comment 4•17 years ago
|
||
> So you want this for b2?
If we get bug #407018 for b2, then it isn't so much of an issue (as we should not be failing.)
But I'd still consider taking it, because:
Once you get into the state where you have no more records older than the max-age cap, we'll hit this bug when we start to look for visits older than the min-age cap and over the sites cap. (Keep in mind thought that once we have dietrich fix for bug #407018, we will not return failure.)
without his fix for bug #407018, after visiting a page, 3.5 seconds later we fire the timer to expire 6 visits. when that timer fires, we will execute a query to find visits older than the min-age cap. (good news is this is not a slow query anymore, thanks to marco!) we'll fail, and then continue to fire this query every 3.5 seconds.
note, this fix prevents this from happening if there are any other unexpected failures in ::ExpiredItems().
additionally, the fix is low risk.
Status: NEW → ASSIGNED
Comment 5•17 years ago
|
||
Comment on attachment 292100 [details] [diff] [review]
patch
Ok lets do it then...
Attachment #292100 -
Flags: approvalM10? → approvalM10+
Assignee | ||
Comment 6•17 years ago
|
||
fixed, thanks for the approval.
Checking in nsNavHistoryExpire.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp,v <-- ns
NavHistoryExpire.cpp
new revision: 1.29; previous revision: 1.28
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: Firefox 3 M11 → Firefox 3 M10
Comment 7•15 years ago
|
||
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".
In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body contains places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.
Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.
Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in
before you can comment on or make changes to this bug.
Description
•