[Tabbed mode] Restore tabs even if TabState is missing

[Tabbed mode] Restore tabs even if TabState is missing

Currently, Clank throws away tabs during initialization if the TabState
couldn't be loaded.  This doesn't always make sense because we store the last
URL of the tab in the TabModel metadata file.  Instead, re-create a Tab
with the last known URL of the Tab as long as it wasn't Incognito --
if an Incognito TabState couldn't be loaded, it's just as likely that the
user lost the encryption cipher key and the Tab should be thrown away,
anyway.

* Add the incognito tab count to the TabModel metadata that is saved
  out to disk.  This is used to determine whether a tab is a
  non-incognito tab.

* Adjust TabPersistentStore to allow upgrading to version 5 of the
  file format.

* Let TabRestoreDetails determine whether or not it contains Incognito
  tab data.

* Change restoreTab(...) to handle cases where the TabState is missing
  for regular tabs.  In these cases, new tabs will be created with the
  last known URL with new IDs.

* Remove a bunch of boolean return values that aren't used.

* Generalize TestTabModelDirectory so that it can support V5 TabModel
  data.

* Add a pair of tests for testing that V5 TabPersistentStore data can
  be restored.

BUG=485217
Committed: https://crrev.com/37753152e78e2ec6fe74278130df9db6b5d7b695
Cr-Commit-Position: refs/heads/master@{#354805}

[gone, 2015-10-16 22:18:36]

dfalcantara@chromium.org changed reviewers:
+ dtrainor@chromium.org

[David Trainor- moved to gerrit, 2015-10-17 05:58:41]

some nits but lgtm.

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java
(right):

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java:48:
private static final int SAVED_STATE_VERSION = 5;
at some point we should probably just go to protos.

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java:72:
* @param incognitoCount         Whether the Tab is definitely Incognito, or null
if it
incognitoCount -> isIncognito?  Also, when can't we tell?  That sounds scary.

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java:746:
Boolean isIncognito = (incognitoCount < 0) ? null : i < incognitoCount;
Ahh so we don't know if we don't know how many tabs are actually incognito or
not.  Ok!

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TestTabModelDirectory.java
(right):

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TestTabModelDirectory.java:245:
* restored (currently) because this Class doesn't support Incognito TabStates.
This comment is a bit confusing.  I thought V5 supports this now?

[gone, 2015-10-19 17:42:06]

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java
(right):

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java:48:
private static final int SAVED_STATE_VERSION = 5;
On 2015/10/17 05:58:41, David Trainor wrote:
> at some point we should probably just go to protos.

Yeah, was planning to but only after we had a standardized way of saving tabs. 
That'll happen soon™, but we should fix tab loss as much as we can given how far
off that always ends up being.

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java:72:
* @param incognitoCount         Whether the Tab is definitely Incognito, or null
if it
On 2015/10/17 05:58:41, David Trainor wrote:
> incognitoCount -> isIncognito?  Also, when can't we tell?  That sounds scary.

Ack; you seem to have figured this out later on.

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java:746:
Boolean isIncognito = (incognitoCount < 0) ? null : i < incognitoCount;
On 2015/10/17 05:58:41, David Trainor wrote:
> Ahh so we don't know if we don't know how many tabs are actually incognito or
> not.  Ok!

Acknowledged.

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TestTabModelDirectory.java
(right):

https://codereview.chromium.org/1408853002/diff/40001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TestTabModelDirectory.java:245:
* restored (currently) because this Class doesn't support Incognito TabStates.
On 2015/10/17 05:58:41, David Trainor wrote:
> This comment is a bit confusing.  I thought V5 supports this now?

Er, it doesn't support decrypting Incognito TabStates.  We don't currently have
any way of explicitly passing in a decryption key, AFAIK.

[gone, 2015-10-19 17:42:09]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2015-10-19 17:42:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/1408853002/#ps60001
(title: "Comments, rebasing")

[commit-bot: I haz the power, 2015-10-19 17:43:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1408853002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1408853002/60001

[commit-bot: I haz the power, 2015-10-19 18:24:16]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-19 18:25:27]

Patchset 4 (id:??) landed as
https://crrev.com/37753152e78e2ec6fe74278130df9db6b5d7b695
Cr-Commit-Position: refs/heads/master@{#354805}