897833 - [email] UI: Card loading waitForData logic for message_list card waits too long; should resolve after cached batch or indication of no cached data

Status: RESOLVED WONTFIX

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

Description

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

I'm seeing bad fallout from waiting for onDataInserted at e-mail startup because of a deterministic-but-race-dependent busted server interaction in bug 873052.  The manifestation of this can be very bad depending on the level of bustage.  When you start the app, we can still show the account setup UI even though you created an account last time (that we failed to complete our initial sync on.)  And then once a sync does complete, the message list can be unresponsive because it is waiting for the refresh of the batched headers.

Naively reading the message_list.js code, it seems like the messagesSlice.atTop check at the bottom of onMessagesSplice should be saving us this problem in the batched-add case (but not in the initial sync case, because we fast-path out at the top if howMany === 0), but I am rather thinking atTop is not getting set until the refresh actually completes based on what I am seeing.

This could explain why our initial responsiveness is still less than we would want it.


Right now, my (hungry, so it's acting on instinct) gut is telling me that what we really want is for reasonably explicit slice semantics to indicate to us once the slice has provided us with actual data.  I think the 'status' might be the right way to convey that.  Basically, I think the sequence should look like:
1) 'queued': This is sent immediately when we actually create the slice on the back-end.  The semantics are that the synchronization has not yet started because of mutex reasons.  Right now we send 'synchronizing' which is very misleading.
2) 'synchronizing': We have the mutex and are actually synchronizing.  You would receive this in the same sliceSplice notification that gives you the batched offline messages.  If there were no messages because it was an empty folder or this is an initial sync, we would still send the 'synchronizing' notification.
3) 'synchronized' / 'syncfailed' / 'syncblocked'.  We're not synchronizing because we finished or failed.


James, does this sound like a sane strategy to you?  Then we can lose that .atTop check at the bottom of the list which seems a bit sketchy anyways.
3)

Comment 1

[James Burke [:jrburke]]

There is a _notifyDataInserted() call in onStatusChange at the moment:

https://github.com/mozilla-b2g/gaia/blob/80f8e8f733c837ab5c3bd1cccae82df9000446da/apps/email/js/cards/message_list.js#L508

so hopefully that helps in the "synced, but no messages" case.

For the first case where the sync did not finish on time, and our cache is still showing the "New Account" state, I can see the case for working something out so that at least an empty message list is saved.

In general, I like the idea of going off a specific state flag instead of atTop, although for message_list, I think it is important that we do save the most recent messages, which are an atTop-like state. For example, I am concerned that the status of synchronized might fire, but our slice is not at the top, so it would not be data that we want cached.

As for the responsiveness during subsequent cache restores, the changes in 892069 remove onDataInserted and waitForData, and just use the DOM cache as a starting point, enabling UI actions as soon as the Card JS instance is bound to the DOM. 

So if it looks like the general approach in 892069 may land, I would probably focus this ticket on:

1) confirming that the status flag would work well with always caching the most recent messages (my impression though is that we still may need an atTop check).

2) For the case where account is configured, but sync does not finish and the app backend freaks out, perhaps use some sort of state to at least start out with the saving the empty message_list card so that on cold start restore we at least start with the right card.

For #2, I used to pass a isStartupCard (removed in 892069 currently) to indicate a startup card, and if that was true, message_list could go ahead and call cache save on init.

Comment 2

[BMO Automation]

Firefox OS is not being worked on