Reland: Add traits to ChainedTasks so we can prioritize accordingly.
The problem with mojo that lead to the revery has been fixed.
Original path: https://chromium-review.googlesource.com/c/chromium/src/+/1517707
Both usages count as bootstrap tasks which will be prioritized by a
subsequent patch.
(cherry picked from commit 8ed84293465fa4eba6a3416729c7a121ff706e96)
TBR: yfriedman@chromium.org
Bug: 863341, 974006
Change-Id: I95cae97d1b8e8d9ed5cdab3b8bd323b5c38d3026
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1617771
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#666687}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1661311
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#332}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java
index 95dee59..65f1b77 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java
@@ -523,11 +523,13 @@
};
if (mNavigationEnabled && mNavigationHandler == null) {
Tab currentTab = mTabModelSelector.getCurrentTab();
- mNavigationHandler = new NavigationHandler(mViewContainer,
- new TabSwitcherActionDelegate(currentTab.getActivity()::onBackPressed,
- mTabModelSelector::getCurrentTab),
- NavigationGlowFactory.forSceneLayer(mViewContainer, mSceneLayer,
- currentTab.getActivity().getWindowAndroid()));
+ if (currentTab != null) {
+ mNavigationHandler = new NavigationHandler(mViewContainer,
+ new TabSwitcherActionDelegate(currentTab.getActivity()::onBackPressed,
+ mTabModelSelector::getCurrentTab),
+ NavigationGlowFactory.forSceneLayer(mViewContainer, mSceneLayer,
+ currentTab.getActivity().getWindowAndroid()));
+ }
}
}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
index 27e5636..4d8638d 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
@@ -415,7 +415,7 @@
// (1)
if (!initialized) {
- tasks.add(() -> {
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, () -> {
try (TraceEvent e = TraceEvent.scoped("CustomTabsConnection.initializeBrowser()")) {
initializeBrowser(ContextUtils.getApplicationContext());
ChromeBrowserInitializer.getInstance().initNetworkChangeNotifier();
@@ -426,7 +426,7 @@
// (2)
if (mayCreateSpareWebContents && !mHiddenTabHolder.hasHiddenTab()) {
- tasks.add(() -> {
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, () -> {
// Temporary fix for https://crbug.com/797832.
// TODO(lizeb): Properly fix instead of papering over the bug, this code should
// not be scheduled unless startup is done. See https://crbug.com/797832.
@@ -441,7 +441,7 @@
}
// (3)
- tasks.add(() -> {
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, () -> {
try (TraceEvent e = TraceEvent.scoped("InitializeViewHierarchy")) {
WarmupManager.getInstance().initializeViewHierarchy(
ContextUtils.getApplicationContext(),
@@ -450,7 +450,7 @@
});
if (!initialized) {
- tasks.add(() -> {
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, () -> {
try (TraceEvent e = TraceEvent.scoped("WarmupInternalFinishInitialization")) {
// (4)
Profile profile = Profile.getLastUsedProfile();
@@ -465,7 +465,7 @@
});
}
- tasks.add(() -> notifyWarmupIsDone(uid));
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, () -> notifyWarmupIsDone(uid));
tasks.start(false);
mWarmupTasks = tasks;
return true;
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/init/ChainedTasks.java b/chrome/android/java/src/org/chromium/chrome/browser/init/ChainedTasks.java
index c2614cc..ee157a2 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/init/ChainedTasks.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/init/ChainedTasks.java
@@ -4,7 +4,11 @@
package org.chromium.chrome.browser.init;
+import android.util.Pair;
+
import org.chromium.base.ThreadUtils;
+import org.chromium.base.task.PostTask;
+import org.chromium.base.task.TaskTraits;
import java.util.LinkedList;
@@ -18,15 +22,16 @@
* - {@link cancel()} must be called from the UI thread.
*/
public class ChainedTasks {
- private LinkedList<Runnable> mTasks = new LinkedList<>();
+ private LinkedList<Pair<TaskTraits, Runnable>> mTasks = new LinkedList<>();
private volatile boolean mFinalized;
private final Runnable mRunAndPost = new Runnable() {
@Override
public void run() {
if (mTasks.isEmpty()) return;
- mTasks.pop().run();
- ThreadUtils.postOnUiThread(this);
+ Pair<TaskTraits, Runnable> pair = mTasks.pop();
+ pair.second.run();
+ if (!mTasks.isEmpty()) PostTask.postTask(mTasks.peek().first, this);
}
};
@@ -34,9 +39,9 @@
* Adds a task to the list of tasks to run. Cannot be called once {@link start()} has been
* called.
*/
- public void add(Runnable task) {
+ public void add(TaskTraits traits, Runnable task) {
if (mFinalized) throw new IllegalStateException("Must not call add() after start()");
- mTasks.add(task);
+ mTasks.add(new Pair<>(traits, task));
}
/**
@@ -59,16 +64,17 @@
public void start(final boolean coalesceTasks) {
if (mFinalized) throw new IllegalStateException("Cannot call start() several times");
mFinalized = true;
+ if (mTasks.isEmpty()) return;
if (coalesceTasks) {
- ThreadUtils.runOnUiThread(new Runnable() {
+ PostTask.runOrPostTask(mTasks.peek().first, new Runnable() {
@Override
public void run() {
- for (Runnable task : mTasks) task.run();
+ for (Pair<TaskTraits, Runnable> pair : mTasks) pair.second.run();
mTasks.clear();
}
});
} else {
- ThreadUtils.postOnUiThread(mRunAndPost);
+ PostTask.postTask(mTasks.peek().first, mRunAndPost);
}
}
}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java b/chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java
index 430c73e..3a7c11ba 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java
@@ -279,14 +279,15 @@
// launch its required components.
if (!delegate.startServiceManagerOnly()
&& !ProcessInitializationHandler.getInstance().postNativeInitializationComplete()) {
- tasks.add(() -> ProcessInitializationHandler.getInstance().initializePostNative());
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP,
+ () -> ProcessInitializationHandler.getInstance().initializePostNative());
}
if (!mNetworkChangeNotifierInitializationComplete) {
- tasks.add(this::initNetworkChangeNotifier);
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, this::initNetworkChangeNotifier);
}
- tasks.add(() -> {
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, () -> {
// This is not broken down as a separate task, since this:
// 1. Should happen as early as possible
// 2. Only submits asynchronous work
@@ -299,23 +300,25 @@
onStartNativeInitialization();
});
- tasks.add(() -> {
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, () -> {
if (delegate.isActivityFinishingOrDestroyed()) return;
delegate.initializeCompositor();
});
- tasks.add(() -> {
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, () -> {
if (delegate.isActivityFinishingOrDestroyed()) return;
delegate.initializeState();
});
- if (!mNativeInitializationComplete) tasks.add(this::onFinishNativeInitialization);
-
- tasks.add(() -> {
+ tasks.add(UiThreadTaskTraits.BOOTSTRAP, () -> {
if (delegate.isActivityFinishingOrDestroyed()) return;
+ // Some tasks posted by this are on the critical path.
delegate.startNativeInitialization();
});
+ if (!mNativeInitializationComplete)
+ tasks.add(UiThreadTaskTraits.DEFAULT, this::onFinishNativeInitialization);
+
if (isAsync) {
// We want to start this queue once the C++ startup tasks have run; allow the
// C++ startup to run asynchonously, and set it up to start the Java queue once
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/partnercustomizations/PartnerBrowserCustomizations.java b/chrome/android/java/src/org/chromium/chrome/browser/partnercustomizations/PartnerBrowserCustomizations.java
index 04958d11..db17acb 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/partnercustomizations/PartnerBrowserCustomizations.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/partnercustomizations/PartnerBrowserCustomizations.java
@@ -15,7 +15,6 @@
import org.chromium.base.CommandLine;
import org.chromium.base.ContextUtils;
import org.chromium.base.Log;
-import org.chromium.base.ThreadUtils;
import org.chromium.base.VisibleForTesting;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.task.AsyncTask;
@@ -299,7 +298,8 @@
initializeAsyncTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
// Cancel the initialization if it reaches timeout.
- ThreadUtils.postOnUiThreadDelayed(() -> initializeAsyncTask.cancel(true), timeoutMs);
+ PostTask.postDelayedTask(
+ UiThreadTaskTraits.DEFAULT, () -> initializeAsyncTask.cancel(true), timeoutMs);
}
/**
@@ -325,7 +325,7 @@
public static void setOnInitializeAsyncFinished(final Runnable callback, long timeoutMs) {
sInitializeAsyncCallbacks.add(callback);
- ThreadUtils.postOnUiThreadDelayed(() -> {
+ PostTask.postDelayedTask(UiThreadTaskTraits.DEFAULT, () -> {
if (sInitializeAsyncCallbacks.remove(callback)) callback.run();
}, sIsInitialized ? 0 : timeoutMs);
}
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/init/ChainedTasksTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/init/ChainedTasksTest.java
index 1ab18b2..fc07634d 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/init/ChainedTasksTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/init/ChainedTasksTest.java
@@ -50,7 +50,9 @@
Arrays.asList(new String[] {"First", "Second", "Third"});
final List<String> messages = new ArrayList<>();
final ChainedTasks tasks = new ChainedTasks();
- for (String message : expectedMessages) tasks.add(new TestRunnable(messages, message));
+ for (String message : expectedMessages) {
+ tasks.add(UiThreadTaskTraits.DEFAULT, new TestRunnable(messages, message));
+ }
TestThreadUtils.runOnUiThreadBlocking(() -> {
tasks.start(true);
@@ -65,7 +67,7 @@
final Semaphore finished = new Semaphore(0);
final ChainedTasks tasks = new ChainedTasks();
- tasks.add(new Runnable() {
+ tasks.add(UiThreadTaskTraits.DEFAULT, new Runnable() {
@Override
public void run() {
try {
@@ -78,8 +80,10 @@
List<String> expectedMessages = Arrays.asList(new String[] {"First", "Second", "Third"});
final List<String> messages = new ArrayList<>();
- for (String message : expectedMessages) tasks.add(new TestRunnable(messages, message));
- tasks.add(new Runnable() {
+ for (String message : expectedMessages) {
+ tasks.add(UiThreadTaskTraits.DEFAULT, new TestRunnable(messages, message));
+ }
+ tasks.add(UiThreadTaskTraits.DEFAULT, new Runnable() {
@Override
public void run() {
finished.release();
@@ -102,8 +106,10 @@
final ChainedTasks tasks = new ChainedTasks();
final Semaphore finished = new Semaphore(0);
- for (String message : expectedMessages) tasks.add(new TestRunnable(messages, message));
- tasks.add(new Runnable() {
+ for (String message : expectedMessages) {
+ tasks.add(UiThreadTaskTraits.DEFAULT, new TestRunnable(messages, message));
+ }
+ tasks.add(UiThreadTaskTraits.DEFAULT, new Runnable() {
@Override
public void run() {
finished.release();
@@ -131,9 +137,9 @@
// Posts 2 tasks, waits for a high priority task to be posted from another thread, and
// carries on.
- tasks.add(new TestRunnable(messages, "First"));
- tasks.add(new TestRunnable(messages, "Second"));
- tasks.add(new Runnable() {
+ tasks.add(UiThreadTaskTraits.DEFAULT, new TestRunnable(messages, "First"));
+ tasks.add(UiThreadTaskTraits.DEFAULT, new TestRunnable(messages, "Second"));
+ tasks.add(UiThreadTaskTraits.DEFAULT, new Runnable() {
@Override
public void run() {
try {
@@ -144,8 +150,8 @@
}
}
});
- tasks.add(new TestRunnable(messages, "Third"));
- tasks.add(new Runnable() {
+ tasks.add(UiThreadTaskTraits.DEFAULT, new TestRunnable(messages, "Third"));
+ tasks.add(UiThreadTaskTraits.DEFAULT, new Runnable() {
@Override
public void run() {
finished.release();
@@ -169,16 +175,16 @@
final ChainedTasks tasks = new ChainedTasks();
final Semaphore finished = new Semaphore(0);
- tasks.add(new TestRunnable(messages, "First"));
- tasks.add(new TestRunnable(messages, "Second"));
- tasks.add(new Runnable() {
+ tasks.add(UiThreadTaskTraits.DEFAULT, new TestRunnable(messages, "First"));
+ tasks.add(UiThreadTaskTraits.DEFAULT, new TestRunnable(messages, "Second"));
+ tasks.add(UiThreadTaskTraits.DEFAULT, new Runnable() {
@Override
public void run() {
tasks.cancel();
}
});
- tasks.add(new TestRunnable(messages, "Third"));
- tasks.add(new Runnable() {
+ tasks.add(UiThreadTaskTraits.DEFAULT, new TestRunnable(messages, "Third"));
+ tasks.add(UiThreadTaskTraits.DEFAULT, new Runnable() {
@Override
public void run() {
finished.release();
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/MainIntentBehaviorMetricsIntegrationTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/MainIntentBehaviorMetricsIntegrationTest.java
index beba824..a266180 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/MainIntentBehaviorMetricsIntegrationTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/MainIntentBehaviorMetricsIntegrationTest.java
@@ -11,6 +11,7 @@
import android.annotation.SuppressLint;
import android.content.ComponentName;
import android.content.Intent;
+import android.net.Uri;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.MediumTest;
@@ -116,7 +117,11 @@
@MediumTest
@Test
public void testCreateNtp() {
- startActivity(true);
+ // startActivity(true) creates a NTP which is problematical for this test if
+ // ChromeTabbedActivity.setupCompositorContent runs before that NTP is created because
+ // that creates a SimpleAnimationLayout which tries to hide the page resulting in a
+ // MainIntentActionType.SWITCH_TABS. Starting from about:blank avoids this confusion.
+ startActivityWithAboutBlank(true);
assertMainIntentBehavior(null);
TestThreadUtils.runOnUiThreadBlocking(
() -> mActivityTestRule.getActivity().getTabCreator(false).launchNTP());
@@ -310,6 +315,18 @@
}
}
+ private void startActivityWithAboutBlank(boolean addLauncherCategory) {
+ Intent intent = new Intent(Intent.ACTION_MAIN);
+ intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+ intent.setData(Uri.parse("about:blank"));
+ if (addLauncherCategory) intent.addCategory(Intent.CATEGORY_LAUNCHER);
+ intent.setComponent(new ComponentName(
+ InstrumentationRegistry.getTargetContext(), ChromeTabbedActivity.class));
+
+ mActivityTestRule.startActivityCompletely(intent);
+ mActivityTestRule.waitForActivityNativeInitializationComplete();
+ }
+
private void startActivity(boolean addLauncherCategory) {
Intent intent = new Intent(Intent.ACTION_MAIN);
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerDisableBookmarksEditingUnitTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerDisableBookmarksEditingUnitTest.java
index 971cf9a..3d341c3 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerDisableBookmarksEditingUnitTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerDisableBookmarksEditingUnitTest.java
@@ -21,6 +21,7 @@
import org.chromium.base.test.util.Feature;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.partnercustomizations.TestPartnerBrowserCustomizationsDelayedProvider;
import org.chromium.chrome.test.partnercustomizations.TestPartnerBrowserCustomizationsProvider;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
@@ -140,13 +141,12 @@
TestThreadUtils.runOnUiThreadBlocking(() -> {
PartnerBrowserCustomizations.initializeAsync(mTestRule.getContextWrapper(), 2000);
});
- PartnerBrowserCustomizations.setOnInitializeAsyncFinished(mTestRule.getCallback(), 300);
-
- mTestRule.getCallbackLock().acquire();
+ PartnerBrowserCustomizations.setOnInitializeAsyncFinished(mTestRule.getCallback());
Assert.assertFalse(PartnerBrowserCustomizations.isInitialized());
Assert.assertFalse(PartnerBrowserCustomizations.isBookmarksEditingDisabled());
+ TestPartnerBrowserCustomizationsDelayedProvider.unblockQuery();
PartnerBrowserCustomizations.setOnInitializeAsyncFinished(mTestRule.getCallback(), 3000);
mTestRule.getCallbackLock().acquire();
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerDisableIncognitoModeUnitTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerDisableIncognitoModeUnitTest.java
index 1cbd880..324cf389 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerDisableIncognitoModeUnitTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerDisableIncognitoModeUnitTest.java
@@ -21,6 +21,7 @@
import org.chromium.base.test.util.Feature;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.partnercustomizations.TestPartnerBrowserCustomizationsDelayedProvider;
import org.chromium.chrome.test.partnercustomizations.TestPartnerBrowserCustomizationsProvider;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
@@ -140,13 +141,12 @@
TestThreadUtils.runOnUiThreadBlocking(() -> {
PartnerBrowserCustomizations.initializeAsync(mTestRule.getContextWrapper(), 2000);
});
- PartnerBrowserCustomizations.setOnInitializeAsyncFinished(mTestRule.getCallback(), 300);
-
- mTestRule.getCallbackLock().acquire();
+ PartnerBrowserCustomizations.setOnInitializeAsyncFinished(mTestRule.getCallback());
Assert.assertFalse(PartnerBrowserCustomizations.isInitialized());
Assert.assertFalse(PartnerBrowserCustomizations.isIncognitoDisabled());
+ TestPartnerBrowserCustomizationsDelayedProvider.unblockQuery();
PartnerBrowserCustomizations.setOnInitializeAsyncFinished(mTestRule.getCallback(), 3000);
mTestRule.getCallbackLock().acquire();
diff --git a/chrome/test/android/javatests/src/org/chromium/chrome/test/partnercustomizations/TestPartnerBrowserCustomizationsDelayedProvider.java b/chrome/test/android/javatests/src/org/chromium/chrome/test/partnercustomizations/TestPartnerBrowserCustomizationsDelayedProvider.java
index cb7d750..f4a4750 100644
--- a/chrome/test/android/javatests/src/org/chromium/chrome/test/partnercustomizations/TestPartnerBrowserCustomizationsDelayedProvider.java
+++ b/chrome/test/android/javatests/src/org/chromium/chrome/test/partnercustomizations/TestPartnerBrowserCustomizationsDelayedProvider.java
@@ -12,6 +12,7 @@
import org.chromium.base.annotations.MainDex;
import java.util.List;
+import java.util.concurrent.CountDownLatch;
/**
* PartnerBrowserCustomizationsProvider example for testing. This adds one second latency for
@@ -22,14 +23,20 @@
public class TestPartnerBrowserCustomizationsDelayedProvider
extends TestPartnerBrowserCustomizationsProvider {
private static String sUriPathToDelay;
+ private static CountDownLatch sLatch;
public TestPartnerBrowserCustomizationsDelayedProvider() {
super();
mTag = TestPartnerBrowserCustomizationsDelayedProvider.class.getSimpleName();
}
+ public static void unblockQuery() {
+ sLatch.countDown();
+ }
+
private void setUriPathToDelay(String path) {
sUriPathToDelay = path;
+ sLatch = new CountDownLatch(1);
}
@Override
@@ -48,7 +55,7 @@
if (sUriPathToDelay == null
|| (pathSegments != null && !pathSegments.isEmpty()
&& TextUtils.equals(pathSegments.get(0), sUriPathToDelay))) {
- Thread.sleep(1000);
+ sLatch.await();
}
} catch (InterruptedException e) {
assert false;