In SessionStore.restoreTab() we use a TabState.flush() call to ensure we properly ignore stale messages. We should be able to use browser epochs here to achieve the same. I started looking at this because I hoped it would have a big impact on e10s startup performance but the sync handler is actually only called for tabs that are ready and have a docShell. At startup those are only the selected tab in each window and removing the flush() call didn't make any difference with even a big session.

This patch mostly moves keeping track of the current epoch from ContentRestore.jsm to content-sessionStore.js as we want to use not only when restoring state into a tab. Unless the previous implementation it doesn't just forget the epoch after restoring. A fresh tab starts with epoch=0 and might be told by the parent that a new one has started.

This is prep work for patch #3 and introduces a few assertions. I used that to ensure the existing functionality is only used as intentioned. It doesn't hurt to leave them in, I would ideally like to introduce a few more fatal assertions in places where it makes sense to confirm a few invariants.

Small addition.

This test uses epochs to split a tab's life into sections and enable us to ignore updates from previous epochs whenever we throw away a tab's state. It adds NOEPOCH_MESSAGES, a set that contains all messages that are okay to be sent without specifying an epoch as they're not sending data or changing state. _browserEpochs is now a map that holds the current epoch for all browsers, if it doesn't have a key yet then we'll default to zero - the initial epoch of any tab. receiveMessage() checks the tab's epoch now at the top when receiving a message. If the message is not in NOEPOCH_MESSAGES and has an epoch other than the current one we will discard it. It removes the TabState.flush() call from SessionStore.restoreTab() and replaces that with starting a new epoch for the given tab.

(In reply to Tim Taubert [:ttaubert] from comment #4) > This test uses epochs to split a tab's life into sections and enable us to Meant to write "this patch".

Same patch as 0003, but without whitespace changes.

Comment on attachment 8602073 [details] [diff] [review] 0001-Bug-1161928-Move-epoch-handling-from-ContentRestore..patch Review of attachment 8602073 [details] [diff] [review]: ----------------------------------------------------------------- Can you fix the comment at the top of the file that refers to epoch?

Comment on attachment 8602087 [details] [diff] [review] 0002-Bug-1161928-Add-assertions-to-ensure-tab-restoration.patch, v2 Review of attachment 8602087 [details] [diff] [review]: ----------------------------------------------------------------- I love the use of assertions! Do you know for sure that NS_ASSERT will cause tests to turn orange?

(In reply to Bill McCloskey (:billm) from comment #7) > Can you fix the comment at the top of the file that refers to epoch? Will do. (In reply to Bill McCloskey (:billm) from comment #8) > I love the use of assertions! Do you know for sure that NS_ASSERT will cause > tests to turn orange? Yes! Tried with NS_ASSERT(false) and that indeed makes the test fail.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e2b6db739d7

https://hg.mozilla.org/integration/fx-team/rev/e9e3afc65b9a https://hg.mozilla.org/integration/fx-team/rev/360d53ac1556 https://hg.mozilla.org/integration/fx-team/rev/133c21f8ab74

https://hg.mozilla.org/mozilla-central/rev/e9e3afc65b9a https://hg.mozilla.org/mozilla-central/rev/360d53ac1556 https://hg.mozilla.org/mozilla-central/rev/133c21f8ab74