New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 649384 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: ----

Blocking:
issue 649381



Sign in to add a comment

Skip multi-instance migration if tab_state0 exists

Project Member Reported by twelling...@chromium.org, Sep 22 2016

Issue description

---------------- Public bug ----------------

Version: 53.0.2785.97, 53.0.2785.124

We have several Play Store reports that users are losing tabs after upgrading to M53, initially reported in 53.0.2785.97.

I landed a fix for a bug in the document mode migration path as part of issue 646146 (https://codereview.chromium.org/2345603002). In order to hit the bug that was fixed, users had to be upgrading from a version of Chrome where document mode was enabled by default.

We released 53.0.2785.124, which contained the fix, and received more user reports of lost tabs.

For one user, it sounds like tabs were lost more than once after upgrading.

I'm going to add a speculative patch to skip the multi-instance migration path if tab_state0 file exists in case there's something going wrong with setting the shared preference for whether the migration has been run.
 

Comment 1 by k...@chromium.org, Sep 22 2016

Cc: k...@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98219047ca2fc2a198f7268de4a3a106b09f7bc2

commit 98219047ca2fc2a198f7268de4a3a106b09f7bc2
Author: twellington <twellington@chromium.org>
Date: Fri Sep 23 02:09:00 2016

Do not overwrite tab_state0 during multi-instance migration

During multi-instance migration, if tab_state0 already exists,
do not overwrite it by renaming tab_state. tab_state0 should not
already exist when multi-instance migration is run, but if it
does and it is overwritten then users may lose tabs.

BUG= 649384 

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

[modify] https://crrev.com/98219047ca2fc2a198f7268de4a3a106b09f7bc2/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabbedModeTabPersistencePolicy.java
[modify] https://crrev.com/98219047ca2fc2a198f7268de4a3a106b09f7bc2/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/MultiInstanceMigrationTest.java
[modify] https://crrev.com/98219047ca2fc2a198f7268de4a3a106b09f7bc2/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-54

Comment 4 by dimu@chromium.org, Sep 26 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Blocking: 649381
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 26 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/272420fed3f1a6933c7bff279ef2ea78e57367ef

commit 272420fed3f1a6933c7bff279ef2ea78e57367ef
Author: Theresa Wellington <twellington@google.com>
Date: Mon Sep 26 18:28:49 2016

Do not overwrite tab_state0 during multi-instance migration

During multi-instance migration, if tab_state0 already exists,
do not overwrite it by renaming tab_state. tab_state0 should not
already exist when multi-instance migration is run, but if it
does and it is overwritten then users may lose tabs.

BUG= 649384 
TBR=tedchoc@chromium.org, wnwen@chromium.org, holte@chromium.org

Review URL: https://codereview.chromium.org/2370923002 .

Review-Url: https://codereview.chromium.org/2360183003
Cr-Original-Commit-Position: refs/heads/master@{#420542}
Cr-Commit-Position: refs/branch-heads/2840@{#532}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/272420fed3f1a6933c7bff279ef2ea78e57367ef/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabbedModeTabPersistencePolicy.java
[modify] https://crrev.com/272420fed3f1a6933c7bff279ef2ea78e57367ef/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/MultiInstanceMigrationTest.java
[modify] https://crrev.com/272420fed3f1a6933c7bff279ef2ea78e57367ef/tools/metrics/histograms/histograms.xml

Comment 7 Deleted

Comment 8 Deleted

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/272420fed3f1a6933c7bff279ef2ea78e57367ef

commit 272420fed3f1a6933c7bff279ef2ea78e57367ef
Author: Theresa Wellington <twellington@google.com>
Date: Mon Sep 26 18:28:49 2016

Do not overwrite tab_state0 during multi-instance migration

During multi-instance migration, if tab_state0 already exists,
do not overwrite it by renaming tab_state. tab_state0 should not
already exist when multi-instance migration is run, but if it
does and it is overwritten then users may lose tabs.

BUG= 649384 
TBR=tedchoc@chromium.org, wnwen@chromium.org, holte@chromium.org

Review URL: https://codereview.chromium.org/2370923002 .

Review-Url: https://codereview.chromium.org/2360183003
Cr-Original-Commit-Position: refs/heads/master@{#420542}
Cr-Commit-Position: refs/branch-heads/2840@{#532}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/272420fed3f1a6933c7bff279ef2ea78e57367ef/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabbedModeTabPersistencePolicy.java
[modify] https://crrev.com/272420fed3f1a6933c7bff279ef2ea78e57367ef/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/MultiInstanceMigrationTest.java
[modify] https://crrev.com/272420fed3f1a6933c7bff279ef2ea78e57367ef/tools/metrics/histograms/histograms.xml

Sign in to add a comment