Add a checks that full browser doesn't started in

ServicificationBackgroundServiceTest.

Add a check to make sure the background tasks are running in ServiceManager
only mode and the full browser doesn't start. Otherwise, if the test falls back
to start the full browser, it won't be able to detect any failure that happens
only in the ServiceManager mode.

To do this, we plumb the startup result from the native to Java via the
callback when ServiceManager started. The callback will tells Java if the full
browser is starting.

Bug: 904549
Change-Id: I39d39166269fa0fcebf54b9d8ece3b6c761a223a
Reviewed-on: https://chromium-review.googlesource.com/c/1363369
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615666}

chrome/android/javatests/src/org/chromium/chrome/browser/ServicificationBackgroundService.java

@@ -9,10 +9,12 @@ import android.content.Context;
 import org.junit.Assert;
 
 import org.chromium.base.ThreadUtils;
+import org.chromium.base.library_loader.LibraryProcessType;
 import org.chromium.base.library_loader.ProcessInitException;
 import org.chromium.chrome.browser.init.BrowserParts;
 import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
 import org.chromium.chrome.browser.init.EmptyBrowserParts;
+import org.chromium.content_public.browser.BrowserStartupController;
 import org.chromium.content_public.browser.test.util.Criteria;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
 
@@ -65,4 +67,19 @@ public class ServicificationBackgroundService extends ChromeBackgroundService {
                     }
                 });
     }
+
+    public void postTaskAndVerifyFullBrowserNotStarted() {
+        // This task will always be queued and executed after
+        // the BrowserStartupControllerImpl#browserStartupComplete() is called on the UI thread when
+        // the full browser starts. So we can use it to checks whether the
+        // {@link mFullBrowserStartupDone} has been set to true.
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                Assert.assertFalse("The full browser is started instead of ServiceManager only.",
+                        BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER)
+                                .isStartupSuccessfullyCompleted());
+            }
+        });
+    }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/ServicificationBackgroundServiceTest.java

@@ -45,6 +45,7 @@ public final class ServicificationBackgroundServiceTest {
         mServicificationBackgroundService.onRunTask(new TaskParams(taskTag));
         mServicificationBackgroundService.checkExpectations(shouldStart);
         mServicificationBackgroundService.waitForServiceManagerStart();
+        mServicificationBackgroundService.postTaskAndVerifyFullBrowserNotStarted();
     }
 
     @Test

chrome/android/javatests/src/org/chromium/chrome/browser/download/ServicificationDownloadTest.java

@@ -88,6 +88,7 @@ public final class ServicificationDownloadTest {
         mServicificationBackgroundService.onRunTask(
                 new TaskParams(ServiceManagerStartupUtils.TASK_TAG));
         mServicificationBackgroundService.waitForServiceManagerStart();
+        mServicificationBackgroundService.postTaskAndVerifyFullBrowserNotStarted();
 
         String tempFile = InstrumentationRegistry.getInstrumentation()
                                   .getTargetContext()

content/app/content_main_runner_impl.cc

@@ -936,8 +936,10 @@ int ContentMainRunnerImpl::RunServiceManager(MainFunctionParams& main_params,
         new ServiceManagerContext(service_manager_thread_->task_runner()));
     download::SetIOTaskRunner(service_manager_thread_->task_runner());
 #if defined(OS_ANDROID)
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(&ServiceManagerStartupComplete));
+    if (start_service_manager_only) {
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE, base::BindOnce(&ServiceManagerStartupComplete));
+    }
 #endif
   }
 

content/public/android/java/src/org/chromium/content/browser/BrowserStartupControllerImpl.java

@@ -317,33 +317,34 @@ public class BrowserStartupControllerImpl implements BrowserStartupController {
             // callbacks will be deferred until browser startup completes.
             mCurrentBrowserStartType = BROWSER_START_TYPE_FULL_BROWSER;
             if (contentStart() > 0) enqueueCallbackExecution(STARTUP_FAILURE);
-        } else if (mCurrentBrowserStartType == BROWSER_START_TYPE_SERVICE_MANAGER_ONLY) {
-            // If full browser startup is not needed, execute all the callbacks now.
-            executeEnqueuedCallbacks(STARTUP_SUCCESS);
+            return;
+        }
+
+        if (mCurrentBrowserStartType == BROWSER_START_TYPE_SERVICE_MANAGER_ONLY) {
+            executeServiceManagerCallbacks(STARTUP_SUCCESS);
         }
         recordStartupUma();
     }
 
     private void executeEnqueuedCallbacks(int startupResult) {
         assert ThreadUtils.runningOnUiThread() : "Callback from browser startup from wrong thread.";
-        // If only ServiceManager is launched, don't set mFullBrowserStartupDone, wait for the full
-        // browser launch to set this variable.
-        mFullBrowserStartupDone = mCurrentBrowserStartType == BROWSER_START_TYPE_FULL_BROWSER;
+        mFullBrowserStartupDone = true;
         mStartupSuccess = (startupResult <= 0);
-        if (mFullBrowserStartupDone) {
-            for (StartupCallback asyncStartupCallback : mAsyncStartupCallbacks) {
-                if (mStartupSuccess) {
-                    asyncStartupCallback.onSuccess();
-                } else {
-                    asyncStartupCallback.onFailure();
-                }
+        for (StartupCallback asyncStartupCallback : mAsyncStartupCallbacks) {
+            if (mStartupSuccess) {
+                asyncStartupCallback.onSuccess();
+            } else {
+                asyncStartupCallback.onFailure();
             }
-            // We don't want to hold on to any objects after we do not need them anymore.
-            mAsyncStartupCallbacks.clear();
         }
-        // The ServiceManager should have been started, call the callbacks now.
-        // TODO(qinmin): Handle mServiceManagerCallbacks in serviceManagerStarted() instead of
-        // here once http://crbug.com/854231 is fixed.
+        // We don't want to hold on to any objects after we do not need them anymore.
+        mAsyncStartupCallbacks.clear();
+
+        executeServiceManagerCallbacks(startupResult);
+    }
+
+    private void executeServiceManagerCallbacks(int startupResult) {
+        mStartupSuccess = (startupResult <= 0);
         for (StartupCallback serviceMangerCallback : mServiceManagerCallbacks) {
             if (mStartupSuccess) {
                 serviceMangerCallback.onSuccess();