Closed Bug 1320324 Opened 3 years ago Closed 3 years ago

Sync error while resetting the add-on to its default values

Categories

(WebExtensions :: Untriaged, defect, P3)

defect

Tracking

(firefox50 unaffected, firefox51 unaffected, firefox52 fixed, firefox53 verified)

VERIFIED FIXED
mozilla53
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- fixed
firefox53 --- verified

People

(Reporter: vasilica.mihasca, Assigned: glasserc)

References

Details

(Whiteboard: [sync]triaged)

Attachments

(3 files)

[Prerequisites]:
- webextensions.storage.sync.enabled set to true
- create a new string setting identity.fxaccounts.autoconfig.uri and set it to https://stable.dev.lcip.org

[Affected versions]:
Firefox 53.0a1 (2016-11-24)
Firefox 52.0a2 (2016-11-22)


[Affected platforms]:
Windows 10 64-bit
Ubuntu 16.04 32-bit


[Steps to reproduce]:
1.Launch Firefox with a clean profile and set up the configuration.
2.Go to about:preferences#sync and login using a valid FxA account. 
3.Install the attached webextension.
4.Navigate to the add-on details tab from about:addons and uncheck some colors or change a few names from textboxes.
5.Force a sync.
6.Click on “Reset to default” button from the add-on details tab.
7.Force a sync.


[Expected Results]:
The default values are successfully synced.


[Actual Results]:
 - The values are reset but the following error is thrown in Browser Console after clicking on “Reset to default” button: Cannot send function call result: other side closed connection  ExtensionParent.jsm:469
 - The following error-sync-1480082060262.txt appears in about:sync-log after forcing the second sync: http://pastebin.com/VgP53DhG


[Additional notes]:
The default values are not synchronized to another Firefox profile.
Assignee: nobody → eglassercamp
Priority: -- → P3
Whiteboard: [sync]triaged
Thanks for reporting. It looks like there's something obvious wrong with the code that handles deleted records/tombstones.
Comment on attachment 8821250 [details]
Bug 1320324: deleted values are not handled correctly on sync,

This looks good to me, but I don't think I'm the right person to review it.
Attachment #8821250 - Flags: review?(kmaglione+bmo) → review?(markh)
Comment on attachment 8821250 [details]
Bug 1320324: deleted values are not handled correctly on sync,

https://reviewboard.mozilla.org/r/100560/#review104446

LGTM!
Attachment #8821250 - Flags: review?(markh) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9ae4e620a16d
deleted values are not handled correctly on sync, r=markh
Keywords: checkin-needed
backed out for eslint failures like https://treeherder.mozilla.org/logviewer.html#?job_id=68025050&repo=autoland
Flags: needinfo?(eglassercamp)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6c2eac75f422
Backed out changeset 9ae4e620a16d for eslint failure
Ouch, sorry about that!
Flags: needinfo?(eglassercamp)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d2a2e98c362d
deleted values are not handled correctly on sync, r=markh
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3fb11b63ef49
Fix ESLint error in test_ext_storage_sync.js.
Ethan, is it something that we want to uplift? Thanks
Flags: needinfo?(eglassercamp)
Comment on attachment 8821250 [details]
Bug 1320324: deleted values are not handled correctly on sync,

Approval Request Comment
[Feature/Bug causing the regression]: chrome.storage.sync, specifically bug 1253740
[User impact if declined]: Syncing settings will not work correctly for the users which have enabled it
[Is this code covered by automated tests?]: Yes (see patch)
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: No
[Why is the change risky/not risky?]: The change only affects users who have enabled the feature using the about:config flag, and only fixes one bug related to that feature
[String changes made/needed]: None
Flags: needinfo?(eglassercamp)
Attachment #8821250 - Flags: approval-mozilla-beta?
Yes, I'd like to uplift it to whatever you are willing to uplift it to.
Comment on attachment 8821250 [details]
Bug 1320324: deleted values are not handled correctly on sync,

fix issue with deleted values in sync, beta52+
Attachment #8821250 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attached file syncerror.txt
Tested this issue on Firefox 53.0b7 (20170327081421) under Windows 10 64-bit and Ubuntu 16.04 32-bit and noticed the following:

 - The following error appears in Browser Console when clicking on “Reset to default” button: Cannot send function call result: other side closed connection  ExtensionParent.jsm:554
 - The default values are successfully synced.
 - An error is encountered while syncing new values after “reset to default” step has already been performed. But, in spite of this, the new values are synced between profiles. I am attaching the sync-log file.

Could this error be related to Bug 1351678 or this is a different issue and must be tracked separately?
Flags: needinfo?(eglassercamp)
Sorry about the delay in getting to this. Yes, I think this is the same as bug 1351678.
Flags: needinfo?(eglassercamp)
Also, I don't know anything about "Cannot send function call result: other side closed connection  ExtensionParent.jsm:554". That isn't related to storage.sync code as far as I know.
Tested again on Firefox 53 (20170413192749) and Firefox 55.0a1 (2017-04-24) under Windows 10 64-bit and Ubuntu 16.04 32-bit using a different webextension (https://addons-dev.allizom.org/en-US/firefox/addon/instant-multi-search/). This issue seems to be fixed and the default values are successfully synced.
Status: RESOLVED → VERIFIED
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.