1112552 - Clean up global-scope pollution from browser-fxaccounts.js

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Dão Gottwald [:dao]]

browser/base/content/browser-fxaccounts.js shares the global browser window scope with many other scripts and shouldn't set random variables there.

Therefore, we should:

* change 'XPCOMUtils.defineLazyGetter(this, "FxAccountsCommon", function () {' to 'XPCOMUtils.defineLazyGetter(gFxAccounts, "FxAccountsCommon", function () {'

* adjust code accessing the FxAccountsCommon variable to access this.FxAccountsCommon instead

* change 'XPCOMUtils.defineLazyModuleGetter(this, "fxaMigrator",' to 'XPCOMUtils.defineLazyModuleGetter(gFxAccounts, "fxaMigrator",'

* adjust code accessing the fxaMigrator variable to access this.fxaMigrator instead

* move PREF_SYNC_START_DOORHANGER, DOORHANGER_ACTIVATE_DELAY_MS and SYNC_MIGRATION_NOTIFICATION_TITLE into the gFxAccounts object

Comment 1

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 1 - Clean up browser-fxaccounts.js

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8537976 [details] [diff] [review]
rev 1 - Clean up browser-fxaccounts.js

Looks good, thanks!

Comment 3

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/01f50f0fbda6

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Something in this push caused mass bustage. Backed out.
https://hg.mozilla.org/integration/fx-team/rev/803bc910c45a

Comment 5

[Dão Gottwald [:dao]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #4)
> Something in this push caused mass bustage. Backed out.
> https://hg.mozilla.org/integration/fx-team/rev/803bc910c45a

Looks like this was caused by XPCOMUtils.defineLazyGetter(gFxAccounts... / XPCOMUtils.defineLazyModuleGetter(gFxAccounts... being called before the gFxAccounts object would be declared.

Abdelrhman, can you update the patch to fix that?

Comment 6

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 2 - Clean up browser-fxaccounts.js

Comment 7

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/1dd103323b7c

Comment 8

[Carsten Book [:Tomcat]]

backed out for bc1 test bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=1515169&repo=fx-team

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Can we please verify that this is green on Try before burning the trees for a third time?

Comment 10

[Dão Gottwald [:dao]]

Stupid test fixed.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=57294ae763e1

https://hg.mozilla.org/integration/fx-team/rev/3c7defb79660

Comment 11

[Dão Gottwald [:dao]]

(In reply to Dão Gottwald [:dao] from comment #10)
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=57294ae763e1

Oops, this wasn't actually running the primarily interesting test suite, i.e. browser-chrome.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3c7defb79660