Commit fd12ffbe authored by Peter Wen's avatar Peter Wen Committed by Commit Bot

Android: Ban all usages of System#exit

Apart from the build/tooling/test/third-party directories listed in
suppressions.xml, every usage of System#exit must be explicit and have
the @SuppressWarnings annotation, preferably with a short explanation of
why it is necessary.

Added @SuppressWarnings for all existing usages.

Removed the System#exit call in ChromeApplication since we have no
visibility into how often the application replacing bug is hit. Throwing
an exception with a custom message will help us see its current impact.

Bug: 1000651
Change-Id: Ia48598b32e339efccb0ca1b55ef03068da505410
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1851770
Auto-Submit: Peter Wen <wnwen@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706426}
parent 41b59fc7
...@@ -403,9 +403,6 @@ public class LibraryLoader { ...@@ -403,9 +403,6 @@ public class LibraryLoader {
mLoaded = true; mLoaded = true;
} catch (UnsatisfiedLinkError e) { } catch (UnsatisfiedLinkError e) {
// Callers typically call System.exit() when catching this exception, make sure that it
// doesn't get lost.
Log.e(TAG, "Unable to load library.", e);
throw new ProcessInitException(LoaderErrors.LOADER_ERROR_NATIVE_LIBRARY_LOAD_FAILED, e); throw new ProcessInitException(LoaderErrors.LOADER_ERROR_NATIVE_LIBRARY_LOAD_FAILED, e);
} }
} }
......
...@@ -272,6 +272,7 @@ public class ChildProcessService { ...@@ -272,6 +272,7 @@ public class ChildProcessService {
mMainThread.start(); mMainThread.start();
} }
@SuppressWarnings("checkstyle:SystemExitCheck") // Allowed due to http://crbug.com/928521#c16.
public void onDestroy() { public void onDestroy() {
Log.i(TAG, "Destroying ChildProcessService pid=%d", Process.myPid()); Log.i(TAG, "Destroying ChildProcessService pid=%d", Process.myPid());
System.exit(0); System.exit(0);
......
...@@ -19,7 +19,6 @@ import org.chromium.base.BuildInfo; ...@@ -19,7 +19,6 @@ import org.chromium.base.BuildInfo;
import org.chromium.base.CommandLineInitUtil; import org.chromium.base.CommandLineInitUtil;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.JNIUtils; import org.chromium.base.JNIUtils;
import org.chromium.base.Log;
import org.chromium.base.PathUtils; import org.chromium.base.PathUtils;
import org.chromium.base.TraceEvent; import org.chromium.base.TraceEvent;
import org.chromium.base.annotations.MainDex; import org.chromium.base.annotations.MainDex;
...@@ -54,7 +53,6 @@ import org.chromium.ui.base.ResourceBundle; ...@@ -54,7 +53,6 @@ import org.chromium.ui.base.ResourceBundle;
*/ */
public class ChromeApplication extends Application { public class ChromeApplication extends Application {
private static final String COMMAND_LINE_FILE = "chrome-command-line"; private static final String COMMAND_LINE_FILE = "chrome-command-line";
private static final String TAG = "ChromiumApplication";
// Public to allow use in ChromeBackupAgent // Public to allow use in ChromeBackupAgent
public static final String PRIVATE_DATA_DIRECTORY_SUFFIX = "chrome"; public static final String PRIVATE_DATA_DIRECTORY_SUFFIX = "chrome";
...@@ -170,8 +168,7 @@ public class ChromeApplication extends Application { ...@@ -170,8 +168,7 @@ public class ChromeApplication extends Application {
// out-of-date application. Kill old applications in this bad state. See // out-of-date application. Kill old applications in this bad state. See
// http://crbug.com/658130 for more context and http://b.android.com/56296 for the bug. // http://crbug.com/658130 for more context and http://b.android.com/56296 for the bug.
if (ContextUtils.getApplicationAssets() == null) { if (ContextUtils.getApplicationAssets() == null) {
Log.e(TAG, "getResources() null, closing app."); throw new RuntimeException("App out of date, getResources() null, closing app.");
System.exit(0);
} }
} }
......
...@@ -395,6 +395,7 @@ public class LaunchIntentDispatcher implements IntentHandler.IntentHandlerDelega ...@@ -395,6 +395,7 @@ public class LaunchIntentDispatcher implements IntentHandler.IntentHandlerDelega
* Handles launching a {@link ChromeTabbedActivity}. * Handles launching a {@link ChromeTabbedActivity}.
*/ */
@SuppressLint("InlinedApi") @SuppressLint("InlinedApi")
@SuppressWarnings("checkstyle:SystemExitCheck") // Allowed due to https://crbug.com/847921#c17.
private @Action int dispatchToTabbedActivity() { private @Action int dispatchToTabbedActivity() {
if (mIsVrIntent) { if (mIsVrIntent) {
for (Activity activity : ApplicationStatus.getRunningActivities()) { for (Activity activity : ApplicationStatus.getRunningActivities()) {
......
...@@ -36,6 +36,7 @@ public class ChildProcessLauncherTestHelperService extends Service { ...@@ -36,6 +36,7 @@ public class ChildProcessLauncherTestHelperService extends Service {
private final Handler.Callback mHandlerCallback = new Handler.Callback() { private final Handler.Callback mHandlerCallback = new Handler.Callback() {
@Override @Override
@SuppressWarnings("checkstyle:SystemExitCheck") // Allowed due to ChildProcessService.
public boolean handleMessage(Message msg) { public boolean handleMessage(Message msg) {
switch (msg.what) { switch (msg.what) {
case MSG_BIND_SERVICE: case MSG_BIND_SERVICE:
......
...@@ -145,11 +145,12 @@ ...@@ -145,11 +145,12 @@
<property name="message" value="Please name the StrictModeContext variable &quot;ignored&quot; (crbug.com/767058#c4)."/> <property name="message" value="Please name the StrictModeContext variable &quot;ignored&quot; (crbug.com/767058#c4)."/>
</module> </module>
<module name="RegexpSinglelineJava"> <module name="RegexpSinglelineJava">
<!-- Tests are also allowed to use System.exit if need be, please add dirs to tools/android/checkstyle/suppressions.xml -->
<property name="id" value="SystemExitCheck"/> <property name="id" value="SystemExitCheck"/>
<property name="severity" value="error"/> <property name="severity" value="error"/>
<property name="format" value="System\.exit\([^0]"/> <property name="format" value="System\.exit\("/>
<property name="ignoreComments" value="true"/> <property name="ignoreComments" value="true"/>
<property name="message" value="Throw an exception instead of calling System#exit with a non-zero exit status (crbug.com/1000651)."/> <property name="message" value="Throw an exception instead of calling System#exit (crbug.com/1000651)."/>
</module> </module>
</module> </module>
</module> </module>
...@@ -15,6 +15,6 @@ ...@@ -15,6 +15,6 @@
<suppress id="MethodNameCheck" files="FirstRunIntegrationUnitTest.java"/> <suppress id="MethodNameCheck" files="FirstRunIntegrationUnitTest.java"/>
<suppress id="MethodNameCheck" files="SplashActivityTest.java"/> <suppress id="MethodNameCheck" files="SplashActivityTest.java"/>
<!-- Third-party libraries, test infrastructure, build, and tooling can use <!-- Third-party libraries, test infrastructure, build, and tooling can use
System#exit freely. --> System#exit freely. Feel free to add other test directories here. -->
<suppress id="SystemExitCheck" files="src/(build|testing|third_party|tools)/"/> <suppress id="SystemExitCheck" files="src/(build|testing|third_party|tools|base/test)/"/>
</suppressions> </suppressions>
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