Commit 2e30a3df authored by boliu's avatar boliu Committed by Commit bot

Revert of android: Randomize initial child service bind order (patchset #3...

Revert of android: Randomize initial child service bind order (patchset #3 id:40001 of https://codereview.chromium.org/2557273004/ )

Reason for revert:
Android keeps the intent for a long time, so
randomize doesn't help at all. More details:
crbug.com/664341#c91

Original issue's description:
> android: Randomize initial child service bind order
>
> Android restarts crashed services with the previous intent.
> If browser crashes and is restarted at the same time, then
> it's possible to bind to the restarted service with stale
> intent and bundle data. See crbug.com/664341#c83 for an
> example. Randomizing the start up order is a pure
> workaround to make this case less likely.
>
> BUG=664341
>
> Committed: https://crrev.com/bb725fed9bd8c672178c8a815f4b1a2500eef444
> Cr-Commit-Position: refs/heads/master@{#437367}

TBR=agrieve@chromium.org,jcivelli@chromium.org,mariakhomenko@chromium.org,hanxi@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=664341

Review-Url: https://codereview.chromium.org/2571723002
Cr-Commit-Position: refs/heads/master@{#438042}
parent d4a8c49b
...@@ -34,7 +34,6 @@ import org.chromium.content.common.SurfaceWrapper; ...@@ -34,7 +34,6 @@ import org.chromium.content.common.SurfaceWrapper;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedList; import java.util.LinkedList;
import java.util.Map; import java.util.Map;
import java.util.Queue; import java.util.Queue;
...@@ -74,12 +73,6 @@ public class ChildProcessLauncher { ...@@ -74,12 +73,6 @@ public class ChildProcessLauncher {
for (int i = 0; i < numChildServices; i++) { for (int i = 0; i < numChildServices; i++) {
mFreeConnectionIndices.add(i); mFreeConnectionIndices.add(i);
} }
// Randomize the order of named child services to workaround this bug:
// Android restarts crashed services with the previous intent. If browser crashes and
// is restarted at the same time, then it's possible to bind to the restarted service
// with stale intent and bundle data. See crbug.com/664341#c84 for an example.
// Randomizing the start up order is a pure workaround to make this case less likely.
Collections.shuffle(mFreeConnectionIndices);
mChildClassName = serviceClassName; mChildClassName = serviceClassName;
mInSandbox = inSandbox; mInSandbox = inSandbox;
} }
......
...@@ -250,16 +250,20 @@ public class ChildProcessLauncherTest extends InstrumentationTestCase { ...@@ -250,16 +250,20 @@ public class ChildProcessLauncherTest extends InstrumentationTestCase {
// Start and connect to a new service of an external APK. // Start and connect to a new service of an external APK.
ChildProcessConnectionImpl externalApkConnection = ChildProcessConnectionImpl externalApkConnection =
allocateConnection(EXTERNAL_APK_PACKAGE_NAME); allocateConnection(EXTERNAL_APK_PACKAGE_NAME);
assertNotNull(externalApkConnection);
// Start and connect to a new service for a regular tab. // Start and connect to a new service for a regular tab.
ChildProcessConnectionImpl tabConnection = allocateConnection(appContext.getPackageName()); ChildProcessConnectionImpl tabConnection = allocateConnection(appContext.getPackageName());
assertNotNull(tabConnection);
// Verify that one connection is allocated for an external APK and a regular tab // Verify that one connection is allocated for an external APK and a regular tab
// respectively. // respectively.
assertEquals(1, ChildProcessLauncher.allocatedSandboxedConnectionsCountForTesting( assertEquals(1, ChildProcessLauncher.allocatedSandboxedConnectionsCountForTesting(
appContext, EXTERNAL_APK_PACKAGE_NAME)); appContext, EXTERNAL_APK_PACKAGE_NAME));
assertEquals(1, allocatedChromeSandboxedConnectionsCount()); assertEquals(1, allocatedChromeSandboxedConnectionsCount());
// Verify that connections allocated for an external APK and the regular tab are from
// different ChildConnectionAllocators, since both ChildConnectionAllocators start
// allocating connections from number 0.
assertEquals(0, externalApkConnection.getServiceNumber());
assertEquals(0, tabConnection.getServiceNumber());
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment