Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(97)

Issue 2274943002: Fix document mode migration. (Closed)

Created:
4 years, 3 months ago by Peter Wen
Modified:
4 years, 3 months ago
Reviewers:
Theresa, gone
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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 Committed: https://crrev.com/9df9c7815b45e142c29162e02ddcdd8f411ec703 Cr-Commit-Position: refs/heads/master@{#414194}

Patch Set 1 #

Patch Set 2 : Remove special timing. #

Total comments: 4

Patch Set 3 : Switch order. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -13 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java View 1 2 2 chunks +10 lines, -3 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassinTest.java View 1 10 chunks +13 lines, -10 lines 0 comments Download

Messages

Total messages: 17 (10 generated)
Peter Wen
Going to be out this afternoon, will respond to comments tomorrow. :)
4 years, 3 months ago (2016-08-24 16:43:27 UTC) #6
gone
lgtm https://chromiumcodereview.appspot.com/2274943002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java File chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java (right): https://chromiumcodereview.appspot.com/2274943002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java#newcode125 chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java:125: private final ObserverList<DocumentModeAssassinObserver> mObservers = does this fit ...
4 years, 3 months ago (2016-08-24 18:34:45 UTC) #9
Peter Wen
https://chromiumcodereview.appspot.com/2274943002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java File chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java (right): https://chromiumcodereview.appspot.com/2274943002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java#newcode125 chrome/android/java/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassin.java:125: private final ObserverList<DocumentModeAssassinObserver> mObservers = On 2016/08/24 18:34:45, dfalcantara ...
4 years, 3 months ago (2016-08-24 21:56:40 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2274943002/40001
4 years, 3 months ago (2016-08-24 21:57:25 UTC) #13
Theresa
lgtm
4 years, 3 months ago (2016-08-24 22:04:41 UTC) #14
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 3 months ago (2016-08-24 23:21:29 UTC) #15
commit-bot: I haz the power
4 years, 3 months ago (2016-08-24 23:22:51 UTC) #17
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/9df9c7815b45e142c29162e02ddcdd8f411ec703
Cr-Commit-Position: refs/heads/master@{#414194}

Powered by Google App Engine
This is Rietveld 408576698