Commit 3d70863b authored by Henrique Nakashima's avatar Henrique Nakashima Committed by Commit Bot

Revert "ChromeBrowserInitializer offers runNowOrAfterFullBrowserStarted()."

This reverts commit 628f57c2.

Reason for revert: Breaks ChromeCustomTabsConnectionTest
(crbug.com/1048438). Might have broken other things. Will
reland a fixed version.

Original change's description:
> ChromeBrowserInitializer offers runNowOrAfterFullBrowserStarted().
>
> 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
> Change-Id: Icdeed1ad6a1999fe518c551c404285170a7118e5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032049
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
> Reviewed-by: Xi Han <hanxi@chromium.org>
> Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#737823}

TBR=mthiesse@chromium.org,hanxi@chromium.org,hnakashima@chromium.org,mheikal@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1045945, 1014098
Change-Id: Ia9f37440cd2c39fc73d74cae8dc52572a01fe742
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037644
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: default avatarHenrique Nakashima <hnakashima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738335}
parent 919ba484
...@@ -38,10 +38,11 @@ public class ClearDataDialogResultRecorder { ...@@ -38,10 +38,11 @@ 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.isFullBrowserInitialized()) { if (accepted || mBrowserInitializer.hasNativeInitializationCompleted()) {
// If accepted, native is going to be loaded for the settings. // If accepted, native is going to be loaded for the settings.
mBrowserInitializer.runNowOrAfterFullBrowserStarted( mBrowserInitializer.runNowOrAfterNativeInitialization(() ->
() -> mUmaRecorder.recordClearDataDialogAction(accepted, triggeredByUninstall)); mUmaRecorder.recordClearDataDialogAction(accepted,
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.runNowOrAfterFullBrowserStarted(runnable); mBrowserInitializer.runNowOrAfterNativeInitialization(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().runNowOrAfterFullBrowserStarted( ChromeBrowserInitializer.getInstance().runNowOrAfterNativeInitialization(
() -> 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().isFullBrowserInitialized()) { if (!ChromeBrowserInitializer.getInstance().hasNativeInitializationCompleted()) {
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().isFullBrowserInitialized()) { if (!ChromeBrowserInitializer.getInstance().hasNativeInitializationCompleted()) {
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.isFullBrowserInitialized()) return false; if (!mChromeBrowserInitializer.hasNativeInitializationCompleted()) return false;
RecordUserAction.record("CustomTabs.SystemBack"); RecordUserAction.record("CustomTabs.SystemBack");
if (mTabProvider.getTab() == null) return false; if (mTabProvider.getTab() == null) return false;
......
...@@ -5,9 +5,8 @@ ...@@ -5,9 +5,8 @@
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.content_public.browser.BrowserStartupController; import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
import java.util.ArrayList; import java.util.ArrayList;
...@@ -100,6 +99,6 @@ public class DownloadMetrics { ...@@ -100,6 +99,6 @@ public class DownloadMetrics {
} }
private static boolean isNativeLoaded() { private static boolean isNativeLoaded() {
return BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER).isNativeStarted(); return ChromeBrowserInitializer.getInstance().hasNativeInitializationCompleted();
} }
} }
...@@ -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().runNowOrAfterFullBrowserStarted( ChromeBrowserInitializer.getInstance().runNowOrAfterNativeInitialization(
this::onFinishNativeInitialization); this::onFinishNativeInitialization);
} }
} }
......
...@@ -66,7 +66,7 @@ public class ChromeBrowserInitializer { ...@@ -66,7 +66,7 @@ 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> mTasksToRunWithFullBrowser; private List<Runnable> mTasksToRunWithNative;
private boolean mPreInflationStartupComplete; private boolean mPreInflationStartupComplete;
private boolean mPostInflationStartupComplete; private boolean mPostInflationStartupComplete;
...@@ -85,37 +85,27 @@ public class ChromeBrowserInitializer { ...@@ -85,37 +85,27 @@ public class ChromeBrowserInitializer {
} }
/** /**
* @return whether native (full browser) initialization is complete. * @return whether native initialization is complete.
*/ */
public boolean isFullBrowserInitialized() {
return mNativeInitializationComplete
&& getBrowserStartupController().isFullBrowserStarted();
}
/**
* @deprecated use isFullBrowserInitialized() instead, the name hasNativeInitializationCompleted
* is not accurate.
*/
@Deprecated
public boolean hasNativeInitializationCompleted() { public boolean hasNativeInitializationCompleted() {
return isFullBrowserInitialized(); return mNativeInitializationComplete;
} }
/** /**
* Either runs a task now, or queue it until native (full browser) initialization is done. * 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. * 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 runNowOrAfterFullBrowserStarted(Runnable task) { public void runNowOrAfterNativeInitialization(Runnable task) {
if (isFullBrowserInitialized()) { if (hasNativeInitializationCompleted()) {
task.run(); task.run();
} else { } else {
if (mTasksToRunWithFullBrowser == null) { if (mTasksToRunWithNative == null) {
mTasksToRunWithFullBrowser = new ArrayList<Runnable>(); mTasksToRunWithNative = new ArrayList<Runnable>();
} }
mTasksToRunWithFullBrowser.add(task); mTasksToRunWithNative.add(task);
} }
} }
...@@ -303,10 +293,6 @@ public class ChromeBrowserInitializer { ...@@ -303,10 +293,6 @@ public class ChromeBrowserInitializer {
tasks.add(UiThreadTaskTraits.DEFAULT, this::onFinishNativeInitialization); tasks.add(UiThreadTaskTraits.DEFAULT, this::onFinishNativeInitialization);
} }
if (!delegate.startServiceManagerOnly()) {
tasks.add(UiThreadTaskTraits.DEFAULT, this::runQueuedTasksWithFullBrowser);
}
int startupMode = int startupMode =
getBrowserStartupController().getStartupMode(delegate.startServiceManagerOnly()); getBrowserStartupController().getStartupMode(delegate.startServiceManagerOnly());
tasks.add(UiThreadTaskTraits.DEFAULT, () -> { tasks.add(UiThreadTaskTraits.DEFAULT, () -> {
...@@ -392,13 +378,6 @@ public class ChromeBrowserInitializer { ...@@ -392,13 +378,6 @@ public class ChromeBrowserInitializer {
SpeechRecognition.initialize(); SpeechRecognition.initialize();
} }
private void runQueuedTasksWithFullBrowser() {
if (mTasksToRunWithFullBrowser != null) {
for (Runnable r : mTasksToRunWithFullBrowser) r.run();
mTasksToRunWithFullBrowser = null;
}
}
private void onFinishNativeInitialization() { private void onFinishNativeInitialization() {
if (mNativeInitializationComplete) return; if (mNativeInitializationComplete) return;
...@@ -427,6 +406,10 @@ public class ChromeBrowserInitializer { ...@@ -427,6 +406,10 @@ 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 runNowOrAfterFullBrowserStarted() to trigger // Chrome may or may not be running. Use runNowOrAfterNativeInitialization() to trigger
// the cancellation if Chrome is running in full browser. If Chrome isn't running in // the cancellation if Chrome is running. If Chrome isn't running,
// full browser, runNowOrAfterFullBrowserStarted() will never call our runnable, so set // runNowOrAfterNativeInitialization() will never call our runnable, so set a pref to
// a pref to remember to cancel on next startup. // 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().runNowOrAfterFullBrowserStarted( ChromeBrowserInitializer.getInstance().runNowOrAfterNativeInitialization(
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 (isFullBrowserLoaded()) { if (isNativeLoaded()) {
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 isFullBrowserLoaded() { private static boolean isNativeLoaded() {
return ChromeBrowserInitializer.getInstance().isFullBrowserInitialized(); return ChromeBrowserInitializer.getInstance().hasNativeInitializationCompleted();
} }
} }
...@@ -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.isFullBrowserInitialized()); Assert.assertFalse(mInstance.hasNativeInitializationCompleted());
} }
@Test @Test
@SmallTest @SmallTest
public void testSynchronousInitialization() throws Exception { public void testSynchronousInitialization() throws Exception {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertFalse(mInstance.isFullBrowserInitialized()); Assert.assertFalse(mInstance.hasNativeInitializationCompleted());
mInstance.handleSynchronousStartup(); mInstance.handleSynchronousStartup();
Assert.assertTrue(mInstance.isFullBrowserInitialized()); Assert.assertTrue(mInstance.hasNativeInitializationCompleted());
return true; return true;
}); });
} }
...@@ -50,16 +50,17 @@ public class ChromeBrowserInitializerTest { ...@@ -50,16 +50,17 @@ public class ChromeBrowserInitializerTest {
} }
}; };
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertFalse(mInstance.isFullBrowserInitialized()); Assert.assertFalse(mInstance.hasNativeInitializationCompleted());
mInstance.handlePreNativeStartup(parts); mInstance.handlePreNativeStartup(parts);
mInstance.handlePostNativeStartup(true, parts); mInstance.handlePostNativeStartup(true, parts);
Assert.assertFalse("Should not be synchronous", mInstance.isFullBrowserInitialized()); Assert.assertFalse(
"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.isFullBrowserInitialized()); mInstance.hasNativeInitializationCompleted());
}); });
Assert.assertTrue(done.tryAcquire(10, TimeUnit.SECONDS)); Assert.assertTrue(done.tryAcquire(10, TimeUnit.SECONDS));
} }
...@@ -69,14 +70,14 @@ public class ChromeBrowserInitializerTest { ...@@ -69,14 +70,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.runNowOrAfterFullBrowserStarted(done::release); mInstance.runNowOrAfterNativeInitialization(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.runNowOrAfterFullBrowserStarted(done::release); mInstance.runNowOrAfterNativeInitialization(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,10 +101,9 @@ public class ClearDataDialogResultRecorderTest { ...@@ -101,10 +101,9 @@ public class ClearDataDialogResultRecorderTest {
} }
private void restartApp() { private void restartApp() {
when(mBrowserInitializer.isFullBrowserInitialized()).thenReturn(false); when(mBrowserInitializer.hasNativeInitializationCompleted()).thenReturn(false);
doNothing() doNothing().when(mBrowserInitializer).runNowOrAfterNativeInitialization(
.when(mBrowserInitializer) mTaskOnNativeInitCaptor.capture());
.runNowOrAfterFullBrowserStarted(mTaskOnNativeInitCaptor.capture());
mRecorder = new ClearDataDialogResultRecorder(() -> mPrefsManager, mBrowserInitializer, mRecorder = new ClearDataDialogResultRecorder(() -> mPrefsManager, mBrowserInitializer,
mUmaRecorder); mUmaRecorder);
} }
...@@ -113,10 +112,9 @@ public class ClearDataDialogResultRecorderTest { ...@@ -113,10 +112,9 @@ public class ClearDataDialogResultRecorderTest {
for (Runnable task : mTaskOnNativeInitCaptor.getAllValues()) { for (Runnable task : mTaskOnNativeInitCaptor.getAllValues()) {
task.run(); task.run();
} }
when(mBrowserInitializer.isFullBrowserInitialized()).thenReturn(true); when(mBrowserInitializer.hasNativeInitializationCompleted()).thenReturn(true);
doAnswer(answerVoid(Runnable::run)) doAnswer(answerVoid(Runnable::run)).when(mBrowserInitializer)
.when(mBrowserInitializer) .runNowOrAfterNativeInitialization(any());
.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.isFullBrowserInitialized()).thenReturn(true); when(browserInitializer.hasNativeInitializationCompleted()).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()
-> ChromeBrowserInitializer.getInstance().isFullBrowserInitialized(), .hasNativeInitializationCompleted(),
"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