[Email] Avoid double New Account insertion

VERIFIED FIXED

Status

Firefox OS
Gaia::E-Mail
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: jrburke, Assigned: jrburke)

Tracking

({regression})

unspecified
x86
Mac OS X
regression

Firefox Tracking Flags

(blocking-b2g:leo+, b2g18 verified)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
I would also like this change to be considered as part of the changes for Bug 860605
(Assignee)

Comment 2

5 years ago
Created attachment 744791 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9518

Pointer to Github pull-request
(Assignee)

Comment 3

5 years ago
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.
Attachment #744791 - Flags: review?(bugmail)
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!
Attachment #744791 - Flags: review?(bugmail) → review+
Oh, and I did test hasCard by running it with the setup card args immediately after the push and it seemed happy.
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.
Assignee: nobody → jrburke
Blocks: 860605
Status: NEW → ASSIGNED
blocking-b2g: --- → leo?
(Assignee)

Comment 7

5 years ago
Fixed, merged to Gaia master.

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

Commit:
https://github.com/mozilla-b2g/gaia/commit/bb459a90a9395ec4f6862ff72f87d17d50602876
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
blocking-b2g: leo? → leo+
Keywords: regression
Uplifted bb459a90a9395ec4f6862ff72f87d17d50602876 to:
v1-train: 98b747f3bd5853bd5ee0dd86a6080a0b8730170a
status-b2g18: --- → fixed
No longer blocks: 860605
verified fixed on Leo with:

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/0c71cbc5fe0c
Gaia   a7b0810580afc734f3d5e441914fe895f9c1923e
BuildID 20130508230207
Version 18.0
Status: RESOLVED → VERIFIED
status-b2g18: fixed → verified
You need to log in before you can comment on or make changes to this bug.