Skip to content

Commit

Permalink
Fix incognito tabs persisting when cct service is active
Browse files Browse the repository at this point in the history
Swiping away the Chrome activity does not cause the process to be killed
if a Chrome Custom Tab is open somewhere. In this case, when Chrome is
started again, the CipherData static still has the old Activity's
incognito key stored in it and the tabs are unexpectedly restored.

This changes the start-up behaviour to explicitly ignore incognito tab
state files when no bundle is passed to the activity's onCreate.

TBR=amineer
BUG=626629

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

Review-Url: https://codereview.chromium.org/2166943002
Cr-Original-Commit-Position: refs/heads/master@{#406747}
Cr-Commit-Position: refs/branch-heads/2785@{crosswalk-project#308}
Cr-Branched-From: 6862397-refs/heads/master@{#403382}
  • Loading branch information
agrieve committed Jul 22, 2016
1 parent 5498390 commit b1bf44c
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,8 @@ public void initializeState() {

Intent intent = getIntent();

CipherFactory.getInstance().restoreFromBundle(getSavedInstanceState());
boolean hadCipherData =
CipherFactory.getInstance().restoreFromBundle(getSavedInstanceState());

boolean noRestoreState =
CommandLine.getInstance().hasSwitch(ChromeSwitches.NO_RESTORE_STATE);
Expand All @@ -564,7 +565,10 @@ public void initializeState() {
// State should be clear when we start first run and hence we do not need to load
// a previous state. This may change the current Model, watch out for initialization
// based on the model.
mTabModelSelectorImpl.loadState();
// Never attempt to restore incognito tabs when this activity was previously swiped
// away in Recents. http://crbug.com/626629
boolean ignoreIncognitoFiles = !hadCipherData;
mTabModelSelectorImpl.loadState(ignoreIncognitoFiles);
}

mIntentWithEffect = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,10 @@ public void saveState() {
/**
* Load the saved tab state. This should be called before any new tabs are created. The saved
* tabs shall not be restored until {@link #restoreTabs} is called.
* @param ignoreIncognitoFiles Whether to skip loading incognito tabs.
*/
public void loadState() {
mTabSaver.loadState();
public void loadState(boolean ignoreIncognitoFiles) {
mTabSaver.loadState(ignoreIncognitoFiles);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,9 @@ public void saveState() {

/**
* Restore saved state. Must be called before any tabs are added to the list.
* @param ignoreIncognitoFiles Whether to skip loading incognito tabs.
*/
public void loadState() {
public void loadState(boolean ignoreIncognitoFiles) {
long time = SystemClock.uptimeMillis();

// If a cleanup task is in progress, cancel it before loading state.
Expand All @@ -380,7 +381,7 @@ public void loadState() {
logExecutionTime("LoadStateTime", time);

mCancelNormalTabLoads = false;
mCancelIncognitoTabLoads = false;
mCancelIncognitoTabLoads = ignoreIncognitoFiles;
mNormalTabsRestored = new SparseIntArray();
mIncognitoTabsRestored = new SparseIntArray();
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ private void confirmTabModelMetadataFileIsCorrect(@Nullable TabStateInfo failure

// Load up the TabModel metadata.
int numExpectedTabs = TEST_INFO.numRegularTabs + TEST_INFO.numIncognitoTabs;
store.loadState();
store.loadState(false /* ignoreIncognitoFiles */);
mockObserver.initializedCallback.waitForCallback(0, 1);
assertEquals(numExpectedTabs, mockObserver.mTabCountAtStartup);
mockObserver.detailsReadCallback.waitForCallback(0, TEST_INFO.contents.length);
Expand Down Expand Up @@ -660,4 +660,4 @@ private void setUpDocumentDirectory() throws Exception {
TabState.SAVED_TAB_STATE_FILE_PREFIX + "_unparseable");

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public void testFindsMaxIdProperly() throws IOException {

int maxId = Math.max(getMaxId(selector0), getMaxId(selector1));
RecordHistogram.disableForTests();
storeIn.loadState();
storeIn.loadState(false /* ignoreIncognitoFiles */);
assertEquals("Invalid next id", maxId + 1,
TabIdManager.getInstance().generateValidId(Tab.INVALID_TAB_ID));
}
Expand Down Expand Up @@ -280,8 +280,8 @@ public void testOnlyLoadsSingleModel() throws IOException {
selectorIn1, 1, mAppContext, null, null);

RecordHistogram.disableForTests();
storeIn0.loadState();
storeIn1.loadState();
storeIn0.loadState(false /* ignoreIncognitoFiles */);
storeIn1.loadState(false /* ignoreIncognitoFiles */);

assertEquals("Unexpected number of tabs to load", 6, storeIn0.getRestoredTabCount());
assertEquals("Unexpected number of tabst o load", 3, storeIn1.getRestoredTabCount());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ public void testBasic() throws Exception {
assertNull(store.mPrefetchActiveTabTask);

// Make sure the metadata file loads properly and in order.
store.loadState();
store.loadState(false /* ignoreIncognitoFiles */);
mockObserver.initializedCallback.waitForCallback(0, 1);
assertEquals(numExpectedTabs, mockObserver.mTabCountAtStartup);

Expand Down Expand Up @@ -318,7 +318,7 @@ public void testInterruptedButStillRestoresAllTabs() throws Exception {
MockTabPersistentStoreObserver firstObserver = new MockTabPersistentStoreObserver();
final TabPersistentStore firstStore = new TabPersistentStore(
firstSelector, 0, mAppContext, firstManager, firstObserver);
firstStore.loadState();
firstStore.loadState(false /* ignoreIncognitoFiles */);
firstObserver.initializedCallback.waitForCallback(0, 1);
assertEquals(numExpectedTabs, firstObserver.mTabCountAtStartup);
firstObserver.detailsReadCallback.waitForCallback(0, numExpectedTabs);
Expand All @@ -342,7 +342,7 @@ public void run() {
// Make sure that all of the Tabs appear in the new one -- even though the new file was
// written before the first TabPersistentStore loaded any TabState files and added them to
// the TabModels.
secondStore.loadState();
secondStore.loadState(false /* ignoreIncognitoFiles */);
secondObserver.initializedCallback.waitForCallback(0, 1);
assertEquals(numExpectedTabs, secondObserver.mTabCountAtStartup);

Expand Down Expand Up @@ -397,7 +397,7 @@ public void testMissingTabStateButStillRestoresTab() throws Exception {
new TabPersistentStore(mockSelector, 0, mAppContext, mockManager, mockObserver);

// Make sure the metadata file loads properly and in order.
store.loadState();
store.loadState(false /* ignoreIncognitoFiles */);
mockObserver.initializedCallback.waitForCallback(0, 1);
assertEquals(numExpectedTabs, mockObserver.mTabCountAtStartup);

Expand Down Expand Up @@ -440,7 +440,7 @@ public void testRestoresTabWithMissingTabStateWhileIgnoringIncognitoTab() throws
new TabPersistentStore(mockSelector, 0, mAppContext, mockManager, mockObserver);

// Load the TabModel metadata.
store.loadState();
store.loadState(false /* ignoreIncognitoFiles */);
mockObserver.initializedCallback.waitForCallback(0, 1);
assertEquals(numExpectedTabs, mockObserver.mTabCountAtStartup);
mockObserver.detailsReadCallback.waitForCallback(0, numExpectedTabs);
Expand Down Expand Up @@ -475,7 +475,7 @@ public void testPrefetchActiveTab() throws Exception {

assertNotNull(store.mPrefetchActiveTabTask);

store.loadState();
store.loadState(false /* ignoreIncognitoFiles */);
store.restoreTabs(true);

ThreadUtils.runOnUiThreadBlocking(new Runnable() {
Expand Down Expand Up @@ -571,7 +571,7 @@ private TestTabModelSelector createAndRestoreRealTabModelImpls(TabModelMetaDataI

// Load up the TabModel metadata.
int numExpectedTabs = info.numRegularTabs + info.numIncognitoTabs;
store.loadState();
store.loadState(false /* ignoreIncognitoFiles */);
mockObserver.initializedCallback.waitForCallback(0, 1);
assertEquals(numExpectedTabs, mockObserver.mTabCountAtStartup);
mockObserver.detailsReadCallback.waitForCallback(0, info.contents.length);
Expand Down

0 comments on commit b1bf44c

Please sign in to comment.