J Zimbrick Reporter
	Description • 11 years ago

Description:
When trying to add an email, the user is taken to a screen that displays "Setting up account" with a loading symbol. The email app never progresses passed this point.

Repro Steps:
1. Update Buri to master/m-c build 20130923040214.
2. Open the E-Mail app.
3. Enter a valid email address and password.
4. Observe that the E-Mail app does not progress passed the "Setting up account" screen. This leaves the user unable to add an e-mail address.

Actual:
E-Mail app does not progress passed "Setting up account" screen, leaving the user unable to add an e-mail account.

Expected:
E-Mail app progresses passed the "Setting up account" screen quickly and the user is able to access their e-mail accounts.

Environmental Variables
Build ID: 20130923040214
Gecko: http://hg.mozilla.org/mozilla-central/rev/f97307cb4c95
Gaia: 3408cc3f6b190c8cd31832fbb8cd2ae571041f29
Platform Version: 27.0a1 

Notes:
Repro frequency: 100% on four devices.
See attached logcat.

	Jason Smith [:jsmith]
	Comment 1 • 11 years ago

Also - find out if this reproduces on 1.2 as well.

	J Zimbrick Reporter
	Comment 2 • 11 years ago

This issue is not occurring on the latest 1.2 build:

Environmental  Variables:
Build ID: 20130922004001
Gecko: http://hg.mozilla.org/releases/mozilla-aurora/rev/63a505ec015c
Gaia: 7b6147372cbf560744a02be50e0a862a825caef6
Platform Version: 26.0a2

User is able to add and use e-mail accounts on 1.2 builds, 1.3 is blocked.

	Mila Davydova
	Comment 3 • 11 years ago

Regression range: 
Build ID: 20130920040201 - user is able to add an email account, however the email app crashes once user is in Inbox 
Gecko: http://hg.mozilla.org/mozilla-central/rev/59beb1868522
Gaia: 78a3e2509b979bd226f01f557bdf229fd3faa42e
Platform Version: 27.0a1

09/21 and 09/22 builds - unable to open an email app at all. Opening an Email app results in a white screen.
Build ID: 20130921040201
Gecko: http://hg.mozilla.org/mozilla-central/rev/a2c31dc69ab3
Gaia: 37866768b8abda56d089c8fe96ec5298fc1df696
Platform Version: 27.0a1

Build ID: 20130923040214 - Unable to add an email account, E-Mail app does not progress passed "Setting up account" screen.
Gecko: http://hg.mozilla.org/mozilla-central/rev/f97307cb4c95
Gaia: 3408cc3f6b190c8cd31832fbb8cd2ae571041f29
Platform Version: 27.0a1

	Dietrich Ayala (:dietrich)
	Comment 4 • 11 years ago

Wes or Andrew, any idea why this build system change would cause this regression?

	Dietrich Ayala (:dietrich)
	Comment 5 • 11 years ago

Also, do we have unit/integration tests for this yet?

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 6 • 11 years ago

No clue, I'm not a build system person.

	Andrew Sutherland [:asuth] (he/him)
	Comment 7 • 11 years ago

As per IRC, this is the same problem causing the dev-gaia tree closure.  In terms of unit tests, this specific failure as understood seems like the type of thing that should actually be covered by gecko tests for b2g operating conditions.  It would not be surprising if this turns out to be a multi-process bug and/or related to the 'app' protocol and/or related to systemXHR and changes to principles for CSP propagation purposes.

	Andrew Sutherland [:asuth] (he/him)
	Comment 8 • 11 years ago

To elaborate, the python Gaia UI tests already test creating 'real' IMAP accounts where we will perform an XHR local autoconfig probe and likely turned red because of this failure.  (Results are behind a VPN still, I think.)  Our integration tests cover creating a real-ish account but because of localhost mappings in the code layer would not hit the XHR layer or the XML parsing layer, and so would pass.  The layers need to be distinct because we do not let our XML autoconfig entries disable SSL/TLS security, but we do let our hard-coded localhost entries do so.

	James Lal [:lightsofapollo]
	Comment 9 • 11 years ago

The backout of https://bugzilla.mozilla.org/show_bug.cgi?id=886164 should resolve this issue during the next build.

	Deian Stefan
	Comment 10 • 11 years ago

Following up on Andrew's bug 886164 comment 39:

This bug shows up because of these two lines:

> 	diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp
> 	index 3f1e71b..89b6dd2 100644
> 	--- a/content/base/src/nsDocument.cpp
> 	+++ b/content/base/src/nsDocument.cpp
> 	@@ -2681,7 +2681,8 @@ nsDocument::InitCSP(nsIChannel* aChannel)
> 		 if (csp) {
> 			 // Copy into principal
> 			 nsIPrincipal* principal = GetPrincipal();
> 	-    principal->SetCsp(csp);
> 	+    rv = principal->SetCsp(csp);
> 	+    NS_ENSURE_SUCCESS(rv, rv);
> 	 #ifdef PR_LOGGING
> 			 PR_LOG(gCspPRLog, PR_LOG_DEBUG,
> 							("Inserted CSP into principal %p", principal));

The point of failure is when trying to set CSP to a principal (not a
null principal) whose origin is app://email.gaiamobile.org. I can dig
deeper (someone that's working on B2G might be better suited), but
right now I can tell you both the already set and supplied CSP
policies are empty (number of policies is 0).

Regardless of the work on bug 886164, I think this error check should
be here.

	Deian Stefan
	Comment 11 • 11 years ago

The DOMParser uses the subject principal for document loading:
https://mxr.mozilla.org/mozilla-central/source/content/base/src/DOMParser.cpp#395
So, if the subject principal has an already set CSP, then
StartDocumentLoad, which calls InitCSP, will fail.

If we instead use a new principal (created from the subject principal
URI, app id, etc. without setting CSP on it) the loading will succeed.
Of course, this assumes that DOMParser doesn't need to abide by CSP.
I'm assuming this is the case since parsed script elements are not
executable and it seems like the img/style tags don't perform network
requests -- i.e., the parser is "pure". Boris does this sound right?

	Deian Stefan
	Comment 12 • 11 years ago

Needinfo failed.. sorry if duplicating.

	Sid Stamm [:geekboy or :sstamm]
	Comment 13 • 11 years ago

Would it make sense to change InitCSP to just append to the old CSP instance if there already is one (instead of failing)?

Also, Deian, is this related to bug 929172?

	Deian Stefan
	Comment 14 • 11 years ago

(In reply to Sid Stamm [:geekboy or :sstamm] from comment #13)
> Would it make sense to change InitCSP to just append to the old CSP instance
> if there already is one (instead of failing)?

I don't think so. We should be using the CSP associated with the principal to restrict document loading, but I don't think we should be accumulating policies. Arguably in this case it would work fine since the policies are actually empty, but in the general case we may end up with a policy that is overly restricting.

> Also, Deian, is this related to bug 929172?

I looked & played with the stuff in bug 929172; I don't think the two are related. (The b2g stuff in inbound & pythong unit tests actually don't fail for me.)

	Boris Zbarsky [:bzbarsky]
	Comment 15 • 11 years ago

DOMParser doesn't do subresource loads or anything.  However the resulting document MUST use the subject principal, because otherwise the subject might not be able to touch the document the DOMParser produces.

How do we handle this in cases when the principal is inherited from a page with CSP (most simply when it has an about:blank subframe)?

	Jason Smith [:jsmith]
	Comment 16 • 11 years ago

Blocking+ for smoketest regression