1309632 - Review services/fxaccounts/*.jsm for possible dead code to remove

Status: RESOLVED DUPLICATE of bug 1448165

(Firefox :: Sync, defect, P3)

Description

[Michelangelo De Simone [Offline] [:mds]]

During the removal of dom/identity (and mozId) it has been noticed that services/fxaccounts/FxAccountsManager.jsm might be partially (or entirely) obsolete.

As suggested on https://bugzilla.mozilla.org/show_bug.cgi?id=1309030#c10 a review of services/fxaccounts/*.jsm may be needed to assess whether that code is still needed or not.

Comment 2

[Lina Butler [:lina]]

It might be good to wait to do this until after the Sync tests are fixed.

Comment 3

[Lina Butler [:lina]]

I think all we need to do now is remove `FxAccountsManager.jsm`, `ON_FXA_UPDATE_NOTIFICATION`, and maybe some tests. This is a great first bug.

Comment 4

[Hossain Al Ikram [:ikram] (QA Contact)]

I wish to work on this bug, hence assigning myself :)

Comment 5

[Hossain Al Ikram [:ikram] (QA Contact)]

> I think all we need to do now is remove `FxAccountsManager.jsm`,
> `ON_FXA_UPDATE_NOTIFICATION`, and maybe some tests. 
Is there any specific tests that you want to be removed?

Comment 6

[Lina Butler [:lina]]

Looks like `FxAccountsManager.jsm` was removed some time ago, so I think all that's left is removing `ON_FXA_UPDATE_NOTIFICATION`. \o/ I'm not sure if we have any that cover that; do you see any failures if you run `./mach test services/fxaccounts`? If not, feel free to just remove that notification, and comments that mention "FxAccountsManager". Thanks!

Comment 7

[17039]

Attachment: Bug 1309632 - removed ON_FXA_UPDATE_NOTIFICATION line from services/fxaccounts/FxAccountsCommon.js;

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

Comment 8

[Lina Butler [:lina]]

Comment on attachment 8944568 [details]
Bug 1309632 - removed ON_FXA_UPDATE_NOTIFICATION line from services/fxaccounts/FxAccountsCommon.js;

https://reviewboard.mozilla.org/r/214734/#review220388

Thanks!

Comment 9

[Lina Butler [:lina]]

Comment on attachment 8944568 [details]
Bug 1309632 - removed ON_FXA_UPDATE_NOTIFICATION line from services/fxaccounts/FxAccountsCommon.js;

https://reviewboard.mozilla.org/r/214734/#review220392

Actually, please remove https://searchfox.org/mozilla-central/rev/b7e3ec2468d42fa59d86c03ec7afeb209813f1d4/services/fxaccounts/FxAccounts.jsm#1291-1292, and `"ON_FXA_UPDATE_NOTIFICATION"` from https://searchfox.org/mozilla-central/rev/b7e3ec2468d42fa59d86c03ec7afeb209813f1d4/tools/lint/eslint/modules.json#80.

Comment 10

[Edouard Oger [:eoger]]

ON_FXA_UPDATE_NOTIFICATION has been removed by bug 1448165, thanks!