Sync doesn't account for clientsEngine not being initialized

RESOLVED FIXED in Firefox 62

Status

()

P1
normal
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: eoger, Assigned: markh)

Tracking

({regression})

unspecified
Firefox 63
regression
Points:
---

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox61 unaffected, firefox62 fixed, firefox63 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 months ago
STR:
Connect to sync, change the sync password on a 2nd device, and restart the browser (should be in "reconnect" state).
Go to sync prefs.
Try disconnecting sync.

Expected:
Sync disconnected without issues

Actual:
>Failed to sanitize TypeError: "engine is undefined"
>	_stopTracking resource://services-sync/service.js:502:43
>	startOver resource://services-sync/service.js:805:11
>	doSyncAndAccountDisconnect resource://services-sync/SyncDisconnect.jsm:123:11
>	_startDisconnect resource://services-sync/SyncDisconnect.jsm:161:11
>	disconnect resource://services-sync/SyncDisconnect.jsm:174:37
>	disconnect resource://services-sync/SyncDisconnect.jsm:203:14
>	accept chrome://browser/content/preferences/in-content/syncDisconnect.js:37:28
>	oncommand chrome://browser/content/preferences/in-content/syncDisconnect.xul:1:1
> syncDisconnect.js:47:7
> [Show/hide message details.] Weave.Service.clientsEngine is undefined
(Assignee)

Updated

7 months ago
Assignee: nobody → markh
Blocks: 1409208
Status: NEW → ASSIGNED
status-firefox61: --- → unaffected
status-firefox62: --- → affected
status-firefox63: --- → affected
Priority: -- → P1
(Assignee)

Comment 1

7 months ago
MozReview-Commit-ID: 8987f26wH16
Comment on attachment 9003957 [details]
Bug 1483017 - ensure Sync is initialized before disconnecting. r?eoger

Thom Chiovoloni [:tcsc] has approved the revision.
Attachment #9003957 - Flags: review+

Comment 3

7 months ago
Pushed by mhammond@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/66fb08227855
ensure Sync is initialized before disconnecting. r=tcsc

Comment 4

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/66fb08227855
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox63: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
What's the severity of this issue to the user? Is this something we should consider taking as an Fx62 RC2 ride-along?
Flags: needinfo?(markh)
status-firefox-esr52: --- → unaffected
status-firefox-esr60: --- → unaffected
Keywords: regression
(Assignee)

Comment 6

7 months ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)
> What's the severity of this issue to the user? Is this something we should
> consider taking as an Fx62 RC2 ride-along?

The disconnect feature may not work, so yeah, I think we should uplift - I'll make the request now.
Flags: needinfo?(markh)
(Assignee)

Comment 7

7 months ago
Comment on attachment 9003957 [details]
Bug 1483017 - ensure Sync is initialized before disconnecting. r?eoger

Approval Request Comment
[Feature/Bug causing the regression]: bug 1409208 
[User impact if declined]: Disconnecting from Sync may not work
[Is this code covered by automated tests?]: No
[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]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Limited to the "disconnect from sync" feature
[String changes made/needed]: None
Attachment #9003957 - Flags: approval-mozilla-beta?
Comment on attachment 9003957 [details]
Bug 1483017 - ensure Sync is initialized before disconnecting. r?eoger

Sounds like a pretty critical issue for a new feature shipping in 62. Approved for Fx62 RC2.
Attachment #9003957 - Flags: approval-mozilla-beta? → approval-mozilla-release+
Flags: qe-verify+

Comment 9

7 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-release/rev/4493cbfb68bb
status-firefox62: affected → fixed
Hi Edouard, I've tried to reproduce this bug on an affected build 62.0b17 (BuildID 20180813190114).

STR I've used:
1.I've launched Firefox with 2 different profiles.
2.I've connected to fx account on both of them.
3.From the second profile I've changed the account password.
4.Restart the first profile and then disconnect.

Unfortunately, I'm not seeing any error message triggered in the browser console.

Since I can't reproduce this issue can you please help us verify it?
Flags: qe-verify+ → needinfo?(eoger)
(Assignee)

Comment 11

7 months ago
(In reply to Maria Berlinger [:maria_berlinger], Release Desktop QA from comment #10)
> Hi Edouard, I've tried to reproduce this bug on an affected build 62.0b17
> (BuildID 20180813190114).
> 
> STR I've used:
> 1.I've launched Firefox with 2 different profiles.
> 2.I've connected to fx account on both of them.
> 3.From the second profile I've changed the account password.
> 4.Restart the first profile and then disconnect.

At this point, try waiting for the first profile to enter a "please reconnect" state in the hamburger menu and about:preferences. Then restart, then immediately disconnect without selecting any of the "sanitize" checkboxes.
Flags: needinfo?(eoger)
(Assignee)

Comment 12

7 months ago
(tbh though, I'm not sure it's worth trying to repro the original bug. IMO a regression test is fine)
You need to log in before you can comment on or make changes to this bug.