expiring more than we need to on shutdown I think this happened as a result of a review comment of mine for bug #402297 I suggested the following change: PRBool sanitizeOnShutdown, sanitizeHistory; prefs->GetBoolPref(PREF_SANITIZE_ON_SHUTDOWN, &sanitizeOnShutdown); prefs->GetBoolPref(PREF_SANITIZE_ITEM_HISTORY, &sanitizeHistory); PRInt32 maxRecords = (sanitizeHistory && sanitizeOnShutdown) ? -1 :EXPIRATION_CAP_PLACES; // vacuum up dangling items rv = ExpireHistoryParanoid(connection, maxRecords); We call this code on quit. We should always pass in EXPIRATION_CAP_PLACES for max records, and remove the pref code. I suggested this code for the scenarion where the user clears history on quit. but in that case, we will call ExpireHistoryParanoid() directly, which passes in -1, and does it after we call ExpireItems(0, &keepGoing); so right now, if you are configured to clear private data on quit, we do twice the work we need to because we call ExpireHistoryParanoid() twice with -1. I'll attach a patch.
Created attachment 294931 [details] [diff] [review] patch
in the case of shutting down and clearing private data, we'll still call ExpireHistoryParanoid() twice, but the first time will be on nsNavHistoryExpire::OnQuit() with EXPIRATION_CAP_PLACES for max records. then, well call it again from nsNavHistoryExpire::ClearHistory(), but after calling ExpireItems() but this time with -1 for max records.
10 years ago
Comment on attachment 294931 [details] [diff] [review] patch Could we instead just not call ExpireHistoryParanoid the first time if the prefs are set? (since we know it'll get called again later) That way it'd be getting called only once.
> Could we instead just not call ExpireHistoryParanoid the first time if the > prefs are set? (since we know it'll get called again later) the first time is from nsNavHistoryExpire::OnQuit(), and we always want to call that in order to do partial expiration. (Think about the common case of *not* clearing private data on shutdown. only onQuit() gets called.) the second time is nsNavHistoryExpire::ClearHistory(), and we call it here after calling ExpireItems() to clean up the database after calling ExpireItems(). I thought about just doing this from onQuit(), but we still need the call in ClearHistory() as it has to be after the call to ExpireItems() and ClearHistory() is called when you manually clear all history when clearing private data. what we could do is, in onQuit, keep the existing code, and if we determine that maxRecords is -1, we do nothing in onQuit() and instead, wait to do the work in ClearHistory(). This would avoid the partial expiration in onQuit() in this scenario. if you think that's a better fix, I can whip that up.
> if you think that's a better fix, I can whip that up. I think that is the right fix, patch coming.
Created attachment 295126 [details] [diff] [review] patch, v2, per dietrich's suggestion if our prefs indicate that we are going to be clearing history, don't bother with the partial expiration onQuit()
Comment on attachment 295126 [details] [diff] [review] patch, v2, per dietrich's suggestion r=me, thanks!
10 years ago
Comment on attachment 295126 [details] [diff] [review] patch, v2, per dietrich's suggestion seeking approval
fixed. Checking in nsNavHistoryExpire.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp,v <-- nsNavHistoryExpire.cpp new revision: 1.33; previous revision: 1.32 done
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