Closed Bug 407018 Opened 17 years ago Closed 17 years ago

expiration not using browser.history_expire_days_min

Categories

(Firefox :: Bookmarks & History, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 3 beta2

People

(Reporter: dietrich, Assigned: dietrich)

References

Details

Attachments

(1 file)

Attached patch fix (deleted) — Splinter Review
found some typos and test errors that cause us to not use the minimum date expiration range.
Attachment #291717 - Flags: review?(sspitzer)
Attachment #291717 - Flags: approvalM10?
Attachment #291717 - Flags: approvalM10?
Priority: -- → P1
Comment on attachment 291717 [details] [diff] [review] fix sorry for not catching the s/selectStatement/selectMinStatement typo during the review
Attachment #291717 - Flags: review?(sspitzer) → review+
Attachment #291717 - Flags: approvalM10?
Comment on attachment 291717 [details] [diff] [review] fix bug in the fix for a P1 blocker, needed to actually fix it, thus aM10 on behalf of drivers
Attachment #291717 - Flags: approvalM10? → approvalM10+
Checking in toolkit/components/places/src/nsNavHistoryExpire.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp,v <-- nsNavHistoryExpire.cpp new revision: 1.26; previous revision: 1.25 done Checking in toolkit/components/places/tests/unit/test_expiration.js; /cvsroot/mozilla/toolkit/components/places/tests/unit/test_expiration.js,v <-- test_expiration.js new revision: 1.12; previous revision: 1.11 done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
It looks like trace_malloc_allocs on bm-xserve11 (OS X) jumped significantly with this checkin. Windows and Linux didn't change though, oddly enough. I'm guessing that the actualy problem is in bug 402880, checked in ~12 hours earlier, but just wasn't exposed without this fix. Note allocation counts and total size from the test runs before and after this checkin: ----------- Output from trace-malloc bloat test: leakstats ------------- ./leakstats starting at Wed Dec 5 13:35:40 2007 Leaks: 999855 bytes, 12027 allocations Maximum Heap Size: 18895765 bytes 172290331 bytes were allocated in 468121 allocations. Logged 870 free (or realloc) calls for which we missed the original malloc. ----------- Output from trace-malloc bloat test: leakstats ------------- ./leakstats starting at Wed Dec 5 13:46:44 2007 Leaks: 996087 bytes, 11975 allocations Maximum Heap Size: 19387710 bytes 315604101 bytes were allocated in 570737 allocations. Logged 873 free (or realloc) calls for which we missed the original malloc. http://build-graphs.mozilla.org/graph/query.cgi?testname=trace_malloc_allocs&units=count&tbox=bm-xserve11.build.mozilla.org&autoscale=1&days=7&avg=1&showpoint=2007:12:05:13:24:58,467098
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Flags: blocking-firefox3?
Confirmed, after backing out trace_malloc_allocs dropped to the previous level.
Flags: blocking-firefox3? → blocking-firefox3+
Seth/Dietrich: do we have an idea of what's causing this and an ETA for a repair? Do we need to just back out bug 402880 entirely until we have this resolved? Trying to figure out what to do for B2 freeze, and whether this follow-up fix is required for B2.
I'm still working on tracking down the allocations. I'm trying to get the tests running locally, as well as going to see about checking back in temporarily and grabbing the logs off of the tinderbox. I'll come up w/ a partial backout patch, since we do want to keep some parts of bug 402880.
On line 478 you have: nsCOMPtr<mozIStorageStatement> selectMinStatement; nsresult rv = aConnection->CreateStatement(sqlMinAge,getter_AddRefs(selectMinStatement)); Where you create the selectMinStatement - but it is never used below (selectStatement is). Was that intended?
> Was that intended? schrep, it sounds like you are looking at lxr (or what is checked in). dietrich patch for this bug fixes that.
(In reply to comment #9) > > Was that intended? > > schrep, it sounds like you are looking at lxr (or what is checked in). > dietrich patch for this bug fixes that. > Right: The patch for this bug fixes the issue Schrep points out. Applying that patch caused the huge trace_malloc_allocs jump on bm-xserve11, so we reverted it.
checked back in, after working w/ sspitzer, rhelmer and coop. initial numbers look good, but the box is having problems now, so not closing this yet. turns out the mac memtest box was not cleaning it's profile, so it's history was getting bigger and older. current theory: once bug 402280 (and patched via this bug), we started expiring more of those visits, and allocating more memory. Checking in toolkit/components/places/src/nsNavHistoryExpire.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp,v <-- nsNavHistoryExpire.cpp new revision: 1.30; previous revision: 1.29 done
Reland the test?
Assignee: nobody → dietrich
Status: REOPENED → NEW
Flags: in-testsuite?
This is the last beta 2 bug. Please update the status ASAP.
This was checked back in... resolving.
Status: NEW → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
relanded tests. Checking in toolkit/components/places/tests/unit/test_expiration.js; /cvsroot/mozilla/toolkit/components/places/tests/unit/test_expiration.js,v <-- test_expiration.js new revision: 1.14; previous revision: 1.13 done
had to do some tweaking of browser.history_expire_days and browser.history_expire_sites to verify this with a profile containing just two days of history. but it looks good on Win XP nightly. adding in-testsuite+ based on comment #15
Status: RESOLVED → VERIFIED
Flags: in-testsuite? → in-testsuite+
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.

Attachment

General

Created:
Updated:
Size: