1191067 - Add Fennec about:accounts page

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Nick Alexander :nalexander [he/him]]

The general feeling from myself and ckarlof is that it is safer to listen to WebChannel messages only from fxa-content-server endpoints embedded in about:accounts.  (The alternate is to listen to WebChannel messages from the known fxa-content-server endpoint no matter how the web content is opened.)

This ticket tracks:
* adding Gecko preferences for the relevant servers:
- auth
- content
- profile
- token
* adding about:accounts chrome UI, like on Desktop;
* handling offline access gracefully;
* registering the WebChannel(s), and ignoring non-about:accounts tabs.

Comment 1

[Ryan Kelly [:rfkelly]]

I like safer, but IIUC this could limit our ability to do useful things in the future, like provide smooth passwordless login on accounts.firefox.com when you're signed in to sync on the device.  Would you consider support for WebChanels from non-about:accounts tabs in some future release, once we have time to get more experience with the implementation?

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Ryan Kelly [:rfkelly] from comment #1)
> I like safer, but IIUC this could limit our ability to do useful things in
> the future, like provide smooth passwordless login on accounts.firefox.com
> when you're signed in to sync on the device.  Would you consider support for
> WebChanels from non-about:accounts tabs in some future release, once we have
> time to get more experience with the implementation?

Yes.  I have questions for markh and vladikoff in this area, actually, so let me ask them here:

* Can you confirm that Firefox Account profile messages are processed from any tab that happens to send them?  (Not just about:accounts tabs.)

* I'm not really a Gecko expert.  Can you suggest the right way to verify that a WebChannel message originates from an about:accounts tab?  (Desktop will need this if and when it transitions to an about:accounts implementation brokered by WebChannels.)

And a lamentation: about:accounts is a non-trivial amount of code to implement and maintain.  I believe it's required for safety and security, and to handle some edge cases that are important on mobile, namely when the device is offline.  It's a pity that the more general WebChannels scheme doesn't quite hit all the edges.

Comment 3

[Ryan Kelly [:rfkelly]]

> It's a pity that the more general WebChannels scheme doesn't quite hit all the edges.

Should we consider expanding it so that it does hit those edges?

Comment 4

[Mark Hammond [:markh] [:mhammond]]

(In reply to Nick Alexander :nalexander from comment #0)
> The general feeling from myself and ckarlof is that it is safer to listen to
> WebChannel messages only from fxa-content-server endpoints embedded in
> about:accounts.  (The alternate is to listen to WebChannel messages from the
> known fxa-content-server endpoint no matter how the web content is opened.)

We already do this "alternative" as we want to support notifications from the user changing their profile directly on the web and not just via about:accounts.

> Yes.  I have questions for markh and vladikoff in this area, actually, so
> let me ask them here:

IIRC, stomlinson was more involved in the webchannel stuff, so I'll clear the ni? for Vlad and Shane can chime in if he feels it necessary.

> * Can you confirm that Firefox Account profile messages are processed from
> any tab that happens to send them?  (Not just about:accounts tabs.)

Yep.

> * I'm not really a Gecko expert.  Can you suggest the right way to verify
> that a WebChannel message originates from an about:accounts tab?

The Fxa webchannel listener function at https://dxr.mozilla.org/mozilla-central/source/services/fxaccounts/FxAccountsWebChannel.jsm#135 has a "sendingContent" param, which includes the browser element - so we could (hopefully ;) check the URL for this browser is about:accounts. You will notice this already supports a "login" command - presumably we'd want to either pass that context to the login helper or check it ourselves before calling it. Note however that this would be somewhat unprecedented (all other channel users just trust the origin) and possibly hacky (that param is really supposed to be treated as an opaque "context" blob for passing back into the channel implementation rather than assuming what it holds)

> (Desktop
> will need this if and when it transitions to an about:accounts
> implementation brokered by WebChannels.)

I actually thought we were hoping to get rid of about:accounts completely - part of the reason we have about:accounts is so we can have chrome priv'd code respond to events/messages from the content server - and we get that "for free" with channels.

> And a lamentation: about:accounts is a non-trivial amount of code to
> implement and maintain.  I believe it's required for safety and security,
> and to handle some edge cases that are important on mobile, namely when the
> device is offline.  It's a pity that the more general WebChannels scheme
> doesn't quite hit all the edges.

That "offline" case is tricky and really doesn't fall into what WebChannels do. However, it's possible the browser could grow builtin support for these "special" content pages - the browser already does this for "social" frames - but it's not clear how this could be done generically - but maybe we don't need to be generic and custom support for the content server might be OK. Roughly, it would probably look like https://dxr.mozilla.org/mozilla-central/source/toolkit/components/social/MozSocialAPI.jsm#31 and when we detect a content server page being loaded we'd also add a WebProgress listener to listen for the errors, much like about:accounts just had added.

That doesn't address the "safety and security" concerns, but the browser folks already considers channels safe enough to send, eg, sensitive profile data over to sumo, and IIUC, loop also uses them for FxA auth.

I hope that helps - I'm not sure I answered the questions you were asking though :)

Comment 5

[Shane Tomlinson [:stomlinson]]

(In reply to Nick Alexander :nalexander from comment #0)
> The general feeling from myself and ckarlof is that it is safer to listen to
> WebChannel messages only from fxa-content-server endpoints embedded in
> about:accounts.  (The alternate is to listen to WebChannel messages from the
> known fxa-content-server endpoint no matter how the web content is opened.)

Nick, "safer" implies "safer than something else." Can you state explicitly the 
"something else" and what concerns lie there? My guess is "the current scheme used
by Fx Desktop."

WebChannel messages are only relayed from content->chrome if the origin of the message
is a registered/expected origin. The alternative you mention (listen to WebChannel messages
from the known fxa-content-server endpoint no matter how the web content is opened) is the
exact scheme used on desktop right now. If you open about:config in Fx 40, search for
identity.fxaccounts.remote.webchannel.uri. It should be set to https://accounts.firefox.com/
A WebChannel is created shortly after startup that listens for messages on value registered
for identity.fxaccounts.remote.webchannel.uri.

This scheme allows FxA to be embedded anywhere without the need to modify any channel related code.
As long as FxA WebChannel messages originate on the expected domain, it "just works". This scheme allows us to embed FxA in the new Fx 40 firstrun flow, which is web hosted content. It also makes it trivial to convert about:accounts from the current CustomEvent based scheme to WebChannels (I have done it in a branch). It also means we can ditch about:accounts (as Mark points out above) and embed FxA directly in about:preferences#sync without messing with the channels.

Your questions about ensuring messages originate on about:accounts feels like the wrong question to ask.
The question should be "is this FxA message from a trusted origin." about:accounts here is *not* the trusted origin, nor is it the source of the message, the FxA content server is. about:accounts is only a container for the FxA content server. The FxA content server should be the trusted source. Where the FxA content server is embedded should not matter to the consumer of the message, as long as the message is from a trusted origin. The WebChannel infrastructure takes care of the origin checking for you, so you should not have to duplicate this effort.

Finally, as Mark points out, one of the long term UX goals on desktop was to eliminate about:accounts entirely, and instead, embed FxA in about:preferences#sync. I do not know whether UX still feels this is a goal, and if it is, whether that affects the Fennec story.

Comment 6

[Nick Alexander :nalexander [he/him]]

Caveat: I don't actually want to ship about:accounts in Fennec; I want to be convinced that bare WebChannels are sufficient.  With that said:

(In reply to Shane Tomlinson [:stomlinson] from comment #5)
> (In reply to Nick Alexander :nalexander from comment #0)
> > The general feeling from myself and ckarlof is that it is safer to listen to
> > WebChannel messages only from fxa-content-server endpoints embedded in
> > about:accounts.  (The alternate is to listen to WebChannel messages from the
> > known fxa-content-server endpoint no matter how the web content is opened.)
> 
> Nick, "safer" implies "safer than something else." Can you state explicitly
> the 
> "something else" and what concerns lie there? My guess is "the current
> scheme used
> by Fx Desktop."

I mean that "accepting messages only from accounts.firefox.com embedded in about:accounts" is safer than "accepting messages only from accounts.firefox.com".

There are two axes (at least!) here: the browser needs to trust the "only from accounts.firefox.com" part; and the browser *product* needs to not be confusing to the user.  I think everybody is convinced that WebChannels do the right thing for the first part.  (markh confirms in his response above.)

I'm concerned that it's easy to maliciously get a user to go to accounts.firefox.com (link!) but hard to get a user to maliciously go to about:accounts (tap this menu, go to this option, ...).  That's a potential security concern.

I'm also concerned that it's easy to understand that about:accounts is special and different from accounts.firefox.com, but it's *not* easy to understand that accounts.firefox.com/?context=fx_fennec_v1 is special and different than accounts.firefox.com.

(If Desktop has plans to address this, I'd love to hear them.  I've heard talk about embedding accounts.firefox.com into the prefs screen, which I think would set the expectation nicely.  Unfortunately, I don't think that's an option in Fennec.)

> WebChannel messages are only relayed from content->chrome if the origin of
> the message
> is a registered/expected origin. The alternative you mention (listen to
> WebChannel messages
> from the known fxa-content-server endpoint no matter how the web content is
> opened) is the
> exact scheme used on desktop right now. If you open about:config in Fx 40,
> search for
> identity.fxaccounts.remote.webchannel.uri. It should be set to
> https://accounts.firefox.com/
> A WebChannel is created shortly after startup that listens for messages on
> value registered
> for identity.fxaccounts.remote.webchannel.uri.
> 
> This scheme allows FxA to be embedded anywhere without the need to modify
> any channel related code.
> As long as FxA WebChannel messages originate on the expected domain, it
> "just works". This scheme allows us to embed FxA in the new Fx 40 firstrun
> flow, which is web hosted content. It also makes it trivial to convert
> about:accounts from the current CustomEvent based scheme to WebChannels (I
> have done it in a branch). It also means we can ditch about:accounts (as
> Mark points out above) and embed FxA directly in about:preferences#sync
> without messing with the channels.
> 
> Your questions about ensuring messages originate on about:accounts feels
> like the wrong question to ask.
> The question should be "is this FxA message from a trusted origin."
> about:accounts here is *not* the trusted origin, nor is it the source of the
> message, the FxA content server is. about:accounts is only a container for
> the FxA content server. The FxA content server should be the trusted source.
> Where the FxA content server is embedded should not matter to the consumer
> of the message, as long as the message is from a trusted origin. The
> WebChannel infrastructure takes care of the origin checking for you, so you
> should not have to duplicate this effort.
> 
> Finally, as Mark points out, one of the long term UX goals on desktop was to
> eliminate about:accounts entirely, and instead, embed FxA in
> about:preferences#sync. I do not know whether UX still feels this is a goal,
> and if it is, whether that affects the Fennec story.

I'm clear on the distinction about message origin; you and I agree on the technical level.  What bare WebChannels don't capture is *user intent*.  about:accounts and embedding FxA into about:preferences#sync does capture that intent.  But due to Fennec technical limitations, we can't do this type of embedding.

Comment 7

[Nick Alexander :nalexander [he/him]]

markh: thanks for the detailed response.  I'm pleased that Desktop has blazed the "trust WebChannels everywhere" ground here.

I think the next action here is for me to communicate with rfeeley about the "bare WebChannels" experience on Fennec.

rfeeley: Fennec would like to skip the about:accounts state and implement what I call "bare WebChannels" directly.  That is, when a Fennec user elects to create a Firefox Account they are sent to a fresh tab with account.firefox.com/?context=fx_fennec_v1.  When the account flow is complete, the browser recognizes the login and starts Sync, perhaps configures Hello or other services, etc.  The tab automatically navigates to the email verification resend page, or to /settings if the account is already verified.

rfeeley, this flow does not make it particularly clear that the new tab is special.  For example, if the user returns to just "accounts.firefox.com" (no context parameter) then they won't be controlling their account any longer.  I think that's acceptable, at least on day 1.  Can you confirm, and if possible comment on what mobile specific thinking you've captured?  (This flow should be similar to a Desktop non-about:accounts backed flow, if you've specced such a thing.)

antlam, I need UX guidance on how the browser should message to the user that Syncing is started.  The context is that an accounts.firefox.com web page is shown, the user has signed up, the browser has recognized the sign up, and created a Firefox Account.  We could show a chrome-level alert; a drop-down of some sort; or an informative system notification.  We could also go to the Firefox Account status activity, or ...  My preference is to show a system notification.

Comment 8

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Nick Alexander :nalexander from comment #7)
> antlam, I need UX guidance on how the browser should message to the user
> that Syncing is started.  

Sounds similar to the "your stuff is loading" for logins we did in bug 1155345.

> The context is that an accounts.firefox.com web
> page is shown, the user has signed up, the browser has recognized the sign
> up, and created a Firefox Account.  

This is great, thanks for the context! I think after these chain of events would be a good spot to place this "loading" image/notification.

> We could show a chrome-level alert; a
> drop-down of some sort; or an informative system notification.  We could
> also go to the Firefox Account status activity, or ...  My preference is to
> show a system notification.

How long do you expect this to persist or "be loading"? I think a system notification makes sense (almost like when you're downloading a file). Could it potentially dismiss itself once completed?

Comment 9

[Chris Karlof [:ckarlof]]

Nick, I generally agree with your sentiment here in Comment 6: Both the about:accounts approach or WebChannel approach should be secure, but in the crazy world of the malicious Web, about:accounts feels safer if someone is trying to trick the user in some way. 

The ship has already sailed for using WebChannels for login to Sync in Desktop (e.g., the new first run flow uses them).

It's worth pointing out that WebChannels do allow us to better capture user intent. E.g., our chrome can create a unique random channel name which is passed to account pages that use WebChannels for login and only accept login messages (or possibly all messages) from account pages over this unique channel name. This would at least make it harder for adversaries to link to account pages that could somehow trick the user into logging in or whatnot. This is how the Hello login works, but I don't believe it's how the first run login flow works though. Nick it might be worth exploring this approach on our mobile implementations if you think it's worthwhile.

Comment 10

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

Bug 1191067 - Pre: Don't include services/fxaccounts in Fennec. r?markh

This also reveals the underlying exception when there is a failure in
a WebChannel callback.

Comment 11

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1191067 - Part 2: Add "pass-through to web" versions of native account activities. r?sebastian

Bug 1191067 - Part 1: Add MOZ_ANDROID_NATIVE_ACCOUNT_UI build flag. r?glandium

Currently, all versions of Firefox run with the existing native
Firefox Account UI.  This flag will opt-in to maintaining that
experience while we transition to a web account UI.  Once we're stable
on the web, we'll remove this flag entirely.

Comment 12

[Nick Alexander :nalexander [he/him]]

(In reply to Chris Karlof [:ckarlof] from comment #9)
> Nick, I generally agree with your sentiment here in Comment 6: Both the
> about:accounts approach or WebChannel approach should be secure, but in the
> crazy world of the malicious Web, about:accounts feels safer if someone is
> trying to trick the user in some way. 

Aye.  It also provides an indirection point for Fennec, clearly delineating "Gecko-side" from "Java-side".  That is, Java-side can always just say "go to about:accounts" and have the Gecko-side work out the right endpoints, etc.  That's a small win for self-hosters.  (This isn't a strong point in favour of about:accounts, just a small benefit.)

> The ship has already sailed for using WebChannels for login to Sync in
> Desktop (e.g., the new first run flow uses them).

This is a good thing, actually -- I trust the Desktop browser engineers to get the security of the feature right.

> It's worth pointing out that WebChannels do allow us to better capture user
> intent. E.g., our chrome can create a unique random channel name which is
> passed to account pages that use WebChannels for login and only accept login
> messages (or possibly all messages) from account pages over this unique
> channel name. This would at least make it harder for adversaries to link to
> account pages that could somehow trick the user into logging in or whatnot.
> This is how the Hello login works, but I don't believe it's how the first
> run login flow works though. Nick it might be worth exploring this approach
> on our mobile implementations if you think it's worthwhile.

Hmm, interesting.  To be truly valuable, the ID needs to be per about:accounts browser tab, I think; which is somewhat contrary to what stomlinson and markh assert.

I think I'm going to implement bare WebChannels, in order to hammer out a few UX details and to get the new messages in place; and then we may choose to follow with an about:accounts implementation on top.

Comment 13

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8650519 [details]
MozReview Request: Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

https://reviewboard.mozilla.org/r/16631/#review14967

Ship It!

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8650520 [details]
MozReview Request: Bug 1191067 - Part 2: Add "pass-through to web" versions of native account activities. r?sebastian

https://reviewboard.mozilla.org/r/16633/#review14985

Ship It!

::: mobile/android/base/moz.build:791
(Diff revision 1)
> -            'MOZ_ANDROID_TAB_QUEUE', 'MOZ_ANDROID_FIREFOX_ACCOUNT_PROFILES'):
> +            'MOZ_ANDROID_TAB_QUEUE', 'MOZ_ANDROID_FIREFOX_ACCOUNT_PROFILES', 'MOZ_ANDROID_NATIVE_ACCOUNT_UI'):

You're AC_DEFINE'ing it, so you don't need this.

Comment 15

[Nick Alexander :nalexander [he/him]]

url:        https://hg.mozilla.org/integration/fx-team/rev/c313396de663d4e33b9301090cd151873980cb09
changeset:  c313396de663d4e33b9301090cd151873980cb09
user:       Nick Alexander <[e-mail]>
date:       Wed Aug 19 16:44:09 2015 -0700
description:
Bug 1191067 - Pre: Don't include services/fxaccounts in Fennec. r=markh

This also reveals the underlying exception when there is a failure in
a WebChannel callback.

url:        https://hg.mozilla.org/integration/fx-team/rev/85e65549172db05871dbe39c2e33c845fc3ed10b
changeset:  85e65549172db05871dbe39c2e33c845fc3ed10b
user:       Nick Alexander <[e-mail]>
date:       Tue Aug 04 16:54:00 2015 -0700
description:
Bug 1191067 - Pre: Add MOZ_ANDROID_NATIVE_ACCOUNT_UI build flag. r=glandium

Currently, all versions of Firefox run with the existing native
Firefox Account UI.  This flag will opt-in to maintaining that
experience while we transition to a web account UI.  Once we're stable
on the web, we'll remove this flag entirely.

Comment 16

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8650519 [details]
MozReview Request: Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

Bug 1191067 - Part 1: Redirect native account UI launches through action intent filters. r?sebastian

This patch stops referring to package/class objects to identify
Android components directly and instead launches through action intent
filters.  The intent filters are scoped to the package, but not marked
as private or as requiring a permission.  A malicious package could
inject itself into an account flow, but I don't think there's much
advantage: the only time a secret is passed between activities is when
the native sign up (CreateAccount) and sign in (SignIn) activities
link between themselves, and in this instance I didn't route through
the action intent filters.  (This is entirely native -- there's no web
analog -- so I didn't use the indirection.)

Comment 17

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8650520 [details]
MozReview Request: Bug 1191067 - Part 2: Add "pass-through to web" versions of native account activities. r?sebastian

Bug 1191067 - Part 2: Add "pass-through to web" versions of native account activities. r?sebastian

Right now, this hard-codes the content server endpoint in Java.
That's not going to last; I just want to show how this approach will
work.

Comment 18

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1191067 - Part 3: Extract Accounts: messages to AccountsHelper. r?sebastian

Bug 1191067 - Part 3: Extract Accounts: messages to AccountsHelper. r?sebastian

There are enough Accounts: messages to separate them from BrowserApp,
and the list is only growing.

This has also the small advantage of removing some non-native event
listeners.

Comment 19

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1191067 - Part 4: Add Accounts:UpdateAccountFromJSON message. r?sebastian

Bug 1191067 - Part 4: Add Accounts:UpdateAccountFromJSON message. r?sebastian

This commit does a few things.  First, it fixes a typo
(s/ForResponse/ForResult/).  It's not clear how this /ever/ worked,
but it did.

Second, it adds an UpdateAccountFromJSON sibling to
CreateAccountFromJSON.  It would have been reasonable to have the
create message do double-duty and update an existing account (we have
the latitude to change the meaning since this API is not yet public)
but I generally prefer each consumer to perform the conditional state
check and to act appropriately.

Third, it generalizes the existing Accounts:Exist message to provide
some details (including email and UID) of any existing Firefox
Account.  The Accounts.exist() API /is/ public, so I introduce a new
(not yet public) API for this richer information.

Comment 20

[Nick Alexander :nalexander [he/him]]

sebastian: here are the first few commits of this sequence.  The next few build out more of the web functionality:

* registering for a WebChannel from the account content server;
* getting messages back and forth via the WebChannel;
* updating the UX around signing up for an account.

I'm aware that you don't have much context on how this all works, so ask questions as needed!  (I'm also aware that we don't have testing for most of this -- all I can say is that testing Android Accounts and SyncAdapters is hard.  It might get easier with in-content UX.)

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c313396de663
https://hg.mozilla.org/mozilla-central/rev/85e65549172d

Comment 22

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8650519 [details]
MozReview Request: Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

https://reviewboard.mozilla.org/r/16631/#review15117

Ship It!

Comment 23

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8650520 [details]
MozReview Request: Bug 1191067 - Part 2: Add "pass-through to web" versions of native account activities. r?sebastian

https://reviewboard.mozilla.org/r/16633/#review15119

::: mobile/android/base/fxa/activities/FxAccountAbstractWebFlowActivity.java:17
(Diff revision 2)
> +public class FxAccountAbstractWebFlowActivity extends FxAccountAbstractActivity {

NIT: The name of the class contains 'abstract' but the class is not defined as 'abstract'. I guess it's only syntactic sugar at this point.

::: mobile/android/base/fxa/activities/FxAccountAbstractWebFlowActivity.java:20
(Diff revision 2)
> +    public final String endpoint;

I wonder why this endpoint needs to be public?

Comment 24

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8651277 [details]
MozReview Request: Bug 1191067 - Part 3: Extract Accounts: messages to AccountsHelper. r?sebastian

https://reviewboard.mozilla.org/r/16879/#review15333

Ship It!

Comment 25

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8651278 [details]
MozReview Request: Bug 1191067 - Part 4: Add Accounts:UpdateAccountFromJSON message. r?sebastian

https://reviewboard.mozilla.org/r/16881/#review15337

::: mobile/android/base/AccountsHelper.java:127
(Diff revision 1)
> +            } catch (Exception e) {

This is not related to these reviews and patches or this line in particular but I've seen us catching generic exceptions in various places. Is this to ensure the callback is triggered at all costs? I'm always afraid that this will silently catch all kinds of unchecked exceptions (e.g. NullPointerException, ClassCastException, ..) in later iterations. Do we have any guidelines in what situations it's okay to do this? Most projects seem to discourage you from doing this.

Comment 26

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8651278 [details]
MozReview Request: Bug 1191067 - Part 4: Add Accounts:UpdateAccountFromJSON message. r?sebastian

https://reviewboard.mozilla.org/r/16881/#review15339

Ship It!

Comment 27

[Nick Alexander :nalexander [he/him]]

For those following along: I landed Parts 1 through 4 as a pre: ticket: Bug 1204937.

Comment 28

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8650519 [details]
MozReview Request: Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

This is a Fennec version of about:accounts, cribbed largely from
Desktop's implementation.  This implementation serves two purposes:

One, it allows all fxa-content-server pref handling to remain in
Gecko.  Java-side consumers redirect to about:accounts?action=... and
have pref munging and parameter addition (like context=fx_fennec_v1,
etc) handled by about:accounts itself.

Two, it handles network connectivity display and error handling.  When
a request is started, we display an animated spinner.  We transition
smoothly from the spinner to the iframe display if we can, and if not
we hide any network error and offer to retry.  This is more important
in Fennec than it is on Desktop.  This approach agrees with Firefox
for iOS.

Some additional notes:

The spinner to iframe transition uses the WebChannel listener to send
LOADED messages to the appropriate XUL <browser> element.  It's worth
remembering that Fennec's Gecko is single process, so the <browser> in
question is in the same process.  None-the-less, we are close to e10s
safe.

There are four actions: signup/signin/force_reauth, and manage.  The
first three try to produce a LOGIN message.  The last uses the
fxa-content-server to manage the Account settings.  *This is not how
this is arranged on Desktop; Desktop redirects to a new tab, not
wrapped in about:accounts.*

Comment 29

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8650519 [details]
MozReview Request: Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

Great, mozreview screwed the flags.  That's why I haven't gotten reviews.  

mfinkle and/or margaret: markh won't get to this today, since he's .au -- could you take a look today?

sebastian: this fits together many pieces you've been looking at too.

Comment 30

[:Margaret Leibovic]

Comment on attachment 8650519 [details]
MozReview Request: Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

https://reviewboard.mozilla.org/r/16631/#review17703

r+ on a quick sanity check. Disclaimer: I didn't take time to understand this entire JS file.

::: mobile/android/chrome/content/aboutAccounts.js:30
(Diff revision 3)
> -const PREF_SYNC_SHOW_CUSTOMIZATION = "services.sync-setup.ui.showCustomizationDialog";
> +Cu.import("resource://gre/modules/Promise.jsm"); /*global Promise */

Instead of using Promise.jsm, you should use the standardized DOM promises. More info here:
https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Promise.jsm

::: mobile/android/chrome/content/aboutAccounts.js:64
(Diff revision 3)
> +  loadedDeferred = Promise.defer();

This promise syntax is obsolete. You should be using the new Promise() syntax. See: https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Promise.jsm/Deferred

Using the new syntax might actually make this logic easier to follow, but this seems sane to me for now, if you want to address that in a follow-up.

Comment 31

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/580ffa6b36a4

Comment 32

[Nick Alexander :nalexander [he/him]]

markh: I landed this because I really want the strings to be in place; but I'd still very much appreciate your comments.  (Their was some mozreview fail preventing me flagging you earlier.)

Comment 33

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/580ffa6b36a4

Comment 34

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8650519 [details]
MozReview Request: Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

https://reviewboard.mozilla.org/r/16631/#review17795

::: mobile/android/chrome/content/aboutAccounts.js:62
(Diff revision 3)
> +function deferTransitionToRemoteAfterLoaded() {

It looks like it is possible a promise created earlier will never resolve, which probably isn't a huge deal, but it might be cleaner to .reject() any existing promise before you overwrite it with a new one.

::: mobile/android/chrome/content/aboutAccounts.js:64
(Diff revision 3)
> +  loadedDeferred = Promise.defer();

same thing Margaret said :) But somewhat sadly, this is one case where the new syntax is actually more painful - you'll probably need to make the promise itself and the resolve/reject functions global. Or you could wrap them up into a deferred-like object (which PromiseUtils.jsm exists for, but IMO it's overkill we have a module dedicated to 4 lines of code :)

LGTM - it's a real shame we are copying so much similar code between desktop and android, but that's not something we can avoid in bugs like this.

Comment 35

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8650519 [details]
MozReview Request: Bug 1191067 - Add Fennec version of about:accounts. r?sebastian,markh

https://reviewboard.mozilla.org/r/16631/#review17829

::: mobile/android/chrome/content/aboutAccounts.js:38
(Diff revision 3)
> -function error(msg) {
> +const log = Cu.import("resource://gre/modules/AndroidLog.jsm", {}).AndroidLog.bind("FxAccounts");

Nice, I didn't know about this. That's handy.

::: mobile/android/chrome/content/aboutAccounts.js:84
(Diff revision 3)
> +    

NIT: white spaces

Comment 36

[Mark Hammond [:markh] [:mhammond]]

reviewboard didn't clear needinfo