Fix document mode migration.

There is an old tab_state file in the tabbed mode state directory. If
it is not deleted and the new tab state is written to the new
tab_state0 file, then when it comes time to run the migration code in
TabPersistentStore the old tab_state file will be given preference
and overwrite the new one with all the document mode tabs that were
just migrated. Thus we need to delete the old tab_state file if it
exists.

BUG=640625

Review-Url: https://codereview.chromium.org/2274943002
Cr-Commit-Position: refs/heads/master@{#414194}

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java

@@ -119,11 +119,10 @@ public class DocumentModeAssassin {
     }
 
     /** IDs of Tabs that have had their TabState files copied between directories successfully. */
-    private final Set<Integer> mMigratedTabIds = new HashSet<Integer>();
+    private final Set<Integer> mMigratedTabIds = new HashSet<>();
 
     /** Observers of the migration pipeline. */
-    private final ObserverList<DocumentModeAssassinObserver> mObservers =
-            new ObserverList<DocumentModeAssassinObserver>();
+    private final ObserverList<DocumentModeAssassinObserver> mObservers = new ObserverList<>();
 
     /** Current stage of the migration. */
     private int mStage = STAGE_UNINITIALIZED;
@@ -342,6 +341,14 @@ public class DocumentModeAssassin {
             @Override
             protected Boolean doInBackground(Void... params) {
                 if (mSerializedMetadata != null) {
+                    // If an old tab state file still exists when we run migration in TPS, then it
+                    // will overwrite the new tab state file that our document tabs migrated to.
+                    File oldMetadataFile = new File(
+                            getTabbedDataDirectory(), TabPersistentStore.SAVED_STATE_FILE);
+                    if (oldMetadataFile.exists() && !oldMetadataFile.delete()) {
+                        Log.e(TAG, "Failed to delete old tab state file: " + oldMetadataFile);
+                    }
+
                     TabPersistentStore.saveListToFile(
                             getTabbedDataDirectory(), TAB_MODEL_INDEX, mSerializedMetadata);
                     return true;

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassinTest.java

@@ -189,7 +189,7 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
         final CallbackHelper changeDoneCallback = new CallbackHelper();
         final CallbackHelper deletionStartedCallback = new CallbackHelper();
         final CallbackHelper deletionDoneCallback = new CallbackHelper();
-        final ArrayList<Integer> copiedIds = new ArrayList<Integer>();
+        final ArrayList<Integer> copiedIds = new ArrayList<>();
         final DocumentModeAssassinObserver observer = new DocumentModeAssassinObserver() {
             @Override
             public void onStageChange(int newStage) {
@@ -219,7 +219,7 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
             }
         };
 
-        setUpDocumentDirectory();
+        setUpDirectories();
         final DocumentModeAssassin assassin = createAssassinForTesting(
                 DocumentModeAssassin.STAGE_UNINITIALIZED, true, true);
         ThreadUtils.runOnUiThread(new Runnable() {
@@ -302,7 +302,7 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
                 DocumentModeAssassin.MAX_MIGRATION_ATTEMPTS_BEFORE_FAILURE);
         editor.apply();
 
-        setUpDocumentDirectory();
+        setUpDirectories();
         final DocumentModeAssassin assassin =
                 createAssassinForTesting(DocumentModeAssassin.STAGE_UNINITIALIZED, true, true);
         ThreadUtils.runOnUiThread(new Runnable() {
@@ -387,7 +387,7 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
         // one of them.  This forces the TabPersistentStore to improvise and use the initial URL
         // that we provide.
         final TabStateInfo failureCase = TestTabModelDirectory.V2_HAARETZ;
-        final Set<Integer> migratedTabIds = new HashSet<Integer>();
+        final Set<Integer> migratedTabIds = new HashSet<>();
         for (int i = 0; i < TAB_STATE_INFO.length; i++) {
             if (failureCase.tabId == TAB_STATE_INFO[i].tabId) continue;
             migratedTabIds.add(TAB_STATE_INFO[i].tabId);
@@ -496,7 +496,7 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
         final CallbackHelper copyDoneCallback = new CallbackHelper();
         final CallbackHelper copyCallback = new CallbackHelper();
         final AtomicInteger firstCopiedId = new AtomicInteger(Tab.INVALID_TAB_ID);
-        final ArrayList<Integer> copiedIds = new ArrayList<Integer>();
+        final ArrayList<Integer> copiedIds = new ArrayList<>();
         final DocumentModeAssassinObserver observer = new DocumentModeAssassinObserver() {
             @Override
             public void onStageChange(int newStage) {
@@ -516,7 +516,7 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
         };
 
         // Kick off copying the tab states.
-        setUpDocumentDirectory();
+        setUpDirectories();
         final DocumentModeAssassin assassin =
                 createAssassinForTesting(DocumentModeAssassin.STAGE_INITIALIZED, false, true);
         ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@@ -550,7 +550,8 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
         // Confirm that the legitimate TabState files were all copied over.
         File[] tabbedModeFilesAfter = mTabbedModeDirectory.getDataDirectory().listFiles();
         assertNotNull(tabbedModeFilesAfter);
-        assertEquals(TAB_STATE_INFO.length, tabbedModeFilesAfter.length);
+        // +1 is for the original tab_state file in the tabbed directory.
+        assertEquals(TAB_STATE_INFO.length + 1, tabbedModeFilesAfter.length);
 
         for (int i = 0; i < TAB_STATE_INFO.length; i++) {
             boolean found = false;
@@ -591,7 +592,7 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
         editor.apply();
 
         // Kick off deleting everything.
-        setUpDocumentDirectory();
+        setUpDirectories();
         final DocumentModeAssassin assassin = createAssassinForTesting(
                 DocumentModeAssassin.STAGE_CHANGE_SETTINGS_DONE, false, true);
         ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@@ -647,8 +648,8 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
         };
     }
 
-    /** Fills in the directory for document mode with a bunch of data. */
-    private void setUpDocumentDirectory() throws Exception {
+    /** Fills in the directories for document and tabbed mode with a bunch of data. */
+    private void setUpDirectories() throws Exception {
         // Write out all of the TabState files into the document mode directory.
         for (int i = 0; i < TAB_STATE_INFO.length; i++) {
             mDocumentModeDirectory.writeTabStateFile(TAB_STATE_INFO[i]);
@@ -659,5 +660,7 @@ public class DocumentModeAssassinTest extends NativeLibraryTestBase {
         writeUselessFileToDirectory(mDocumentModeDirectory.getDataDirectory(),
                 TabState.SAVED_TAB_STATE_FILE_PREFIX + "_unparseable");
 
+        writeUselessFileToDirectory(mTabbedModeDirectory.getDataDirectory(),
+                TabPersistentStore.SAVED_STATE_FILE);
     }
 }