Closed Bug 477583 Opened 13 years ago Closed 12 years ago

Backups of bookmarks stops working if a future backup exists

Categories

(Firefox :: Bookmarks & History, defect)

3.0 Branch
x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3.7a1

People

(Reporter: fabio217, Assigned: mak)

References

Details

Attachments

(3 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6 (.NET CLR 3.5.30729)

Backups of bookmarks will stop working as soon as a *.json file with a newer date is found in the backup folder.
If you happen move ahead the date on your computer, to test some software in my case, and during that time open and close Mozilla a backup will be created and stamped with the fake date. Once you set the computer calendar to the real date and time Mozilla backup will stop working. 

Reproducible: Always

Steps to Reproduce:
1.Open and close all running instances of Mozilla.(This step should create a backup file. *.json)
2.Move forwared the computer date (lets say 30 days) 
3.Open and close Mozilla.
4.Check the backup folder for the latest backup file (*.json) It will be stamped with the current date +30 days
5.Move back the date to the current one.
6.Open Mozilla, then close it.
7.Check the backup folder and you will notice that no new backups are being made.
Actual Results:  
Backups of bookmarks stop working

Expected Results:  
Backup of bookmarks should work

The software has to be able to always make backups.  Probably, the software will have to label the backups differently to distinguish them from the rest.
Do you mean automatic backups stop working?
have you by chance changed the number of backups value in about:config?

(In reply to comment #1)
> Do you mean automatic backups stop working?

Yes, I mean automatic backups stop working
(In reply to comment #2)
> have you by chance changed the number of backups value in about:config?

No, I have not changed the number of backups value.
Version: unspecified → 3.0 Branch
Keywords: qawanted
I can confirm this behavior on Win XP Shiretoko nightly build from 20090414

Once the time was shifted forward then back, the only way to get an automatic backup is to remove all the backups.  Then, that one doesn't get updated on subsequent shutdowns.  Is there a timer that only updates after some period of time of browser usage?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: qawanted
bah, yeah, getMostRecentBackup is a bogus util.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Summary: Backups of bookmarks stops working → Backups of bookmarks stops working if a future backup exists
non critical or major because requires a non common setup (not many people move on the clock, people doing that are usually testing something, so they are expert enough to workaround the problem removing the future backup)
Severity: critical → normal
Flags: in-testsuite?
Attached patch patch v1.0 (obsolete) — Splinter Review
Attachment #399656 - Flags: review?(mano)
this adds some cleanup to backup code, tries to reuse some common code paths to avoid bugs, groups things together.
i also increased number of backups to 10, due to recent sqlite corruption issues we have seen, that often are detected by users after some day (so 5 days are a really small amount of time).
I moved idle time before backup to 15 mins (was 1 hour), to reduce chances to run that at shutdown. Users with many bookmarks could experience slower shutdowns if that happens. I've not moved it to idle-daily because it would have less chances to run, and most likely would always run at first shutdown.

Also added a unit test for this specific (future backup) case.
Attached patch patch v1.1 (obsolete) — Splinter Review
unbitrot
Attachment #399656 - Attachment is obsolete: true
Attached patch patch v1.2Splinter Review
i forgot a qrefresh :(
Attachment #403453 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/798a53f68398

cc-ing Gavin since he is using this code in Feenec and if this lands on 1.9.2 he could need to update it, changes for import should be trivial though, just add the helper object in the middle: PU.backups.utilname
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
Flags: in-testsuite? → in-testsuite+
The renaming in this patch (PlacesUtils -> PlacesUtils.backups) isn't ideal - Fennec currently builds off of both trunk and branch gecko with the same frontend code. Since there's little practical benefit to that part of the change, can we just undo it?
(In reply to comment #14)
> The renaming in this patch (PlacesUtils -> PlacesUtils.backups) isn't ideal -
> Fennec currently builds off of both trunk and branch gecko with the same
> frontend code. Since there's little practical benefit to that part of the
> change, can we just undo it?
So what, we can't ever rename anything that Fennec uses because it builds on two different branches?  That seems like it is unnecessarily tying the hands of developers.  Is Fennec shipping on both mozilla-central and 1.9.2 right now?
Gavin pointed out that the win is not worth the breakage, and finally i have to agree with him, i'll restore something in the filed bug.

But for the most part i agree fennec should move using separate branches not only for frontend since it's more likely in future the divergence between trunk and 1.9.2 APIs will get larger.
this patch includes fix for bug 520547, that address a compatibility concern, reducing risk of incompatibilities of existing code.

The patch comes with tests.
It brings the following improvements:
- finding/walking backups is faster
- if a future backup exists we will still create backups (actually we give up)
- backup on idle time is reduced to 15 minutes (to try avoid backing up at shutdown) from 1 hour
- number of backups increased to 10 to prevent dataloss when a not-immediately-visible database corruption happens
Attachment #404831 - Flags: approval1.9.2?
We already have a reviewed patch that will fix that, i'll push it now, btw thanks
(That patch is in bug 520743.)
Depends on: 520743
yeah, if we push the above patch to 1.9.2 we should still fix the strict warning here though.
(In reply to comment #16)
> So what, we can't ever rename anything that Fennec uses because it builds on
> two different branches? That seems like it is unnecessarily tying the hands of
> developers.  Is Fennec shipping on both mozilla-central and 1.9.2 right now?

Sorry, I didn't see this comment until now. I didn't say "we can't ever rename anything" - your rephrasing is unnecessarily inflammatory.

Yes, Fennec currently runs on both 1.9.2 and trunk from a single front-end repository. This change introduced pain (API change, fennec broke on trunk) for practically no gain, so Marco and I agreed to revert it.

Fennec may have to deal with having separate code across multiple branches in the future, but hopefully only in response to a compatibility change whose benefits outweigh its cost!
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
Comment on attachment 404831 [details] [diff] [review]
roll-up patch for 1.9.2 branch

seems too late to be considering this on branch to solve an uncommon problem (per comment 7)
Attachment #404831 - Flags: approval1.9.2?
You need to log in before you can comment on or make changes to this bug.