Closed
Bug 714202
Opened 11 years ago
Closed 11 years ago
Addon Sync: addons failed: this._changes[0] is undefined in pruneChangesBeforeDate
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
VERIFIED
FIXED
mozilla12
Tracking | Status | |
---|---|---|
firefox11 | --- | verified |
People
(Reporter: i, Assigned: gps)
Details
Attachments
(3 files, 1 obsolete file)
12.08 KB,
text/plain
|
Details | |
2.98 KB,
patch
|
rnewman
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
2.93 KB,
text/plain
|
Details |
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:12.0a1) Gecko/20111229 Firefox/12.0a1 Build ID: 20111229031056 Steps to reproduce: I enabled add-on sync. Actual results: When I updated nightly at Dec 29th, I encountered an syncing error. When I disable add-on sync, no error occur. I have to reset the recovery key to solve it.
Severity: normal → blocker
OS: All → Windows 7
Priority: -- → P1
Hardware: All → x86_64
Comment 1•11 years ago
|
||
Greg, take a look?
Assignee: nobody → gps
Severity: blocker → normal
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: P1 → --
Summary: sync error when add-on sync enabled → Addon Sync: addons failed: this._changes[0] is undefined in pruneChangesBeforeDate
Assignee | ||
Comment 2•11 years ago
|
||
It appears to be failing in the 2nd line of the following: while (this._changes.length > 0) { if (this._changes[0][0] >= date) { return; } delete this._changes[0]; } The elements of this._changes should always be arrays. This is most weird. The next step is to figure out how an undefined element is being written to that array. As part of fixing this, I'll have to determine whether it is possible bad data was being serialized to the reconciler's JSON file. If so, I'll need to sanitize data on load so issues in the wild repair themselves. Jian: can you upload or give a link to a copy of your weave/addonsreconciler.json file from the profile directory? This file may contain personal information. If you don't want the world to see it, you can email it to me at gps@mozilla.com.
Assignee | ||
Comment 3•11 years ago
|
||
Looks like we are using the delete operator incorrectly. From https://developer.mozilla.org/en/JavaScript/Reference/Operators/delete: > When you delete an array element, the array length is not affected. For > example, if you delete a[3], a[4] is still a[4] and a[3] is undefined. *sigh* It also looks like there is no explicit test coverage for this. I'll code up a fix. I don't think undefined is being written to the reconciler's JSON file because JS will error before the mutated array has an opportunity to be written. But, I would still like verification from OP that the saved JSON is sane.
Assignee | ||
Comment 4•11 years ago
|
||
I switched the implementation to use Array.filter. Includes a new test ensuring that the function behaves properly. The test failed on previous code and passes with this patch. This patch will eventually need to land in Firefox 11 (currently Aurora) because it is a critical Sync bug.
Attachment #585010 -
Flags: review?(rnewman)
Assignee | ||
Updated•11 years ago
|
OS: Windows 7 → All
Hardware: x86_64 → All
Comment 5•11 years ago
|
||
Comment on attachment 585010 [details] [diff] [review] Use Array.filter to produce new array Review of attachment 585010 [details] [diff] [review]: ----------------------------------------------------------------- ::: services/sync/tests/unit/test_addons_reconciler.js @@ +174,5 @@ > + do_check_eq(young, reconciler._changes[0][0]); > + do_check_eq("bar", reconciler._changes[0][2]); > + > + run_next_test(); > +}); Please also add tests for: * Pruning an empty change array * Pruning a change array for which no items will be removed * Pruning a change array for which all items will be removed
Attachment #585010 -
Flags: review?(rnewman)
Attachment #585010 -
Flags: review+
Attachment #585010 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 6•11 years ago
|
||
Patch with MOAR tests per review. All tests pass.
Attachment #585010 -
Attachment is obsolete: true
Attachment #585010 -
Flags: approval-mozilla-aurora?
Attachment #585027 -
Flags: review?(rnewman)
Updated•11 years ago
|
Attachment #585027 -
Flags: review?(rnewman)
Attachment #585027 -
Flags: review+
Attachment #585027 -
Flags: approval-mozilla-aurora?
This is the addonsreconciler.json in my profile directory. I think there's no particular personal information in it. Hope it will help you to fix the bug.
Assignee | ||
Comment 8•11 years ago
|
||
Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/9dda4ea46da1 rnewman gave approval in IRC to skip s-c and get this in m-c as quick as possible.
Assignee | ||
Comment 9•11 years ago
|
||
Jian: I wanted to thank you for an awesome first bug report! It's pretty rare (at least for Sync) for someone's first bug report to be completely diagnosable from the initial comment. Your thoroughness is very much appreciated. Have a great new year.
Reporter | ||
Comment 10•11 years ago
|
||
It's my pleasure. I'm glad to hear that. I love FF though it's not perfect. Actually I'm majored in Computer Architecture and I once planed to read the source code of FF, but I feel that it is really difficult to begin with. Anyway, I've made a little contribution to FF now, which makes me very happy. lol Happy new year!
Comment 11•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9dda4ea46da1
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Comment 12•11 years ago
|
||
Comment on attachment 585027 [details] [diff] [review] Use Array.filter to produce new array, v2 [Triage Comment] Approved for Aurora to fix a major add-on sync issue.
Attachment #585027 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 13•11 years ago
|
||
Pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/e9e19f91a325
Updated•11 years ago
|
status-firefox11:
--- → fixed
Comment 14•11 years ago
|
||
I don't see this just enabling add-ons sync on nightly or aurora. Is there any specific setup or conditions to reproduce this?
Assignee | ||
Comment 15•11 years ago
|
||
Tracy: this is fixed in Nightly and will not repro there. To repro on Aurora 11.0a2, if you perform enough add-on actions, you should eventually run into this bug.
Updated•10 years ago
|
Status: RESOLVED → VERIFIED
Updated•4 years ago
|
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•