511207 - last page not removed from history upon close of browser

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Mark Smith [:mcs]]

Steps to reproduce:
1) Configure Firefox to show a blank page when it starts up.
2) Configure Firefox to clear browsing history when it closes.
3) Visit a new site and then quickly exit Firefox.
4) Start Firefox and notice that that last site is in your history even though
   everything else has been cleared.

I suspect this has something to do with the lazy add strategy for history (see bug 443328).

Reproduced with Firefox 3.5.2 on Mac and Windows.  Could not reproduce in 3.0.13.

Comment 1

[Marco Bonardo [:mak]]

yes, it is possible that the visit is just added after clear history due to LAZY_ADD, that means you close the browser less than 3 seconds after the page starts loading.

Comment 2

[Mark Smith [:mcs]]

I am surprised that there does not appear to be a way to disable LAZY_ADD at run time (e.g., a hidden preference).

Should nsNavHistory::RemoveAllPages() clear the lazy messages list?

Should nsNavHistory::RemoveVisitsByTimeframe() clear lazy messages that fit between aBeginTime and aEndTime?

Comment 3

[Marco Bonardo [:mak]]

that's an interesting question, i think so, they should clean up the lazy messages if that's applicable. There is plan to change how adding visits works, but for now we could fix it like that.
Also RemovePagesByTimeframe should do that.

Since you looked atthe code already, do you want to provide a patch?

Comment 4

[Marco Bonardo [:mak]]

Probably would be easier to just force a commit before the cleanup methods start doing cleanup
#ifdef LAZY_ADD
  CommitLazyMessages();
#endif

Comment 5

[Marco Bonardo [:mak]]

asking blocking for privacy reasons.

Comment 6

[Marco Bonardo [:mak]]

Attachment: patch v1.0

I can't write a test for this. Or better, i can write a test for this, but each method would require more than 3 seconds (lazy timer) to be tested, so the test would require 12 seconds to run. So, i'm flagging litmus, it's a pretty easy test to do manually.

Comment 7

[Shawn Wilsher :sdwilsh]

Comment on attachment 399885 [details] [diff] [review]
patch v1.0

r=sdwilsh

Comment 8

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/c5f0551cef51

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bbb69b524b63