customtabs: Allow parallel requests to be processed before native is available.

Parallel requests are initiated when an intent is dispatched to a CustomTabs
activity. At that point, native is not guaranteed to be initialized, but the
request needs the browser to be initialized, not merely the native library to be
loaded. In this case, delay the call until this condition is fullfilled.

To that end, add a delayed task mechanism to ChromeBrowserInitializer. Add tests
to this class, and while in the neighborhood, convert Runnable to lambdas.

Note that this is not exposed to clients yet, as enabling the feature still
requires the browser to be initialized. This first CL introduces the mechanism
and ensures that existing behavior is preserved.

Bug: 901835
Change-Id: I184e1da5ae974458d19f6a88a3f26ba6c443cb57
Reviewed-on: https://chromium-review.googlesource.com/c/1349227Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611054}

chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java

@@ -854,7 +854,8 @@ public class CustomTabsConnection {
         if (mWarmupTasks != null) mWarmupTasks.cancel();
 
         maybePreconnectToRedirectEndpoint(session, url, intent);
-        handleParallelRequest(session, intent);
+        ChromeBrowserInitializer.getInstance().runNowOrAfterNativeInitialization(
+                () -> handleParallelRequest(session, intent));
         maybePrefetchResources(session, intent);
     }
 

chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java

@@ -46,6 +46,8 @@ import org.chromium.policy.CombinedPolicyProvider;
 import org.chromium.ui.resources.ResourceExtractor;
 
 import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Locale;
 
 /**
@@ -59,6 +61,7 @@ public class ChromeBrowserInitializer {
     private static BrowserStartupController sBrowserStartupController;
     private final ChromeApplication mApplication;
     private final Locale mInitialLocale = Locale.getDefault();
+    private List<Runnable> mTasksToRunWithNative;
 
     private boolean mPreInflationStartupComplete;
     private boolean mPostInflationStartupComplete;
@@ -111,6 +114,24 @@ public class ChromeBrowserInitializer {
         return mNativeInitializationComplete;
     }
 
+    /**
+     * Either runs a task now, or queue it until native initialization is done.
+     *
+     * All Runnables added this way will run in a single UI thread task.
+     *
+     * @param task The task to run.
+     */
+    public void runNowOrAfterNativeInitialization(Runnable task) {
+        if (hasNativeInitializationCompleted()) {
+            task.run();
+        } else {
+            if (mTasksToRunWithNative == null) {
+                mTasksToRunWithNative = new ArrayList<Runnable>();
+            }
+            mTasksToRunWithNative.add(task);
+        }
+    }
+
     /**
      * Initializes the Chrome browser process synchronously.
      *
@@ -262,70 +283,41 @@ public class ChromeBrowserInitializer {
         // launch its required components.
         if (!delegate.startServiceManagerOnly()
                 && !ProcessInitializationHandler.getInstance().postNativeInitializationComplete()) {
-            tasks.add(new Runnable() {
-                @Override
-                public void run() {
-                    ProcessInitializationHandler.getInstance().initializePostNative();
-                }
-            });
+            tasks.add(() -> ProcessInitializationHandler.getInstance().initializePostNative());
         }
 
         if (!mNetworkChangeNotifierInitializationComplete) {
-            tasks.add(new Runnable() {
-                @Override
-                public void run() {
-                    initNetworkChangeNotifier();
-                }
-            });
+            tasks.add(this::initNetworkChangeNotifier);
         }
 
-        tasks.add(new Runnable() {
-            @Override
-            public void run() {
-                // This is not broken down as a separate task, since this:
-                // 1. Should happen as early as possible
-                // 2. Only submits asynchronous work
-                // 3. Is thus very cheap (profiled at 0.18ms on a Nexus 5 with Lollipop)
-                // It should also be in a separate task (and after) initNetworkChangeNotifier, as
-                // this posts a task to the UI thread that would interfere with preconneciton
-                // otherwise. By preconnecting afterwards, we make sure that this task has run.
-                delegate.maybePreconnect();
-
-                onStartNativeInitialization();
-            }
+        tasks.add(() -> {
+            // This is not broken down as a separate task, since this:
+            // 1. Should happen as early as possible
+            // 2. Only submits asynchronous work
+            // 3. Is thus very cheap (profiled at 0.18ms on a Nexus 5 with Lollipop)
+            // It should also be in a separate task (and after) initNetworkChangeNotifier, as
+            // this posts a task to the UI thread that would interfere with preconneciton
+            // otherwise. By preconnecting afterwards, we make sure that this task has run.
+            delegate.maybePreconnect();
+
+            onStartNativeInitialization();
         });
 
-        tasks.add(new Runnable() {
-            @Override
-            public void run() {
-                if (delegate.isActivityDestroyed()) return;
-                delegate.initializeCompositor();
-            }
+        tasks.add(() -> {
+            if (delegate.isActivityDestroyed()) return;
+            delegate.initializeCompositor();
         });
 
-        tasks.add(new Runnable() {
-            @Override
-            public void run() {
-                if (delegate.isActivityDestroyed()) return;
-                delegate.initializeState();
-            }
+        tasks.add(() -> {
+            if (delegate.isActivityDestroyed()) return;
+            delegate.initializeState();
         });
 
-        if (!mNativeInitializationComplete) {
-            tasks.add(new Runnable() {
-                @Override
-                public void run() {
-                    onFinishNativeInitialization();
-                }
-            });
-        }
+        if (!mNativeInitializationComplete) tasks.add(this::onFinishNativeInitialization);
 
-        tasks.add(new Runnable() {
-            @Override
-            public void run() {
-                if (delegate.isActivityDestroyed()) return;
-                delegate.finishNativeInitialization();
-            }
+        tasks.add(() -> {
+            if (delegate.isActivityDestroyed()) return;
+            delegate.finishNativeInitialization();
         });
 
         if (isAsync) {
@@ -426,6 +418,10 @@ public class ChromeBrowserInitializer {
         });
 
         MemoryPressureUma.initializeForBrowser();
+        if (mTasksToRunWithNative != null) {
+            for (Runnable r : mTasksToRunWithNative) r.run();
+            mTasksToRunWithNative = null;
+        }
     }
 
     private ActivityStateListener createActivityStateListener() {

chrome/android/java_sources.gni

@@ -1960,6 +1960,7 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/gcore/MockConnectedTaskTest.java",
   "javatests/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListenerTest.java",
   "javatests/src/org/chromium/chrome/browser/init/ChainedTasksTest.java",
+  "javatests/src/org/chromium/chrome/browser/init/ChromeBrowserInitializerTest.java",
   "javatests/src/org/chromium/chrome/browser/hardware_acceleration/ChromeTabbedActivityHWATest.java",
   "javatests/src/org/chromium/chrome/browser/hardware_acceleration/CustomTabActivityHWATest.java",
   "javatests/src/org/chromium/chrome/browser/hardware_acceleration/ManifestHWATest.java",

chrome/android/javatests/src/org/chromium/chrome/browser/init/ChromeBrowserInitializerTest.java

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.init;
+
+import android.support.test.filters.SmallTest;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.ThreadUtils;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
+
+/** Tests for ChromeBrowserInitializer. */
+@RunWith(ChromeJUnit4ClassRunner.class)
+public class ChromeBrowserInitializerTest {
+    private ChromeBrowserInitializer mInstance;
+
+    @Before
+    public void setUp() {
+        mInstance = ChromeBrowserInitializer.getInstance();
+        Assert.assertFalse(mInstance.hasNativeInitializationCompleted());
+    }
+
+    @Test
+    @SmallTest
+    public void testSynchronousInitialization() throws Exception {
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            Assert.assertFalse(mInstance.hasNativeInitializationCompleted());
+            mInstance.handleSynchronousStartup();
+            Assert.assertTrue(mInstance.hasNativeInitializationCompleted());
+            return true;
+        });
+    }
+
+    @Test
+    @SmallTest
+    public void testAsynchronousStartup() throws Exception {
+        final Semaphore done = new Semaphore(0);
+        BrowserParts parts = new EmptyBrowserParts() {
+            @Override
+            public void finishNativeInitialization() {
+                done.release();
+            }
+        };
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            Assert.assertFalse(mInstance.hasNativeInitializationCompleted());
+            mInstance.handlePreNativeStartup(parts);
+            mInstance.handlePostNativeStartup(true, parts);
+
+            Assert.assertFalse(
+                    "Should not be synchronous", mInstance.hasNativeInitializationCompleted());
+            return true;
+        });
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            Assert.assertFalse("Inititialization tasks should yield to new UI thread tasks",
+                    mInstance.hasNativeInitializationCompleted());
+        });
+        Assert.assertTrue(done.tryAcquire(10, TimeUnit.SECONDS));
+    }
+
+    @Test
+    @SmallTest
+    public void testDelayedTasks() throws Exception {
+        final Semaphore done = new Semaphore(0);
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            mInstance.runNowOrAfterNativeInitialization(done::release);
+            Assert.assertFalse("Should not run synchronously", done.tryAcquire());
+            mInstance.handleSynchronousStartup();
+            Assert.assertTrue(done.tryAcquire());
+            return true;
+        });
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            mInstance.runNowOrAfterNativeInitialization(done::release);
+            // Runs right away in the same task is initialization is done.
+            Assert.assertTrue(done.tryAcquire());
+        });
+    }
+}