868141 - [Email] Avoid double New Account insertion

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[James Burke [:jrburke]]

With the changes from Bug 864414 I introduced a bug where the New Account card can be inserted twice. While it does not cause a breakage, it affects startup time for the New Account case, and I believe is one of the causes of the variability in the ttl tests.

Comment 1

[James Burke [:jrburke]]

I would also like this change to be considered as part of the changes for Bug 860605

Comment 2

[James Burke [:jrburke]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9518

Pointer to Github pull-request

Comment 3

[James Burke [:jrburke]]

Comment on attachment 744791 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9518

The injectStartCard function tried to inject the New Account card as soon as possible, however the final state of the account is also checked in acctSlice.oncomplete, which is fired using the cached backend data. That oncomplete handler was called before the dynamic load of setup-cards.js completed. Since the New Account card had not been inserted yet, it triggered another call to pushCard was done.

This change now only does the New Account card work in the oncomplete callback.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 744791 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9518

This does seem much safer, but I'm worried that oncachereset can still cause the same problem to occur since it results in a similar control flow path.  I've added a commit that makes hasCard aware of pending async card pushes as well as fixing a scary shadowing of 'args' at https://github.com/mozilla-b2g/gaia/pull/9522

I wanted to do the pull request against your repo, but the github UI is broken or something, because I couldn't find your gaia fork in that list nor would it filter down to it or let me just type it.

Anywho, r=asuth with that change integrated somehow (you can squash it or just merge both together or whatever).

The TTL numbers do come out much more nicely this way.  Thanks!

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Oh, and I did test hasCard by running it with the setup card args immediately after the push and it seemed happy.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

This bug is really part of bug 860605 so we would like its leo applied to this one.  In the interest of making uplifts less confusing, this is its own bug, however.

Comment 7

[James Burke [:jrburke]]

Fixed, merged to Gaia master.

Pull request:
https://github.com/mozilla-b2g/gaia/pull/9518

Commit:
https://github.com/mozilla-b2g/gaia/commit/bb459a90a9395ec4f6862ff72f87d17d50602876

Comment 8

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted bb459a90a9395ec4f6862ff72f87d17d50602876 to:
v1-train: 98b747f3bd5853bd5ee0dd86a6080a0b8730170a

Comment 9

[Tracy Walker [:tracy]]

verified fixed on Leo with:

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/0c71cbc5fe0c
Gaia   a7b0810580afc734f3d5e441914fe895f9c1923e
BuildID 20130508230207
Version 18.0