diff options
author | dfalcantara <dfalcantara@chromium.org> | 2015-10-28 17:56:37 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-10-29 00:57:46 +0000 |
commit | d95effdf36025ca669aca06823d6e61433da8c73 (patch) | |
tree | 28f443141e6d5be1974222319acdda8e874aeb95 | |
parent | 8379ac5916cb0ef3832c1369fca6d547ed07673b (diff) | |
download | chromium_src-d95effdf36025ca669aca06823d6e61433da8c73.zip chromium_src-d95effdf36025ca669aca06823d6e61433da8c73.tar.gz chromium_src-d95effdf36025ca669aca06823d6e61433da8c73.tar.bz2 |
Move process killing from ChromeLifetimeController to BrowserRestartActivity
The ActivityStateListener implemented by ChromeLifetimeController
is unreliable for restarting the process because onDestroy() events
aren't being propagated to the listeners and the watchdog seems to
die along with the Activities. Instead, always fire an Intent to
the BrowserRestartActivity, which is now always used used to kill
and (optionally) restart the main Chrome process.
* Moves the watchdog to the BrowserRestartActivity.
* BrowserRestartActivity takes over all process killing responsibilities
from the ChromeLifetimeController.
* Clarifies a few things in the comments.
BUG=545453
Review URL: https://codereview.chromium.org/1428773002
Cr-Commit-Position: refs/heads/master@{#356713}
4 files changed, 87 insertions, 71 deletions
diff --git a/chrome/android/java/AndroidManifest.xml b/chrome/android/java/AndroidManifest.xml index 4e72a16..0e29e6a 100644 --- a/chrome/android/java/AndroidManifest.xml +++ b/chrome/android/java/AndroidManifest.xml @@ -399,7 +399,7 @@ by a child template that "extends" this file. <activity android:name="org.chromium.chrome.browser.BrowserRestartActivity" android:launchMode="singleInstance" android:exported="false" - android:theme="@android:style/Theme.NoDisplay" + android:theme="@android:style/Theme.Translucent.NoTitleBar" android:excludeFromRecents="true" android:process=":browser_restart_process"> </activity> diff --git a/chrome/android/java/src/org/chromium/chrome/browser/BrowserRestartActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/BrowserRestartActivity.java index 17cb540..90825b0c 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/BrowserRestartActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/BrowserRestartActivity.java @@ -8,34 +8,87 @@ import android.app.Activity; import android.content.Context; import android.content.Intent; import android.os.Bundle; +import android.os.Handler; +import android.os.Looper; import android.os.Process; +import android.text.TextUtils; import org.chromium.base.ApplicationStatus; import org.chromium.chrome.browser.util.IntentUtils; /** - * Activity that kills and restarts Chrome, then immediately kills itself. - * Works around an Android framework issue for alarms set via the AlarmManager: crbug.com/515919. + * Kills and (optionally) restarts the main Chrome process, then immediately kills itself. + * + * Starting this Activity should only be done by the {@link ChromeLifetimeController}. + * + * This Activity runs on a separate process from the main Chrome browser and cannot see the main + * process' Activities. It works around an Android framework issue for alarms set via the + * AlarmManager, which requires a minimum alarm duration of 5 seconds: https://crbug.com/515919. */ public class BrowserRestartActivity extends Activity { - static final String EXTRA_PID = "org.chromium.chrome.browser.BrowserRestartActivity.pid"; + static final String ACTION_START_WATCHDOG = + "org.chromium.chrome.browser.BrowserRestartActivity.start_watchdog"; + static final String ACTION_KILL_PROCESS = + "org.chromium.chrome.browser.BrowserRestartActivity.kill_process"; + + static final String EXTRA_MAIN_PID = + "org.chromium.chrome.browser.BrowserRestartActivity.main_pid"; + static final String EXTRA_RESTART = + "org.chromium.chrome.browser.BrowserRestartActivity.restart"; + + private static final String TAG = "BrowserRestartActivity"; + + // The amount of time to wait for Chrome to destroy all the activities of the main process + // before this Activity forcefully kills it. + private static final long WATCHDOG_DELAY_MS = 1000; @Override public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); + handleIntent(getIntent()); + } + + @Override + public void onNewIntent(Intent intent) { + handleIntent(intent); + } + + private void handleIntent(final Intent intent) { + if (TextUtils.equals(ACTION_START_WATCHDOG, intent.getAction())) { + // Kill the main process if Android fails to finish our Activities in a timely manner. + Handler handler = new Handler(Looper.getMainLooper()); + handler.postDelayed(new Runnable() { + @Override + public void run() { + destroyProcess(intent); + } + }, WATCHDOG_DELAY_MS); + } else if (TextUtils.equals(ACTION_KILL_PROCESS, intent.getAction())) { + destroyProcess(intent); + } else { + assert false; + } + } - int pid = IntentUtils.safeGetIntExtra(getIntent(), BrowserRestartActivity.EXTRA_PID, -1); - if (pid != -1) { - Process.killProcess(pid); + private void destroyProcess(Intent intent) { + // Kill the main Chrome process. + int mainBrowserPid = IntentUtils.safeGetIntExtra( + intent, BrowserRestartActivity.EXTRA_MAIN_PID, -1); + assert mainBrowserPid != -1; + Process.killProcess(mainBrowserPid); - // Restart Chrome. + // Fire an Intent to restart Chrome. + boolean restart = IntentUtils.safeGetBooleanExtra( + intent, BrowserRestartActivity.EXTRA_RESTART, false); + if (restart) { Context context = ApplicationStatus.getApplicationContext(); - Intent intent = new Intent(Intent.ACTION_MAIN); - intent.setPackage(context.getPackageName()); - intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); - context.startActivity(intent); + Intent restartIntent = new Intent(Intent.ACTION_MAIN); + restartIntent.setPackage(context.getPackageName()); + restartIntent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); + context.startActivity(restartIntent); } + // Kill this process. finish(); Process.killProcess(Process.myPid()); } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java index 7493b68..4f7d4c5 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java @@ -482,7 +482,7 @@ public class ChromeApplication extends ContentApplication { removeSessionCookies(); ApplicationStatus.registerApplicationStateListener(createApplicationStateListener()); AppBannerManager.setAppDetailsDelegate(createAppDetailsDelegate()); - mChromeLifetimeController = new ChromeLifetimeController(this); + mChromeLifetimeController = new ChromeLifetimeController(); PrefServiceBridge.getInstance().migratePreferences(this); } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeLifetimeController.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeLifetimeController.java index b433b50..45bdeec 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeLifetimeController.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeLifetimeController.java @@ -7,55 +7,37 @@ package org.chromium.chrome.browser; import android.app.Activity; import android.content.Context; import android.content.Intent; -import android.os.Handler; -import android.os.Looper; import android.os.Process; import org.chromium.base.ActivityState; import org.chromium.base.ApplicationStatus; -import org.chromium.base.Log; import java.lang.ref.WeakReference; /** - * Handles killing and potentially restarting Chrome's main Browser process. Note that this class - * relies on main Chrome activities to properly call {@link Activity#finish()} on themselves so that - * it will be notified that all {@link Activity}s under this {@link Application} have been - * destroyed. + * Answers requests to kill and (potentially) restart Chrome's main browser process. + * + * This class fires an Intent to start the {@link BrowserRestartActivity}, which will utltimately + * kill the main browser process from its own process. + * + * https://crbug.com/515919 details why another Activity is used instead of using the AlarmManager. + * https://crbug.com/545453 details why the BrowserRestartActivity handles the process killing. */ class ChromeLifetimeController implements ApplicationLifetime.Observer, ApplicationStatus.ActivityStateListener { private static final String TAG = "cr.LifetimeController"; - // The amount of time to wait for Chrome to destroy all the activities. - private static final long WATCHDOG_DELAY = 1000; - - private final Context mContext; private boolean mRestartChromeOnDestroy; private int mRemainingActivitiesCount = 0; - private final Handler mHandler; - /** - * Creates a {@link ChromeLifetimeController} instance. - * @param context A {@link Context} instance. The application context will be saved from this - * one. - */ - public ChromeLifetimeController(Context context) { - mContext = context.getApplicationContext(); + public ChromeLifetimeController() { ApplicationLifetime.addObserver(this); - mHandler = new Handler(Looper.getMainLooper()); } @Override public void onTerminate(boolean restart) { mRestartChromeOnDestroy = restart; - // We've called terminate twice, just wait for the first call to take effect. - if (mRemainingActivitiesCount > 0) { - Log.w(TAG, "onTerminate called twice"); - return; - } - for (WeakReference<Activity> weakActivity : ApplicationStatus.getRunningActivities()) { Activity activity = weakActivity.get(); if (activity != null) { @@ -65,49 +47,30 @@ class ChromeLifetimeController implements ApplicationLifetime.Observer, } } - // Post a watchdog -- if Android is taking a long time to call onDestroy, kill the process. - mHandler.postDelayed(new Runnable() { - @Override - public void run() { - if (mRemainingActivitiesCount > 0) { - destroyProcess(); - } - } - }, WATCHDOG_DELAY); + // Start the Activity that will ultimately kill this process. + fireBrowserRestartActivityIntent(BrowserRestartActivity.ACTION_START_WATCHDOG); } - @Override public void onActivityStateChange(Activity activity, int newState) { assert mRemainingActivitiesCount > 0; if (newState == ActivityState.DESTROYED) { mRemainingActivitiesCount--; if (mRemainingActivitiesCount == 0) { - destroyProcess(); + fireBrowserRestartActivityIntent(BrowserRestartActivity.ACTION_KILL_PROCESS); } } - } - private void destroyProcess() { - mHandler.post(new Runnable() { - @Override - public void run() { - if (mRestartChromeOnDestroy) { - Log.w(TAG, "Launching BrowserRestartActivity to restart Chrome."); - Context context = ApplicationStatus.getApplicationContext(); - Intent intent = new Intent(); - intent.setClassName( - context.getPackageName(), BrowserRestartActivity.class.getName()); - intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); - intent.putExtra(BrowserRestartActivity.EXTRA_PID, Process.myPid()); - context.startActivity(intent); - } else { - Log.w(TAG, "Killing process directly."); - Process.killProcess(Process.myPid()); - } - mRestartChromeOnDestroy = false; - } - }); + private void fireBrowserRestartActivityIntent(String action) { + Context context = ApplicationStatus.getApplicationContext(); + Intent intent = new Intent(); + intent.setAction(action); + intent.setClassName( + context.getPackageName(), BrowserRestartActivity.class.getName()); + intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); + intent.putExtra(BrowserRestartActivity.EXTRA_MAIN_PID, Process.myPid()); + intent.putExtra(BrowserRestartActivity.EXTRA_RESTART, mRestartChromeOnDestroy); + context.startActivity(intent); } } |