1220906 - Delete Old Sync Android Account type

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, defect)

Description

[Nick Alexander :nalexander [he/him]]

Messaging in Bug 1220904, actual removal of the Android Account type here.

Comment 1

[Nick Alexander :nalexander [he/him]]

This should track 46.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

This won't make 46 if we wait for a contributor...

Comment 3

[Craig Arno]

Will those of us who are still using Sync 1.1 (Weave) with our own custom servers be able to turn off Firefox updates so we don't lose the Sync 1.1 client service?

Comment 4

[Vivek Balakrishnan[:vivek]]

Attachment: MozReview Request: Bug 1220906 : Delete deprecated Sync Android Account type r?nalexander

Review commit: https://reviewboard.mozilla.org/r/31259/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31259/

Comment 5

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709105 [details]
MozReview Request: Bug 1220906 : Delete deprecated Sync Android Account type r?nalexander

https://reviewboard.mozilla.org/r/31259/#review28045

This looks good, modulo my one substantive issue.  Please check that we don't change the APK permissions (using `aapt dump badging`, etc).

I'm not comfortable landing this without ripping out all the code consumers.  Can you make this part 1, and get to that?  Start with removing Fennec consumers of `SyncAccounts` and the field https://dxr.mozilla.org/mozilla-central/source/mobile/android/services/src/main/java/org/mozilla/gecko/sync/SyncConstants.java#21.  Then there's lots of Old Sync account and setup code that can go, including the Sync-specific pickler and update code.  I can help with questions here.

Thanks for jumping on this!

::: mobile/android/base/AndroidManifest.xml.in
(Diff revision 1)
> -#include ../services/manifests/SyncAndroidManifest_permissions.xml.in

There's a single permission that needs to be kept from here:

https://dxr.mozilla.org/mozilla-central/source/mobile/android/services/manifests/SyncAndroidManifest_permissions.xml.in#24

Move it into the main manifest (rather than into a services manifest, for example).

I checked the services and activities, and they look clean.  Be careful, though...

Comment 6

[Richard Newman [:rnewman]]

(In reply to Craig Arno from comment #3)
> Will those of us who are still using Sync 1.1 (Weave) with our own custom
> servers be able to turn off Firefox updates so we don't lose the Sync 1.1
> client service?

You can disable automatic updates in Google Play, and you can turn off Firefox's own updates for Nightly and Aurora in Settings.

It's your decision whether you'd rather have performance, stability, and security fixes with Mozilla's Sync setup, do the work to host your own Sync 1.5 service, or stick with Firefox 45 and any bugs and vulnerabilities it has.

Comment 7

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1220906 - Part 1: Delete deprecated Sync Android Account type. r=nalexander

Review commit: https://reviewboard.mozilla.org/r/31331/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31331/

Comment 8

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1220906 - Part 2: Deprecate Accounts.syncAccountsExist. r?rnewman

Review commit: https://reviewboard.mozilla.org/r/31333/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31333/

Comment 9

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1220906 - Part 3: Remove references to SyncAccounts and SyncConstants.ACCOUNTTYPE_SYNC outside of *.sync.*. r?rnewman

Review commit: https://reviewboard.mozilla.org/r/31335/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31335/

Comment 10

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1220906 - Part 4: Remove tests referencing SyncAccounts. r?rnewman

Review commit: https://reviewboard.mozilla.org/r/31337/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31337/

Comment 11

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1220906 - Part 5: Remove Old Sync setup and JPAKE code. r?rnewman

Review commit: https://reviewboard.mozilla.org/r/31339/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31339/

Comment 12

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1220906 - Part 6: Remove Old Sync configuration code. r?rnewman

Review commit: https://reviewboard.mozilla.org/r/31341/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31341/

Comment 13

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1220906 - Part 7: Miscellaneous purging. r?rnewman

Review commit: https://reviewboard.mozilla.org/r/31343/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31343/

Comment 14

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1220906 - Part 8: Get rid of Old Sync-specific stages. r?rnewman

The two stages are EnsureClusterURLStage (the FxA + Sync cluster URL
is determined directly from the token provided by the token server),
and MigrationSentinelSyncStage.  This allowed to merge the two
{Base}GlobalSessionCallback classes together, as well as removing the
specializing FxAcccountGlobalSession.

Review commit: https://reviewboard.mozilla.org/r/31345/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31345/

Comment 15

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=10efdd7fba39

Comment 16

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=172ce1148cb5

Comment 17

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709222 [details]
MozReview Request: Bug 1220906 - Part 1: Delete deprecated Sync Android Account type. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31331/diff/1-2/

Comment 18

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709223 [details]
MozReview Request: Bug 1220906 - Part 2: Deprecate Accounts.syncAccountsExist. r?rnewman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31333/diff/1-2/

Comment 19

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709224 [details]
MozReview Request: Bug 1220906 - Part 3: Remove references to SyncAccounts and SyncConstants.ACCOUNTTYPE_SYNC outside of *.sync.*. r?rnewman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31335/diff/1-2/

Comment 20

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709225 [details]
MozReview Request: Bug 1220906 - Part 4: Remove tests referencing SyncAccounts. r?rnewman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31337/diff/1-2/

Comment 21

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709226 [details]
MozReview Request: Bug 1220906 - Part 5: Remove Old Sync setup and JPAKE code. r?rnewman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31339/diff/1-2/

Comment 22

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709227 [details]
MozReview Request: Bug 1220906 - Part 6: Remove Old Sync configuration code. r?rnewman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31341/diff/1-2/

Comment 23

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709228 [details]
MozReview Request: Bug 1220906 - Part 7: Miscellaneous purging. r?rnewman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31343/diff/1-2/

Comment 24

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709229 [details]
MozReview Request: Bug 1220906 - Part 8: Get rid of Old Sync-specific stages. r?rnewman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31345/diff/1-2/

Comment 25

[Richard Newman [:rnewman]]

Comment on attachment 8709222 [details]
MozReview Request: Bug 1220906 - Part 1: Delete deprecated Sync Android Account type. r=nalexander

https://reviewboard.mozilla.org/r/31331/#review28239

Comment 26

[Richard Newman [:rnewman]]

Comment on attachment 8709223 [details]
MozReview Request: Bug 1220906 - Part 2: Deprecate Accounts.syncAccountsExist. r?rnewman

https://reviewboard.mozilla.org/r/31333/#review28241

Comment 27

[Richard Newman [:rnewman]]

Comment on attachment 8709224 [details]
MozReview Request: Bug 1220906 - Part 3: Remove references to SyncAccounts and SyncConstants.ACCOUNTTYPE_SYNC outside of *.sync.*. r?rnewman

https://reviewboard.mozilla.org/r/31335/#review28243

::: mobile/android/base/java/org/mozilla/gecko/preferences/SyncPreference.java:103
(Diff revision 2)
>          // Otherwise, launch the FxA "Get started" activity, which will

Fix comment.

::: mobile/android/base/java/org/mozilla/gecko/widget/ActivityChooserModel.java:1313
(Diff revision 2)
> -        if (!FirefoxAccounts.firefoxAccountsExist(mContext) &&
> +        if (!FirefoxAccounts.firefoxAccountsExist(mContext))  {

Nit: double-spaced.

::: mobile/android/services/src/main/java/org/mozilla/gecko/fxa/AccountLoader.java:47
(Diff revision 2)
> -    Account foundAccount = FirefoxAccounts.getFirefoxAccount(context);
> +    return FirefoxAccounts.getFirefoxAccount(context);

Inline `context`.

Comment 28

[Richard Newman [:rnewman]]

Comment on attachment 8709225 [details]
MozReview Request: Bug 1220906 - Part 4: Remove tests referencing SyncAccounts. r?rnewman

https://reviewboard.mozilla.org/r/31337/#review28245

Comment 29

[Richard Newman [:rnewman]]

Comment on attachment 8709226 [details]
MozReview Request: Bug 1220906 - Part 5: Remove Old Sync setup and JPAKE code. r?rnewman

https://reviewboard.mozilla.org/r/31339/#review28247

Comment 30

[Richard Newman [:rnewman]]

Comment on attachment 8709227 [details]
MozReview Request: Bug 1220906 - Part 6: Remove Old Sync configuration code. r?rnewman

https://reviewboard.mozilla.org/r/31341/#review28249

Comment 31

[Richard Newman [:rnewman]]

Comment on attachment 8709228 [details]
MozReview Request: Bug 1220906 - Part 7: Miscellaneous purging. r?rnewman

https://reviewboard.mozilla.org/r/31343/#review28251

Comment 32

[Richard Newman [:rnewman]]

Comment on attachment 8709229 [details]
MozReview Request: Bug 1220906 - Part 8: Get rid of Old Sync-specific stages. r?rnewman

https://reviewboard.mozilla.org/r/31345/#review28253

Comment 33

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/c6f2522efb3d94f8a6916b69df0e68fad0fa1c7c
Bug 1220906 - Part 1: Delete deprecated Sync Android Account type. r=nalexander

https://hg.mozilla.org/integration/fx-team/rev/75423ef3ac7719f7e5557974c3ad5ec725319059
Bug 1220906 - Part 2: Deprecate Accounts.syncAccountsExist. r=rnewman

https://hg.mozilla.org/integration/fx-team/rev/b6aea26e42fb3f98797dfeb5b83737a62a992fdf
Bug 1220906 - Part 3: Remove references to SyncAccounts and SyncConstants.ACCOUNTTYPE_SYNC outside of *.sync.*. r=rnewman

https://hg.mozilla.org/integration/fx-team/rev/b9a94252738efc8c9964366ec14e5f8cdb91f8ad
Bug 1220906 - Part 4: Remove tests referencing SyncAccounts. r=rnewman

https://hg.mozilla.org/integration/fx-team/rev/c9b4b30a42b1eec041008c45fed0f7ec3cefec59
Bug 1220906 - Part 5: Remove Old Sync setup and JPAKE code. r=rnewman

https://hg.mozilla.org/integration/fx-team/rev/0b67a9cbbe6593db6d35c23c8e8fce0d12ff2a34
Bug 1220906 - Part 6: Remove Old Sync configuration code. r=rnewman

https://hg.mozilla.org/integration/fx-team/rev/35cbe50b67460f06c18d3014b4521e1c708a5ef9
Bug 1220906 - Part 7: Miscellaneous purging. r=rnewman

https://hg.mozilla.org/integration/fx-team/rev/57327dc816470c156305f3cb0ac221fd5716d5eb
Bug 1220906 - Part 8: Get rid of Old Sync-specific stages. r=rnewman

Comment 34

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8709222 [details]
MozReview Request: Bug 1220906 - Part 1: Delete deprecated Sync Android Account type. r=nalexander

This is a lightly modified version of vivek's patch, which already had my r+.

Comment 35

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c6f2522efb3d
https://hg.mozilla.org/mozilla-central/rev/75423ef3ac77
https://hg.mozilla.org/mozilla-central/rev/b6aea26e42fb
https://hg.mozilla.org/mozilla-central/rev/b9a94252738e
https://hg.mozilla.org/mozilla-central/rev/c9b4b30a42b1
https://hg.mozilla.org/mozilla-central/rev/0b67a9cbbe65
https://hg.mozilla.org/mozilla-central/rev/35cbe50b6746
https://hg.mozilla.org/mozilla-central/rev/57327dc81647

Comment 36

[Nick Alexander :nalexander [he/him]]

Release Note Request (optional, but appreciated)

[Why is this notable]: removes a large chunk of functionality, which Mozilla has long since deprecated, but which self hosters may still be using.  The number of such self hosters is unknown but very low -- probably less than 10k users.

[Suggested wording]: Removed support for Old Sync accounts based on Firefox Sync 1.1.  Use the new Firefox Account based Sync instead.

[Links (documentation, blog post, etc)]: https://blog.mozilla.org/services/2015/07/31/shutting-down-the-legacy-sync-service/

Comment 37

[Liz Henry (:lizzard) (relman/hg->git project)]

Noted with the link for beta 46.