1437382 - Private tab updates shortly before backgrounding aren't guaranteed to make it into storage

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Session Restore, enhancement, P1)

Description

[Jan Henning [:JanH]]

When we get an application-background event, the session store attempts to flush its data into storage, however while I guess that at some point this might have worked as designed, after a number of refactorings in that area (possibly especially bug 1260243 and bug 1346413) this is certainly no longer guaranteed to happen before we return control from the relevant Android lifecycle events.

For normal tabs this isn't quite as tragic as it seems, because while we could be killed at any moment after returning from onPause(), in practice most of the time we probably still manage to finish writing the new session store file anyway, even while already backgrounded.

For private browsing tabs this is different, though: As we don't want to write them to disk, we store them as part of our savedInstanceState. The current situation means that the data flush triggered by our application-background handling happens too late to update mPrivateBrowsingSession in GeckoApp in time for the onSavedInstanceState callback.
This means that the private browsing data held as part of our savedInstanceState in case of OOM-kills might be up to ~10 seconds out of date, which is enough time for some supposedly closed private tabs to resurface after an OOM background kill.

STR:
1. Turn on "browser.sessionstore.debug_logging" in about:config, restart Firefox and watch logcat for messages from "GeckoSessionStore".
2. Open some private tabs and wait until session store activity has settled down.
3. Trigger a session store write by scrolling the current tab slightly/selecting a different tab and wait until saveState/writeFile handling for this change has completed.
4. Now close some of those private tabs and then immediately hit the home button or switch to some other app.
5. Run "adb shell am kill <Firefox package name>" to simulate an OOM kill.
6. Open Firefox again.
7. The private tabs you closed in step 4.) unexpectedly reappear.

Comment 1

[Jan Henning [:JanH]]

Having looked some more, I think that bug 1260243 actually isn't to blame here and the straw that broke the camel's back was probably bug 1346413 - now we're triggering application-background handling only after/during onStop, which is definitiviely too late for onSaveInstanceState, whereas previously this was kicked off (when things worked) in onPause.

We still had a race condition, though, I think, because our Android onPause-handling *didn't* actually block for Gecko completing its "application-background" work, so depending on how much time there was between onPause and onSaveInstanceState and how soon the session store managed to do its work in response to application-background, this might or might not have worked even previously.

There are two possible ways to solve this: One is to block during onSaveInstanceState until the session store has sent over fresh private tabs data. By making sure that we *only* wait for the private data to be sent over, and *not* waiting for everything else that currently happens as part of application-background, including the session store writing the non-private tabs to disk, this can be made to happen quite quickly, but unfortunately it still introduces the possibility of our UI hanging if Gecko is very busy and takes longer than normally to respond.

The other way is to speed up processing of tab events relating to private tabs within the session store, so they're not subject to the regular write throttling interval of by default 10 s.

Since I think that the most important issue is persisting TabClose events as reliably as possible, I propose the following combined solution:
1. onSaveInstanceState will wait for the session store to send over fresh private tabs data, but only for a limited amount of time, say on the order of 500 ms (and add some telemetry to get some better data on how long we typically have to wait here than just my phone). onSaveInstanceState means we're going into the background, so we can afford to wait a little, but not too much in case the user returns to Firefox immediately or we're only switching to a different activity within Firefox, e.g. our settings. If we have to abort the wait, it means we'll miss private tabs that have navigated or been opened within the last ~ 10 seconds, but...
2. ... to make sure that GeckoApp's mPrivateBrowsingSession is less likely to be out of date in that regard, speed up the processing of TabClose events, so they don't have to wait up to 10 seconds before being transmitted to GeckoApp.

Comment 2

[Jan Henning [:JanH]]

(In reply to Jan Henning [:JanH] from comment #1)
> speed up the processing of TabClose events, so they don't have to wait
> up to 10 seconds before being transmitted to GeckoApp.

That is TabClose events relating to private tabs.

Comment 3

[Jan Henning [:JanH]]

Try run for new Robocop test:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d5435a5312d3c0c84e87ab17bf6ff5f86b649bf7

Comment 4

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 0 - Cleanup imports (and unused variables).

Review commit: https://reviewboard.mozilla.org/r/222842/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222842/

Comment 5

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 1 - Cleanup GeckoActivityMonitor.

Review commit: https://reviewboard.mozilla.org/r/222844/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222844/

Comment 6

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 2 - Allow triggering application-background earlier when possible.

If onActivitySaveInstanceState is called, this normally happens just before
onActivityStopped, which means we can attempt to use the same logic currently
used for onActivityStopped to determine whether the whole app is going into the
background or we're doing just an internal activity switch.

This allows us to trigger our Gecko onPause/"application-background" handling
*before* e.g. GeckoApp's onSaveInstanceState handling, meaning we still have
time to update the private browsing data held in GeckoApp before it is passed
off to the OS.

Review commit: https://reviewboard.mozilla.org/r/222846/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222846/

Comment 7

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 3 - Flush session store tab data separately from application-background.

We need to block onSaveInstanceState until we're sure that our private browsing
data is up to date, however we can't block on the whole of our application-
background handling (GeckoThread.onPause), as that will take too much time.

In addition, we need to update the stored tab data not just when the application
goes into the background, but every time we're leaving GeckoApp and onSave-
InstanceState is called, e.g. when visiting our settings activity.

Review commit: https://reviewboard.mozilla.org/r/222848/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222848/

Comment 8

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time.

There are two parts to this:
1. While the file writing itself can be done either synchronously or asynchro-
   nously, the session store's _writeFile function as a whole always behaves
   asynchronously.
   To make sure that the private tab data held in GeckoApp is updated before
   being passed off to the OS during onSaveInstanceState, we therefore need to
   send it while still executing synchronously within our onApplicationBack-
   ground handling, i.e. before calling _writeFile.
2. Sending the data to Java needs to happen synchronously as well, so we need
   to listen for "PrivateBrowsing:Data" on the Gecko thread now. This in turn
   means that some sychronisation is now required between the UI thread handling
   onSaveInstanceState and the Gecko thread that is actually receiving the data.

To avoid hanging the UI and causing ANRs, we only wait a limited amount of time
for Gecko to respond with fresh private tabs data, though.
As this still leaves a certain possibility of outdated private browsing data
being saved and possibly restored after an OOM-kill, we're also going to speed
up the processing of TabClosed events by the session store in the following
parts.

Review commit: https://reviewboard.mozilla.org/r/222850/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222850/

Comment 9

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 5 - Test that flushing pending session store data sends the expected messages to Java.

We attempt to get the session store into a known state as far as is possible
from Java and then test various situations to check that the expected
"PrivateBrowsing:Data" message is received in each case.

Review commit: https://reviewboard.mozilla.org/r/222852/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222852/

Comment 10

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 6 - Switch to using PrivateBrowsing helper method in session store.

As far as I can tell, that line dates back to approximately the time Private-
BrowsingUtils were introduced in the first place.

Review commit: https://reviewboard.mozilla.org/r/222856/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222856/

Comment 11

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 7 - Change session store time callback behaviour.

When we get a timer callback for delayed saving, we already only proceed if
we've still got a write pending. Conversely if for whatever reasons _saveState()
is called from outside of a timer callback, any still pending save timer is
cancelled again.

With that in mind, when executing the delayed write timer callback there's no
reason to call the public version of saveState(), whose only extra task is to
increment the pending write count again. Instead, we can just directly call the
internal _saveState() version.

Review commit: https://reviewboard.mozilla.org/r/222858/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222858/

Comment 12

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 8 - Extract functions for creating and cancelling the delayed write timer.

Review commit: https://reviewboard.mozilla.org/r/222860/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222860/

Comment 13

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 9 - Track number of outstanding "private tabs only" saveState calls.

Private tabs are saved in memory only by sending them to GeckoApp, so to speed
up processing (compare part 3), we want to avoid writing out the full session
store file for normal tabs as well if all outstanding saveState(Delayed) calls
concerned private tabs only.

To that effect, we slightly change the semantics of our pendingWrites counter
and now increment it each time saveStateDelayed is called, even if the save
timer is already running. This is because if e.g. a private tab update started
the timer and then another saveStateDelayed call happens for a non-private tab,
we need to change our plans and write the normal session store file after all as
well. Tracking every saveStateDelayed call allows us to do this.

Because writeFile only cares about the fact whether additional pending writes
were queued while it was executing asynchronously or not, but not about the
absolute amount of pending writes (if no additional writes were queued, the
count is simply reset to 0), incrementing the pending writes count even
when the timer is already running causes no ill effects.

Review commit: https://reviewboard.mozilla.org/r/222862/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222862/

Comment 14

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 10 - Use a reduced save delay when saving private tabs.

... and also shorten any already running save timer if necessary.

This is because the private tab data kept by GeckoApp, that will be restored if
we are OOM-killed, cannot be updated anymore after Firefox goes into the back-
ground, even if we aren't immediately killed by the OS.

Because during backgrounding the UI only waits a limited amount of time for the
latest private tab data in order to avoid causing an ANR if Gecko is busy, we
need to compensate by sending private tab data updates faster to GeckoApp than
the usual write throttling interval of every 10 s would allow.

To allow multiple successive tab events to be batched together in one update,
e.g. if the user closes *all* private tabs, we still introduce a small save
delay of a few hundred ms.

Review commit: https://reviewboard.mozilla.org/r/222864/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222864/

Comment 15

[Jan Henning [:JanH]]

Attachment: Bug 1437382 - Part 11 - Shorten save delay when private tabs are closed.

In most cases (e.g. new tabs added, page navigation, scrolling, etc.), we can
live with the fact the the private tab data held by GeckoApp might be up to ~10
seconds out of date if we don't manage to send an update within the time limit
given by the UI during backgrounding.

Where the closing of private tabs is concerned, this is different, as not
remembering that the user already closed some tabs just before switching away
from Firefox could lead to potentially embarrassing situations when the user
returns and unexpectedly finds those tabs still open.
Therefore we now use the infrastructure added in the previous parts to speed up
the saving process when private tabs are closed.

Review commit: https://reviewboard.mozilla.org/r/222866/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222866/

Comment 16

[Eugen Sawin [:esawin]]

Comment on attachment 8953737 [details]
Bug 1437382 - Part 6 - Switch to using PrivateBrowsing helper method in session store.

https://reviewboard.mozilla.org/r/222856/#review229162

Comment 17

[Eugen Sawin [:esawin]]

Comment on attachment 8953738 [details]
Bug 1437382 - Part 7 - Change session store time callback behaviour.

https://reviewboard.mozilla.org/r/222858/#review229166

Comment 18

[Eugen Sawin [:esawin]]

Comment on attachment 8953739 [details]
Bug 1437382 - Part 8 - Extract functions for creating and cancelling the delayed write timer.

https://reviewboard.mozilla.org/r/222860/#review229248

Comment 19

[Eugen Sawin [:esawin]]

Comment on attachment 8953739 [details]
Bug 1437382 - Part 8 - Extract functions for creating and cancelling the delayed write timer.

https://reviewboard.mozilla.org/r/222860/#review229254

Comment 20

[Eugen Sawin [:esawin]]

Comment on attachment 8953740 [details]
Bug 1437382 - Part 9 - Track number of outstanding "private tabs only" saveState calls.

https://reviewboard.mozilla.org/r/222862/#review229262

Comment 21

[Eugen Sawin [:esawin]]

Comment on attachment 8953741 [details]
Bug 1437382 - Part 10 - Use a reduced save delay when saving private tabs.

https://reviewboard.mozilla.org/r/222864/#review229276

::: mobile/android/components/SessionStore.js:991
(Diff revision 1)
>        // Interval until the next disk operation is allowed
>        let currentDelay = this._lastSaveTime + this._interval - Date.now();
>  
>        // If we have to wait, set a timer, otherwise saveState directly
> -      let delay = Math.max(currentDelay, MINIMUM_SAVE_DELAY);
> +      let delay = aPrivateTabsOnly ?
> +                    SAVE_INTERVAL_PRIVATE_TABS : Math.max(currentDelay, MINIMUM_SAVE_DELAY);

We (should) a use custom indentation rule for the ternary operator:
<condition>
  ? <yes>
  : <no>

::: mobile/android/components/SessionStore.js:991
(Diff revision 1)
>        // Interval until the next disk operation is allowed
>        let currentDelay = this._lastSaveTime + this._interval - Date.now();
>  
>        // If we have to wait, set a timer, otherwise saveState directly
> -      let delay = Math.max(currentDelay, MINIMUM_SAVE_DELAY);
> +      let delay = aPrivateTabsOnly ?
> +                    SAVE_INTERVAL_PRIVATE_TABS : Math.max(currentDelay, MINIMUM_SAVE_DELAY);

We (should) a use custom indentation rule for the ternary operator:
<condition>
  ? <yes>
  : <no>

::: mobile/android/components/SessionStore.js:991
(Diff revision 1)
>        // Interval until the next disk operation is allowed
>        let currentDelay = this._lastSaveTime + this._interval - Date.now();
>  
>        // If we have to wait, set a timer, otherwise saveState directly
> -      let delay = Math.max(currentDelay, MINIMUM_SAVE_DELAY);
> +      let delay = aPrivateTabsOnly ?
> +                    SAVE_INTERVAL_PRIVATE_TABS : Math.max(currentDelay, MINIMUM_SAVE_DELAY);

We (should) a use custom indentation rule for the ternary operator:
<condition>
  ? <yes>
  : <no>

Comment 22

[Eugen Sawin [:esawin]]

Comment on attachment 8953742 [details]
Bug 1437382 - Part 11 - Shorten save delay when private tabs are closed.

https://reviewboard.mozilla.org/r/222866/#review229280

Comment 23

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8953731 [details]
Bug 1437382 - Part 0 - Cleanup imports (and unused variables).

https://reviewboard.mozilla.org/r/222842/#review229318

Comment 24

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8953732 [details]
Bug 1437382 - Part 1 - Cleanup GeckoActivityMonitor.

https://reviewboard.mozilla.org/r/222844/#review229320

Comment 25

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8953733 [details]
Bug 1437382 - Part 2 - Allow triggering application-background earlier when possible.

https://reviewboard.mozilla.org/r/222846/#review229324

Comment 26

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8953734 [details]
Bug 1437382 - Part 3 - Flush session store tab data separately from application-background.

https://reviewboard.mozilla.org/r/222848/#review229326

Comment 27

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8953735 [details]
Bug 1437382 - Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time.

https://reviewboard.mozilla.org/r/222850/#review229330

::: mobile/android/base/java/org/mozilla/gecko/GeckoApp.java:614
(Diff revision 1)
>      }
>  
>      @Override
>      protected void onSaveInstanceState(Bundle outState) {
> +        synchronized (this) {
> +            mPrivateBrowsingSessionOutdated = true;

If GeckoApplication triggered and _completed_ a flush before this, I think this line here will make us wait unnecessarily?

::: mobile/android/components/SessionStore.js:1088
(Diff revision 1)
> +      log("_saveState() writing normal data, " +
> +           normalData.windows[0].tabs.length + " tabs in window[0]");
> +    } else {
> +      log("_saveState() writing empty normal data");
> +    }
> +    this._writeFile(this._sessionFile, this._sessionFileTemp, normalData, aAsync);

Any reason for moving this to after sending private session?

Comment 28

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8953736 [details]
Bug 1437382 - Part 5 - Test that flushing pending session store data sends the expected messages to Java.

https://reviewboard.mozilla.org/r/222852/#review229344

Comment 29

[Jan Henning [:JanH]]

Comment on attachment 8953735 [details]
Bug 1437382 - Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time.

https://reviewboard.mozilla.org/r/222850/#review229330

> If GeckoApplication triggered and _completed_ a flush before this, I think this line here will make us wait unnecessarily?

GeckoApplication's background handling is triggered by the GeckoActivityMonitor, which in turn is called only *after* we've called super.onSaveInstanceState, as the ActivityLifecycleCallbacks are handled by Android's base Activity class. (I suppose I should add a comment to that effect) So we'll always have triggered a fresh flush for which we're attempting to wait below.

> Any reason for moving this to after sending private session?

When flushing tabs, \_writeFile writes synchronously, which takes time, plus due to the way \_write is structured to support writing both synchronously or asynchronously, unless I'm mistaken we always have to take a detour via the event loop before being able to continue with the part following the .then{... at https://dxr.mozilla.org/mozilla-central/rev/7208b6a7b11c3ed8c87a7f17c9c30a8f9583e791/mobile/android/components/SessionStore.js#1173.

All that time spent writing non-private tabs
- means we're blocking the UI thread longer than necessary
- makes it more likely that the wait time expires and we fail to get the most current private session store data into the savedInstanceState

Hence I've swapped the order of those two operations around.

Comment 30

[Jan Henning [:JanH]]

Comment on attachment 8953734 [details]
Bug 1437382 - Part 3 - Flush session store tab data separately from application-background.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222848/diff/1-2/

Comment 31

[Jan Henning [:JanH]]

Comment on attachment 8953735 [details]
Bug 1437382 - Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222850/diff/1-2/

Comment 32

[Jan Henning [:JanH]]

Comment on attachment 8953736 [details]
Bug 1437382 - Part 5 - Test that flushing pending session store data sends the expected messages to Java.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222852/diff/1-2/

Comment 33

[Jan Henning [:JanH]]

Comment on attachment 8953737 [details]
Bug 1437382 - Part 6 - Switch to using PrivateBrowsing helper method in session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222856/diff/1-2/

Comment 34

[Jan Henning [:JanH]]

Comment on attachment 8953738 [details]
Bug 1437382 - Part 7 - Change session store time callback behaviour.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222858/diff/1-2/

Comment 35

[Jan Henning [:JanH]]

Comment on attachment 8953739 [details]
Bug 1437382 - Part 8 - Extract functions for creating and cancelling the delayed write timer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222860/diff/1-2/

Comment 36

[Jan Henning [:JanH]]

Comment on attachment 8953740 [details]
Bug 1437382 - Part 9 - Track number of outstanding "private tabs only" saveState calls.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222862/diff/1-2/

Comment 37

[Jan Henning [:JanH]]

Comment on attachment 8953741 [details]
Bug 1437382 - Part 10 - Use a reduced save delay when saving private tabs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222864/diff/1-2/

Comment 38

[Jan Henning [:JanH]]

Comment on attachment 8953742 [details]
Bug 1437382 - Part 11 - Shorten save delay when private tabs are closed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222866/diff/1-2/

Comment 39

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8953735 [details]
Bug 1437382 - Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time.

https://reviewboard.mozilla.org/r/222850/#review229330

> GeckoApplication's background handling is triggered by the GeckoActivityMonitor, which in turn is called only *after* we've called super.onSaveInstanceState, as the ActivityLifecycleCallbacks are handled by Android's base Activity class. (I suppose I should add a comment to that effect) So we'll always have triggered a fresh flush for which we're attempting to wait below.

In that case would it make sense to always flush from GeckoApp (and not flush from GeckoApplication?). During a backgrounding event where GeckoApp is not involved, presumably we don't need to deal with session store either?

> When flushing tabs, \_writeFile writes synchronously, which takes time, plus due to the way \_write is structured to support writing both synchronously or asynchronously, unless I'm mistaken we always have to take a detour via the event loop before being able to continue with the part following the .then{... at https://dxr.mozilla.org/mozilla-central/rev/7208b6a7b11c3ed8c87a7f17c9c30a8f9583e791/mobile/android/components/SessionStore.js#1173.
> 
> All that time spent writing non-private tabs
> - means we're blocking the UI thread longer than necessary
> - makes it more likely that the wait time expires and we fail to get the most current private session store data into the savedInstanceState
> 
> Hence I've swapped the order of those two operations around.

Ok that makes sense. Thanks!

Comment 40

[Jan Henning [:JanH]]

Comment on attachment 8953735 [details]
Bug 1437382 - Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time.

https://reviewboard.mozilla.org/r/222850/#review229330

> In that case would it make sense to always flush from GeckoApp (and not flush from GeckoApplication?). During a backgrounding event where GeckoApp is not involved, presumably we don't need to deal with session store either?

In a way yes, you're right - currently the session store only handles full Firefox tabs, which are more or less linked to GeckoApp/BrowserApp. Even if/when some day somebody gets the session store to work with GeckoView as well I suppose the API for that will look differently, with the session store only responsible for collecting and providing the serialised tab data and everything else (including actually saving the data) being left to the GeckoView consumer.

However:
1. When an activity is finishing, onSaveInstanceState won't be called. The GeckoActivityMonitor can simply fall back to dispatching onApplicationBackground() during onStop() instead, but within GeckoApp I'd have to add additional code to trigger a tabs flush during onStop() as well, but only if the activity is finishing.
This scenario is relevant when we're using GeckoApp's finishAndShutdown() method as opposed to messaging browser.js with an "Browser:Quit" message - in the latter case shutdown is initiated by Gecko and the session store will flush pending changes autonomously, whereas in the former case we just finish GeckoApp, rely on our application-background handling to do some minimal cleanup and then just kill our process.
2. Once GeckoApp has been started, the full Firefox tabs opened through it will remain open as long the process running Gecko stays alive, even if the GeckoApp activity is temporarily destroyed while it's in background, either because we're completely in background, or just having a different foreground activity like our settings or anything GeckoView-based. While most probably there won't be anything interesting happening to those tabs while the main browser UI is in background, because of scripts running in background it's not totally excluded. Because the likelihood of us getting killed only depends on whether the whole app is backgrounded or not, I'd still like to flush the current session store state to disk when that happens.

Comment 41

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8953735 [details]
Bug 1437382 - Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time.

https://reviewboard.mozilla.org/r/222850/#review229940

Comment 42

[Jan Henning [:JanH]]

Comment on attachment 8953736 [details]
Bug 1437382 - Part 5 - Test that flushing pending session store data sends the expected messages to Java.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222852/diff/2-3/

Comment 43

[Jan Henning [:JanH]]

Comment on attachment 8953737 [details]
Bug 1437382 - Part 6 - Switch to using PrivateBrowsing helper method in session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222856/diff/2-3/

Comment 44

[Jan Henning [:JanH]]

Comment on attachment 8953738 [details]
Bug 1437382 - Part 7 - Change session store time callback behaviour.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222858/diff/2-3/

Comment 45

[Jan Henning [:JanH]]

Comment on attachment 8953739 [details]
Bug 1437382 - Part 8 - Extract functions for creating and cancelling the delayed write timer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222860/diff/2-3/

Comment 46

[Jan Henning [:JanH]]

Comment on attachment 8953740 [details]
Bug 1437382 - Part 9 - Track number of outstanding "private tabs only" saveState calls.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222862/diff/2-3/

Comment 47

[Jan Henning [:JanH]]

Comment on attachment 8953741 [details]
Bug 1437382 - Part 10 - Use a reduced save delay when saving private tabs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222864/diff/2-3/

Comment 48

[Jan Henning [:JanH]]

Comment on attachment 8953742 [details]
Bug 1437382 - Part 11 - Shorten save delay when private tabs are closed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222866/diff/2-3/

Comment 49

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/04b001dda76a
Part 0 - Cleanup imports (and unused variables). r=jchen
https://hg.mozilla.org/integration/autoland/rev/7eb4043fdaca
Part 1 - Cleanup GeckoActivityMonitor. r=jchen
https://hg.mozilla.org/integration/autoland/rev/9f1acf3452e1
Part 2 - Allow triggering application-background earlier when possible. r=jchen
https://hg.mozilla.org/integration/autoland/rev/50522bf54000
Part 3 - Flush session store tab data separately from application-background. r=jchen
https://hg.mozilla.org/integration/autoland/rev/22531e0df456
Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time. r=jchen
https://hg.mozilla.org/integration/autoland/rev/4b7d24b7ec8b
Part 5 - Test that flushing pending session store data sends the expected messages to Java. r=jchen
https://hg.mozilla.org/integration/autoland/rev/e06211b51624
Part 6 - Switch to using PrivateBrowsing helper method in session store. r=esawin
https://hg.mozilla.org/integration/autoland/rev/6c92b6fac3e3
Part 7 - Change session store time callback behaviour. r=esawin
https://hg.mozilla.org/integration/autoland/rev/41f601758661
Part 8 - Extract functions for creating and cancelling the delayed write timer. r=esawin
https://hg.mozilla.org/integration/autoland/rev/6db1cb600f52
Part 9 - Track number of outstanding "private tabs only" saveState calls. r=esawin
https://hg.mozilla.org/integration/autoland/rev/2f104556b212
Part 10 - Use a reduced save delay when saving private tabs. r=esawin
https://hg.mozilla.org/integration/autoland/rev/6bcb8ed863ab
Part 11 - Shorten save delay when private tabs are closed. r=esawin

Comment 50

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/04b001dda76a
https://hg.mozilla.org/mozilla-central/rev/7eb4043fdaca
https://hg.mozilla.org/mozilla-central/rev/9f1acf3452e1
https://hg.mozilla.org/mozilla-central/rev/50522bf54000
https://hg.mozilla.org/mozilla-central/rev/22531e0df456
https://hg.mozilla.org/mozilla-central/rev/4b7d24b7ec8b
https://hg.mozilla.org/mozilla-central/rev/e06211b51624
https://hg.mozilla.org/mozilla-central/rev/6c92b6fac3e3
https://hg.mozilla.org/mozilla-central/rev/41f601758661
https://hg.mozilla.org/mozilla-central/rev/6db1cb600f52
https://hg.mozilla.org/mozilla-central/rev/2f104556b212
https://hg.mozilla.org/mozilla-central/rev/6bcb8ed863ab