Commit f3a946b6 authored by Henrique Nakashima's avatar Henrique Nakashima Committed by Commit Bot

ChromeBrowserInitializer offers runNowOrAfterFullBrowserStarted().

Reland of
https://chromium-review.googlesource.com/c/chromium/src/+/2032049
with fixes so avoid failing ChromeCustomTabsConnectionTest
(crbug.com/1048438).

Instead of runNowOrAfterNativeInitialization(), the method should
enqueue tasks to run when full browser starts, and not when native is
started. There is a distinction - reduced mode means native is started but full browser, which includes Profiles, isn't.

This fixes a crash when transitioning from reduced mode to full browser if a task that uses Profile was scheduled with
runNowOrAfterNativeInitialization(). Now, it will only run after the
transition, rather than immediately.

Also change hasNativeInitializationCompleted() to
isFullBrowserInitialized() and update clients. All but DownloadMetrics are meant to run in full browser, so that should actually revert their behavior back to what it was before reduced mode was introduced.

DownloadMetrics still checks if native was loaded, which includes in
reduced mode.

Bug: 1045945, 1014098, 1048438
Change-Id: If99179a5720d7e550ee5653a8625021f58fc262e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037818Reviewed-by: default avatarMohamed Heikal <mheikal@chromium.org>
Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738370}
parent e35c9618
...@@ -38,11 +38,10 @@ public class ClearDataDialogResultRecorder { ...@@ -38,11 +38,10 @@ public class ClearDataDialogResultRecorder {
* @param triggeredByUninstall Whether the dialog was triggered by uninstall. * @param triggeredByUninstall Whether the dialog was triggered by uninstall.
*/ */
public void handleDialogResult(boolean accepted, boolean triggeredByUninstall) { public void handleDialogResult(boolean accepted, boolean triggeredByUninstall) {
if (accepted || mBrowserInitializer.hasNativeInitializationCompleted()) { if (accepted || mBrowserInitializer.isFullBrowserInitialized()) {
// If accepted, native is going to be loaded for the settings. // If accepted, native is going to be loaded for the settings.
mBrowserInitializer.runNowOrAfterNativeInitialization(() -> mBrowserInitializer.runNowOrAfterFullBrowserStarted(
mUmaRecorder.recordClearDataDialogAction(accepted, () -> mUmaRecorder.recordClearDataDialogAction(accepted, triggeredByUninstall));
triggeredByUninstall));
} else { } else {
// Avoid loading native just for the sake of recording. Save the info and record // Avoid loading native just for the sake of recording. Save the info and record
// on next Chrome launch. // on next Chrome launch.
......
...@@ -159,6 +159,6 @@ public class TrustedWebActivityUmaRecorder { ...@@ -159,6 +159,6 @@ public class TrustedWebActivityUmaRecorder {
} }
private void doWhenNativeLoaded(Runnable runnable) { private void doWhenNativeLoaded(Runnable runnable) {
mBrowserInitializer.runNowOrAfterNativeInitialization(runnable); mBrowserInitializer.runNowOrAfterFullBrowserStarted(runnable);
} }
} }
...@@ -811,7 +811,7 @@ public class CustomTabsConnection { ...@@ -811,7 +811,7 @@ public class CustomTabsConnection {
if (mWarmupTasks != null) mWarmupTasks.cancel(); if (mWarmupTasks != null) mWarmupTasks.cancel();
maybePreconnectToRedirectEndpoint(session, url, intent); maybePreconnectToRedirectEndpoint(session, url, intent);
ChromeBrowserInitializer.getInstance().runNowOrAfterNativeInitialization( ChromeBrowserInitializer.getInstance().runNowOrAfterFullBrowserStarted(
() -> handleParallelRequest(session, intent)); () -> handleParallelRequest(session, intent));
maybePrefetchResources(session, intent); maybePrefetchResources(session, intent);
} }
...@@ -819,7 +819,7 @@ public class CustomTabsConnection { ...@@ -819,7 +819,7 @@ public class CustomTabsConnection {
private void maybePreconnectToRedirectEndpoint( private void maybePreconnectToRedirectEndpoint(
CustomTabsSessionToken session, String url, Intent intent) { CustomTabsSessionToken session, String url, Intent intent) {
// For the preconnection to not be a no-op, we need more than just the native library. // For the preconnection to not be a no-op, we need more than just the native library.
if (!ChromeBrowserInitializer.getInstance().hasNativeInitializationCompleted()) { if (!ChromeBrowserInitializer.getInstance().isFullBrowserInitialized()) {
return; return;
} }
if (!ChromeFeatureList.isEnabled(ChromeFeatureList.CCT_REDIRECT_PRECONNECT)) return; if (!ChromeFeatureList.isEnabled(ChromeFeatureList.CCT_REDIRECT_PRECONNECT)) return;
...@@ -878,7 +878,7 @@ public class CustomTabsConnection { ...@@ -878,7 +878,7 @@ public class CustomTabsConnection {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
if (!intent.hasExtra(PARALLEL_REQUEST_URL_KEY)) return ParallelRequestStatus.NO_REQUEST; if (!intent.hasExtra(PARALLEL_REQUEST_URL_KEY)) return ParallelRequestStatus.NO_REQUEST;
if (!ChromeBrowserInitializer.getInstance().hasNativeInitializationCompleted()) { if (!ChromeBrowserInitializer.getInstance().isFullBrowserInitialized()) {
return ParallelRequestStatus.FAILURE_NOT_INITIALIZED; return ParallelRequestStatus.FAILURE_NOT_INITIALIZED;
} }
if (!mClientManager.getAllowParallelRequestForSession(session)) { if (!mClientManager.getAllowParallelRequestForSession(session)) {
......
...@@ -196,7 +196,7 @@ public class CustomTabActivityNavigationController implements StartStopWithNativ ...@@ -196,7 +196,7 @@ public class CustomTabActivityNavigationController implements StartStopWithNativ
* Handles back button navigation. * Handles back button navigation.
*/ */
public boolean navigateOnBack() { public boolean navigateOnBack() {
if (!mChromeBrowserInitializer.hasNativeInitializationCompleted()) return false; if (!mChromeBrowserInitializer.isFullBrowserInitialized()) return false;
RecordUserAction.record("CustomTabs.SystemBack"); RecordUserAction.record("CustomTabs.SystemBack");
if (mTabProvider.getTab() == null) return false; if (mTabProvider.getTab() == null) return false;
......
...@@ -5,8 +5,9 @@ ...@@ -5,8 +5,9 @@
package org.chromium.chrome.browser.download; package org.chromium.chrome.browser.download;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.library_loader.LibraryProcessType;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.browser.init.ChromeBrowserInitializer; import org.chromium.content_public.browser.BrowserStartupController;
import java.util.ArrayList; import java.util.ArrayList;
...@@ -99,6 +100,6 @@ public class DownloadMetrics { ...@@ -99,6 +100,6 @@ public class DownloadMetrics {
} }
private static boolean isNativeLoaded() { private static boolean isNativeLoaded() {
return ChromeBrowserInitializer.getInstance().hasNativeInitializationCompleted(); return BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER).isNativeStarted();
} }
} }
...@@ -110,7 +110,7 @@ public class HomepagePolicyManager implements PrefObserver { ...@@ -110,7 +110,7 @@ public class HomepagePolicyManager implements PrefObserver {
mIsHomepageLocationPolicyEnabled = !TextUtils.isEmpty(mHomepage); mIsHomepageLocationPolicyEnabled = !TextUtils.isEmpty(mHomepage);
if (isFeatureFlagEnabled()) { if (isFeatureFlagEnabled()) {
ChromeBrowserInitializer.getInstance().runNowOrAfterNativeInitialization( ChromeBrowserInitializer.getInstance().runNowOrAfterFullBrowserStarted(
this::onFinishNativeInitialization); this::onFinishNativeInitialization);
} }
} }
......
...@@ -66,11 +66,12 @@ public class ChromeBrowserInitializer { ...@@ -66,11 +66,12 @@ public class ChromeBrowserInitializer {
private static ChromeBrowserInitializer sChromeBrowserInitializer; private static ChromeBrowserInitializer sChromeBrowserInitializer;
private static BrowserStartupController sBrowserStartupController; private static BrowserStartupController sBrowserStartupController;
private final Locale mInitialLocale = Locale.getDefault(); private final Locale mInitialLocale = Locale.getDefault();
private List<Runnable> mTasksToRunWithNative; private List<Runnable> mTasksToRunWithFullBrowser;
private boolean mPreInflationStartupComplete; private boolean mPreInflationStartupComplete;
private boolean mPostInflationStartupComplete; private boolean mPostInflationStartupComplete;
private boolean mNativeInitializationComplete; private boolean mNativeInitializationComplete;
private boolean mFullBrowserInitializationComplete;
private boolean mNetworkChangeNotifierInitializationComplete; private boolean mNetworkChangeNotifierInitializationComplete;
/** /**
...@@ -85,27 +86,36 @@ public class ChromeBrowserInitializer { ...@@ -85,27 +86,36 @@ public class ChromeBrowserInitializer {
} }
/** /**
* @return whether native initialization is complete. * @return whether native (full browser) initialization is complete.
*/ */
public boolean isFullBrowserInitialized() {
return mFullBrowserInitializationComplete;
}
/**
* @deprecated use isFullBrowserInitialized() instead, the name hasNativeInitializationCompleted
* is not accurate.
*/
@Deprecated
public boolean hasNativeInitializationCompleted() { public boolean hasNativeInitializationCompleted() {
return mNativeInitializationComplete; return isFullBrowserInitialized();
} }
/** /**
* Either runs a task now, or queue it until native initialization is done. * Either runs a task now, or queue it until native (full browser) initialization is done.
* *
* All Runnables added this way will run in a single UI thread task. * All Runnables added this way will run in a single UI thread task.
* *
* @param task The task to run. * @param task The task to run.
*/ */
public void runNowOrAfterNativeInitialization(Runnable task) { public void runNowOrAfterFullBrowserStarted(Runnable task) {
if (hasNativeInitializationCompleted()) { if (isFullBrowserInitialized()) {
task.run(); task.run();
} else { } else {
if (mTasksToRunWithNative == null) { if (mTasksToRunWithFullBrowser == null) {
mTasksToRunWithNative = new ArrayList<Runnable>(); mTasksToRunWithFullBrowser = new ArrayList<Runnable>();
} }
mTasksToRunWithNative.add(task); mTasksToRunWithFullBrowser.add(task);
} }
} }
...@@ -293,6 +303,10 @@ public class ChromeBrowserInitializer { ...@@ -293,6 +303,10 @@ public class ChromeBrowserInitializer {
tasks.add(UiThreadTaskTraits.DEFAULT, this::onFinishNativeInitialization); tasks.add(UiThreadTaskTraits.DEFAULT, this::onFinishNativeInitialization);
} }
if (!delegate.startServiceManagerOnly()) {
tasks.add(UiThreadTaskTraits.DEFAULT, this::onFinishFullBrowserInitialization);
}
int startupMode = int startupMode =
getBrowserStartupController().getStartupMode(delegate.startServiceManagerOnly()); getBrowserStartupController().getStartupMode(delegate.startServiceManagerOnly());
tasks.add(UiThreadTaskTraits.DEFAULT, () -> { tasks.add(UiThreadTaskTraits.DEFAULT, () -> {
...@@ -378,6 +392,15 @@ public class ChromeBrowserInitializer { ...@@ -378,6 +392,15 @@ public class ChromeBrowserInitializer {
SpeechRecognition.initialize(); SpeechRecognition.initialize();
} }
private void onFinishFullBrowserInitialization() {
mFullBrowserInitializationComplete = true;
if (mTasksToRunWithFullBrowser != null) {
for (Runnable r : mTasksToRunWithFullBrowser) r.run();
mTasksToRunWithFullBrowser = null;
}
}
private void onFinishNativeInitialization() { private void onFinishNativeInitialization() {
if (mNativeInitializationComplete) return; if (mNativeInitializationComplete) return;
...@@ -406,10 +429,6 @@ public class ChromeBrowserInitializer { ...@@ -406,10 +429,6 @@ public class ChromeBrowserInitializer {
}); });
MemoryPressureUma.initializeForBrowser(); MemoryPressureUma.initializeForBrowser();
if (mTasksToRunWithNative != null) {
for (Runnable r : mTasksToRunWithNative) r.run();
mTasksToRunWithNative = null;
}
// Needed for field trial metrics to be properly collected in ServiceManager only mode. // Needed for field trial metrics to be properly collected in ServiceManager only mode.
ChromeCachedFlags.getInstance().cacheServiceManagerOnlyFlags(); ChromeCachedFlags.getInstance().cacheServiceManagerOnlyFlags();
......
...@@ -106,14 +106,14 @@ public class AutoFetchNotifier { ...@@ -106,14 +106,14 @@ public class AutoFetchNotifier {
return; return;
} }
// Chrome may or may not be running. Use runNowOrAfterNativeInitialization() to trigger // Chrome may or may not be running. Use runNowOrAfterFullBrowserStarted() to trigger
// the cancellation if Chrome is running. If Chrome isn't running, // the cancellation if Chrome is running in full browser. If Chrome isn't running in
// runNowOrAfterNativeInitialization() will never call our runnable, so set a pref to // full browser, runNowOrAfterFullBrowserStarted() will never call our runnable, so set
// remember to cancel on next startup. // a pref to remember to cancel on next startup.
SharedPreferencesManager.getInstance().writeInt( SharedPreferencesManager.getInstance().writeInt(
ChromePreferenceKeys.OFFLINE_AUTO_FETCH_USER_CANCEL_ACTION_IN_PROGRESS, action); ChromePreferenceKeys.OFFLINE_AUTO_FETCH_USER_CANCEL_ACTION_IN_PROGRESS, action);
// This will call us back with cancellationComplete(). // This will call us back with cancellationComplete().
ChromeBrowserInitializer.getInstance().runNowOrAfterNativeInitialization( ChromeBrowserInitializer.getInstance().runNowOrAfterFullBrowserStarted(
AutoFetchNotifier::cancelInProgress); AutoFetchNotifier::cancelInProgress);
// Finally, whether chrome is running or not, remove the notification. // Finally, whether chrome is running or not, remove the notification.
closeInProgressNotification(); closeInProgressNotification();
......
...@@ -126,7 +126,7 @@ public class ChromeGcmListenerService extends GcmListenerService { ...@@ -126,7 +126,7 @@ public class ChromeGcmListenerService extends GcmListenerService {
* the next time Chrome is launched into foreground. * the next time Chrome is launched into foreground.
*/ */
private static boolean maybePersistLazyMessage(GCMMessage message) { private static boolean maybePersistLazyMessage(GCMMessage message) {
if (isNativeLoaded()) { if (isFullBrowserLoaded()) {
return false; return false;
} }
...@@ -221,7 +221,7 @@ public class ChromeGcmListenerService extends GcmListenerService { ...@@ -221,7 +221,7 @@ public class ChromeGcmListenerService extends GcmListenerService {
GCMDriver.dispatchMessage(message); GCMDriver.dispatchMessage(message);
} }
private static boolean isNativeLoaded() { private static boolean isFullBrowserLoaded() {
return ChromeBrowserInitializer.getInstance().hasNativeInitializationCompleted(); return ChromeBrowserInitializer.getInstance().isFullBrowserInitialized();
} }
} }
...@@ -25,16 +25,16 @@ public class ChromeBrowserInitializerTest { ...@@ -25,16 +25,16 @@ public class ChromeBrowserInitializerTest {
@Before @Before
public void setUp() { public void setUp() {
mInstance = ChromeBrowserInitializer.getInstance(); mInstance = ChromeBrowserInitializer.getInstance();
Assert.assertFalse(mInstance.hasNativeInitializationCompleted()); Assert.assertFalse(mInstance.isFullBrowserInitialized());
} }
@Test @Test
@SmallTest @SmallTest
public void testSynchronousInitialization() throws Exception { public void testSynchronousInitialization() throws Exception {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertFalse(mInstance.hasNativeInitializationCompleted()); Assert.assertFalse(mInstance.isFullBrowserInitialized());
mInstance.handleSynchronousStartup(); mInstance.handleSynchronousStartup();
Assert.assertTrue(mInstance.hasNativeInitializationCompleted()); Assert.assertTrue(mInstance.isFullBrowserInitialized());
return true; return true;
}); });
} }
...@@ -50,17 +50,16 @@ public class ChromeBrowserInitializerTest { ...@@ -50,17 +50,16 @@ public class ChromeBrowserInitializerTest {
} }
}; };
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertFalse(mInstance.hasNativeInitializationCompleted()); Assert.assertFalse(mInstance.isFullBrowserInitialized());
mInstance.handlePreNativeStartup(parts); mInstance.handlePreNativeStartup(parts);
mInstance.handlePostNativeStartup(true, parts); mInstance.handlePostNativeStartup(true, parts);
Assert.assertFalse( Assert.assertFalse("Should not be synchronous", mInstance.isFullBrowserInitialized());
"Should not be synchronous", mInstance.hasNativeInitializationCompleted());
return true; return true;
}); });
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertFalse("Inititialization tasks should yield to new UI thread tasks", Assert.assertFalse("Inititialization tasks should yield to new UI thread tasks",
mInstance.hasNativeInitializationCompleted()); mInstance.isFullBrowserInitialized());
}); });
Assert.assertTrue(done.tryAcquire(10, TimeUnit.SECONDS)); Assert.assertTrue(done.tryAcquire(10, TimeUnit.SECONDS));
} }
...@@ -70,14 +69,14 @@ public class ChromeBrowserInitializerTest { ...@@ -70,14 +69,14 @@ public class ChromeBrowserInitializerTest {
public void testDelayedTasks() throws Exception { public void testDelayedTasks() throws Exception {
final Semaphore done = new Semaphore(0); final Semaphore done = new Semaphore(0);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mInstance.runNowOrAfterNativeInitialization(done::release); mInstance.runNowOrAfterFullBrowserStarted(done::release);
Assert.assertFalse("Should not run synchronously", done.tryAcquire()); Assert.assertFalse("Should not run synchronously", done.tryAcquire());
mInstance.handleSynchronousStartup(); mInstance.handleSynchronousStartup();
Assert.assertTrue(done.tryAcquire()); Assert.assertTrue(done.tryAcquire());
return true; return true;
}); });
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mInstance.runNowOrAfterNativeInitialization(done::release); mInstance.runNowOrAfterFullBrowserStarted(done::release);
// Runs right away in the same task is initialization is done. // Runs right away in the same task is initialization is done.
Assert.assertTrue(done.tryAcquire()); Assert.assertTrue(done.tryAcquire());
}); });
......
...@@ -101,9 +101,10 @@ public class ClearDataDialogResultRecorderTest { ...@@ -101,9 +101,10 @@ public class ClearDataDialogResultRecorderTest {
} }
private void restartApp() { private void restartApp() {
when(mBrowserInitializer.hasNativeInitializationCompleted()).thenReturn(false); when(mBrowserInitializer.isFullBrowserInitialized()).thenReturn(false);
doNothing().when(mBrowserInitializer).runNowOrAfterNativeInitialization( doNothing()
mTaskOnNativeInitCaptor.capture()); .when(mBrowserInitializer)
.runNowOrAfterFullBrowserStarted(mTaskOnNativeInitCaptor.capture());
mRecorder = new ClearDataDialogResultRecorder(() -> mPrefsManager, mBrowserInitializer, mRecorder = new ClearDataDialogResultRecorder(() -> mPrefsManager, mBrowserInitializer,
mUmaRecorder); mUmaRecorder);
} }
...@@ -112,9 +113,10 @@ public class ClearDataDialogResultRecorderTest { ...@@ -112,9 +113,10 @@ public class ClearDataDialogResultRecorderTest {
for (Runnable task : mTaskOnNativeInitCaptor.getAllValues()) { for (Runnable task : mTaskOnNativeInitCaptor.getAllValues()) {
task.run(); task.run();
} }
when(mBrowserInitializer.hasNativeInitializationCompleted()).thenReturn(true); when(mBrowserInitializer.isFullBrowserInitialized()).thenReturn(true);
doAnswer(answerVoid(Runnable::run)).when(mBrowserInitializer) doAnswer(answerVoid(Runnable::run))
.runNowOrAfterNativeInitialization(any()); .when(mBrowserInitializer)
.runNowOrAfterFullBrowserStarted(any());
} }
} }
...@@ -122,7 +122,7 @@ public class CustomTabActivityContentTestEnvironment extends TestWatcher { ...@@ -122,7 +122,7 @@ public class CustomTabActivityContentTestEnvironment extends TestWatcher {
when(tabFactory.getTabModelSelector()).thenReturn(tabModelSelector); when(tabFactory.getTabModelSelector()).thenReturn(tabModelSelector);
when(tabModelSelector.getModel(anyBoolean())).thenReturn(tabModel); when(tabModelSelector.getModel(anyBoolean())).thenReturn(tabModel);
when(connection.getSpeculatedUrl(any())).thenReturn(SPECULATED_URL); when(connection.getSpeculatedUrl(any())).thenReturn(SPECULATED_URL);
when(browserInitializer.hasNativeInitializationCompleted()).thenReturn(true); when(browserInitializer.isFullBrowserInitialized()).thenReturn(true);
// Default setup is toolbarManager doesn't consume back press event. // Default setup is toolbarManager doesn't consume back press event.
when(toolbarManager.back()).thenReturn(false); when(toolbarManager.back()).thenReturn(false);
......
...@@ -194,9 +194,9 @@ public class ChromeActivityTestRule<T extends ChromeActivity> extends ActivityTe ...@@ -194,9 +194,9 @@ public class ChromeActivityTestRule<T extends ChromeActivity> extends ActivityTe
* @param activity The {@link ChromeActivity} to wait for. * @param activity The {@link ChromeActivity} to wait for.
*/ */
public static void waitForActivityNativeInitializationComplete(ChromeActivity activity) { public static void waitForActivityNativeInitializationComplete(ChromeActivity activity) {
CriteriaHelper.pollUiThread(() CriteriaHelper.pollUiThread(
-> ChromeBrowserInitializer.getInstance() ()
.hasNativeInitializationCompleted(), -> ChromeBrowserInitializer.getInstance().isFullBrowserInitialized(),
"Native initialization never finished", "Native initialization never finished",
20 * CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL, 20 * CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL,
CriteriaHelper.DEFAULT_POLLING_INTERVAL); CriteriaHelper.DEFAULT_POLLING_INTERVAL);
......
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