Skip to content

Commit

Permalink
Remove argument from BrowserStartupController.StartupCallback.onSucce…
Browse files Browse the repository at this point in the history
…ss().

BrowserStartupController.StartupCallback.onSuccess() has 'alreadyStarted'
argument, which is not used anywhere.

Bug: 841403
Change-Id: I8faf23c3b1d61d217460c0e3943dadeb6111ee05
Reviewed-on: https://chromium-review.googlesource.com/1055796
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Dmitry Skiba <dskiba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558819}
  • Loading branch information
Dmitry Skiba authored and Commit Bot committed May 15, 2018
1 parent 97776d1 commit 3eae9af
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ public void onFailure() {
}

@Override
public void onSuccess(boolean success) {
public void onSuccess() {
tasks.start(false);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ private static void flushCachedMetrics() {
if (!browserStartup.isStartupSuccessfullyCompleted()) {
browserStartup.addStartupCompletedObserver(new StartupCallback() {
@Override
public void onSuccess(boolean alreadyStarted) {
public void onSuccess() {
flushCachedMetrics();
}
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ private static void runWhenChromeLoadsNative(final Runnable r) {
if (!browserStartup.isStartupSuccessfullyCompleted()) {
browserStartup.addStartupCompletedObserver(new StartupCallback() {
@Override
public void onSuccess(boolean alreadyStarted) {
public void onSuccess() {
r.run();
}
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ public void run() {
.addStartupCompletedObserver(
new BrowserStartupController.StartupCallback() {
@Override
public void onSuccess(boolean alreadyStarted) {
public void onSuccess() {
ensureNativeSideInitialized();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,17 @@ private static void onNativeLaunched(final Context context, final Runnable task)
@Override
public void run() {
BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER)
.addStartupCompletedObserver(
new StartupCallback() {
@Override
public void onSuccess(boolean alreadyStarted) {
task.run();
}
.addStartupCompletedObserver(new StartupCallback() {
@Override
public void onSuccess() {
task.run();
}

@Override
public void onFailure() {
// Startup failed.
}
});
@Override
public void onFailure() {
// Startup failed.
}
});
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,12 @@ public class BrowserStartupController {
* This provides the interface to the callbacks for successful or failed startup
*/
public interface StartupCallback {
void onSuccess(boolean alreadyStarted);
void onSuccess();
void onFailure();
}

private static final String TAG = "cr.BrowserStartup";

// Helper constants for {@link StartupCallback#onSuccess}.
private static final boolean ALREADY_STARTED = true;
private static final boolean NOT_ALREADY_STARTED = false;

// Helper constants for {@link #executeEnqueuedCallbacks(int, boolean)}.
@VisibleForTesting
static final int STARTUP_SUCCESS = -1;
Expand All @@ -72,7 +68,7 @@ private static void setShouldStartGpuProcessOnBrowserStartup(boolean enable) {
@CalledByNative
static void browserStartupComplete(int result) {
if (sInstance != null) {
sInstance.executeEnqueuedCallbacks(result, NOT_ALREADY_STARTED);
sInstance.executeEnqueuedCallbacks(result);
}
}

Expand Down Expand Up @@ -113,7 +109,7 @@ static boolean shouldStartGpuProcessOnBrowserStartup() {
public void run() {
addStartupCompletedObserver(new StartupCallback() {
@Override
public void onSuccess(boolean alreadyStarted) {
public void onSuccess() {
assert mTracingController == null;
Context context = ContextUtils.getApplicationContext();
mTracingController = new TracingControllerAndroid(context);
Expand Down Expand Up @@ -191,7 +187,7 @@ public void run() {
if (mHasCalledContentStart) return;
if (contentStart() > 0) {
// Failed. The callbacks may not have run, so run them.
enqueueCallbackExecution(STARTUP_FAILURE, NOT_ALREADY_STARTED);
enqueueCallbackExecution(STARTUP_FAILURE);
}
}
});
Expand Down Expand Up @@ -220,7 +216,7 @@ public void startBrowserProcessesSync(boolean singleProcess) throws ProcessInitE
if (!mHasCalledContentStart) {
if (contentStart() > 0) {
// Failed. The callbacks may not have run, so run them.
enqueueCallbackExecution(STARTUP_FAILURE, NOT_ALREADY_STARTED);
enqueueCallbackExecution(STARTUP_FAILURE);
startedSuccessfully = false;
}
}
Expand Down Expand Up @@ -268,13 +264,13 @@ public void addStartupCompletedObserver(StartupCallback callback) {
}
}

private void executeEnqueuedCallbacks(int startupResult, boolean alreadyStarted) {
private void executeEnqueuedCallbacks(int startupResult) {
assert ThreadUtils.runningOnUiThread() : "Callback from browser startup from wrong thread.";
mStartupDone = true;
mStartupSuccess = (startupResult <= 0);
for (StartupCallback asyncStartupCallback : mAsyncStartupCallbacks) {
if (mStartupSuccess) {
asyncStartupCallback.onSuccess(alreadyStarted);
asyncStartupCallback.onSuccess();
} else {
asyncStartupCallback.onFailure();
}
Expand All @@ -285,11 +281,11 @@ private void executeEnqueuedCallbacks(int startupResult, boolean alreadyStarted)

// Queue the callbacks to run. Since running the callbacks clears the list it is safe to call
// this more than once.
private void enqueueCallbackExecution(final int startupFailure, final boolean alreadyStarted) {
private void enqueueCallbackExecution(final int startupFailure) {
new Handler().post(new Runnable() {
@Override
public void run() {
executeEnqueuedCallbacks(startupFailure, alreadyStarted);
executeEnqueuedCallbacks(startupFailure);
}
});
}
Expand All @@ -299,7 +295,7 @@ private void postStartupCompleted(final StartupCallback callback) {
@Override
public void run() {
if (mStartupSuccess) {
callback.onSuccess(ALREADY_STARTED);
callback.onSuccess();
} else {
callback.onFailure();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,11 @@ private static class TestStartupCallback implements BrowserStartupController.Sta
private boolean mWasSuccess;
private boolean mWasFailure;
private boolean mHasStartupResult;
private boolean mAlreadyStarted;

@Override
public void onSuccess(boolean alreadyStarted) {
public void onSuccess() {
assert !mHasStartupResult;
mWasSuccess = true;
mAlreadyStarted = alreadyStarted;
mHasStartupResult = true;
}

Expand Down Expand Up @@ -131,9 +129,6 @@ public void run() {

Assert.assertTrue("Callback should have been executed.", callback.mHasStartupResult);
Assert.assertTrue("Callback should have been a success.", callback.mWasSuccess);
Assert.assertFalse(
"Callback should be told that the browser process was not already started.",
callback.mAlreadyStarted);
}

@Test
Expand Down Expand Up @@ -185,11 +180,6 @@ public void run() {
Assert.assertTrue("Callback 2 should have been a success.", callback2.mWasSuccess);
Assert.assertTrue("Callback 3 should have been executed.", callback3.mHasStartupResult);
Assert.assertTrue("Callback 3 should have been a success.", callback3.mWasSuccess);
// Some startup tasks might have been enqueued after the browser process was started, but
// not the first one which kicked of the startup.
Assert.assertFalse(
"Callback 1 should be told that the browser process was not already started.",
callback1.mAlreadyStarted);
}

@Test
Expand Down Expand Up @@ -255,12 +245,8 @@ public void run() {

Assert.assertTrue("Callback 3 should have been executed.", callback3.mHasStartupResult);
Assert.assertTrue("Callback 3 should have been a success.", callback3.mWasSuccess);
Assert.assertTrue("Callback 3 should be told that the browser process was already started.",
callback3.mAlreadyStarted);
Assert.assertTrue("Callback 4 should have been executed.", callback4.mHasStartupResult);
Assert.assertTrue("Callback 4 should have been a success.", callback4.mWasSuccess);
Assert.assertTrue("Callback 4 should be told that the browser process was already started.",
callback4.mAlreadyStarted);
}

@Test
Expand Down Expand Up @@ -412,9 +398,6 @@ public void run() {

Assert.assertTrue("Callback should have been executed.", callback.mHasStartupResult);
Assert.assertTrue("Callback should have been a success.", callback.mWasSuccess);
Assert.assertFalse(
"Callback should be told that the browser process was not already started.",
callback.mAlreadyStarted);
}

@Test
Expand Down Expand Up @@ -459,8 +442,6 @@ public void run() {

Assert.assertTrue("Callback should have been executed.", callback.mHasStartupResult);
Assert.assertTrue("Callback should have been a success.", callback.mWasSuccess);
Assert.assertTrue("Callback should be told that the browser process was already started.",
callback.mAlreadyStarted);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,9 @@ public void onCreate(final Bundle savedInstanceState) {
try {
BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER)
.startBrowserProcessesAsync(
true,
new BrowserStartupController.StartupCallback() {
true, new BrowserStartupController.StartupCallback() {
@Override
public void onSuccess(boolean alreadyStarted) {
public void onSuccess() {
finishInitialization(savedInstanceState);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,9 @@ protected void onCreate(final Bundle savedInstanceState) {
try {
BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER)
.startBrowserProcessesAsync(
true,
new BrowserStartupController.StartupCallback() {
true, new BrowserStartupController.StartupCallback() {
@Override
public void onSuccess(boolean alreadyStarted) {
public void onSuccess() {
finishInitialization(savedInstanceState);
}

Expand Down

0 comments on commit 3eae9af

Please sign in to comment.