See https://treeherder.mozilla.org/logviewer?job_id=342228417&repo=autoland&lineNumber=3553 for a failure

I'm looking into this.

To make it easier to debug the issue, I have commented out the rest of the test right away.

Observations so far:

• Right away after running this test for a few times, you might notice that sometimes it passes and sometimes it fails. In cases when it fails, the issue is that tabState.entries[0] is undefined here.
• I added some logging and see that the expected historychange update does indeed happen, but in cases when we have no entries, the update happens too late, after the test has finished, or at least after the assertion. That makes it seem that sometimes the historychange update takes too long. So this was my next question, why does it take longer sometimes?
• I traced down code that initiates the correct historychange update, and it happens to be requestTabStateFlush() that gets called because of await TabStateFlusher.flush(browser); in the test.
• So following the previous theory, how can it be that sometimes this update "takes longer" if we await for this promise? So it looks like the promise that we return from TabStateFlusher.flush is

Promise.race([
      nativePromise.then(_ => requestPromise),
      request.cancelPromise,
    ]);

• I decided to add debug prints to places where might either resolve requestPromise or request.cancelPromise, which was in resolve() and in resolveAll(). The latter is used used for "when the content process crashed or the final update message was seen". When the test failed the next time, I saw that resolveAll has been called first. It was called from onFinalTabStateUpdateComplete , which at some point was triggered by a call here within when message "browser-shutdown-tabstate-updated" was received. This message is sent from WindowGlobalParent::NotifySessionStoreUpdatesComplete, which was called by nsFrameLoader::RequestFinalTabStateFlush (but not invoked by JS code). When I printed out the bc, it was one belonging to the previous navigation, about:blank. I also checked if that bc's current window global was equal to this and it was not (GetBrowsingContext()->GetCurrentWindowGlobal() == this). Both of these things indicate that this is a session store update call that was corresponding to a previous navigation, and is not for the current navigation. The test passes if I add a change within WindowGlobalParent::NotifySessionStoreUpdatesComplete to also check for IsCurrentGlobal here. Going to see if try looks good for other tests as well.

Unfortunately, checking if we check if the wgp is the current window global before sending "browser-shutdown-tabstate-updated", we have incorrect behaviour in cases when are removing a tab and expect pertinent updates to go through, e.g. here.

Maybe we don't need the above check, but instead need to augment resolveAll code (and perhaps other code too) in TabStateFlusher.jsm to take into account that a call to resolveAll for a specific browser might not be made by the latest session store update. Perhaps, when issuing message "browser-shutdown-tabstate-updated", WindowGlobalParent should also send along it's associated browsing context's id, so that in resolveAll we can see if we should ignore the update.

Does bug 1716444 happen to help here?

Fixed by bug 1716444: https://treeherder.mozilla.org/jobs?repo=try&revision=a6fc1edb6fa167fdf0ef424bc736eb4c83b74d76