Don't enable strict mode in non-bugfood builds.

There were some calls to StrictMode#enableDefaults in our codebase that were not guarded by bugfood checks. In testing, #enableDefaults causes warnings to be printed that otherwise would not.

This CL makes it so that we never call enableDefaults on non-bugfood builds, and cleans up and consolidates our usage of strict mode logic by introducing a new class called DialerStrictMode.

Test: none
PiperOrigin-RevId: 163521622
Change-Id: I841b4198a5dd6084ee104dc6907165e3379d0451

5 files changed, 109 insertions, 38 deletions

diff --git a/java/com/android/dialer/binary/common/DialerApplication.java b/java/com/android/dialer/binary/common/DialerApplication.java
index 08666a21c..0d38541e5 100644
--- a/java/com/android/dialer/binary/common/DialerApplication.java
+++ b/java/com/android/dialer/binary/common/DialerApplication.java
@@ -17,18 +17,17 @@
 package com.android.dialer.binary.common;
 
 import android.app.Application;
-import android.os.StrictMode;
 import android.os.Trace;
 import android.support.annotation.NonNull;
 import android.support.v4.os.BuildCompat;
 import com.android.dialer.blocking.BlockedNumbersAutoMigrator;
 import com.android.dialer.blocking.FilteredNumberAsyncQueryHandler;
-import com.android.dialer.buildtype.BuildType;
 import com.android.dialer.calllog.CallLogComponent;
 import com.android.dialer.common.concurrent.DefaultDialerExecutorFactory;
 import com.android.dialer.inject.HasRootComponent;
 import com.android.dialer.notification.NotificationChannelManager;
 import com.android.dialer.persistentlog.PersistentLogger;
+import com.android.dialer.strictmode.DialerStrictMode;
 
 /** A common application subclass for all Dialer build variants. */
 public abstract class DialerApplication extends Application implements HasRootComponent {
@@ -38,9 +37,8 @@ public abstract class DialerApplication extends Application implements HasRootCo
   @Override
   public void onCreate() {
     Trace.beginSection("DialerApplication.onCreate");
-    if (BuildType.get() == BuildType.BUGFOOD) {
-      enableStrictMode();
-    }
+    DialerStrictMode.onApplicationCreate();
+
     super.onCreate();
     new BlockedNumbersAutoMigrator(
             this.getApplicationContext(),
@@ -56,13 +54,6 @@ public abstract class DialerApplication extends Application implements HasRootCo
     Trace.endSection();
   }
 
-  private void enableStrictMode() {
-    StrictMode.setThreadPolicy(
-        new StrictMode.ThreadPolicy.Builder().detectAll().penaltyLog().penaltyDeath().build());
-    StrictMode.setVmPolicy(
-        new StrictMode.VmPolicy.Builder().detectAll().penaltyLog().penaltyDeath().build());
-  }
-
   /**
    * Returns a new instance of the root component for the application. Sub classes should define a
    * root component that extends all the sub components "HasComponent" intefaces. The component
diff --git a/java/com/android/dialer/configprovider/SharedPrefConfigProvider.java b/java/com/android/dialer/configprovider/SharedPrefConfigProvider.java
index fad25a409..6ee469572 100644
--- a/java/com/android/dialer/configprovider/SharedPrefConfigProvider.java
+++ b/java/com/android/dialer/configprovider/SharedPrefConfigProvider.java
@@ -20,11 +20,11 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.content.SharedPreferences.Editor;
-import android.os.StrictMode;
 import android.support.annotation.Nullable;
 import com.android.dialer.common.Assert;
 import com.android.dialer.common.LogUtil;
 import com.android.dialer.inject.ApplicationContext;
+import com.android.dialer.strictmode.DialerStrictMode;
 import com.android.dialer.util.DialerUtils;
 import javax.inject.Inject;
 
@@ -95,37 +95,26 @@ class SharedPrefConfigProvider implements ConfigProvider {
 
   @Override
   public String getString(String key, String defaultValue) {
-    return bypassStrictMode(
+    // Reading shared prefs on the main thread is generally safe since a single instance is cached.
+    return DialerStrictMode.bypass(
         () -> getSharedPrefs(appContext).getString(PREF_PREFIX + key, defaultValue));
   }
 
   @Override
   public long getLong(String key, long defaultValue) {
-    return bypassStrictMode(
+    // Reading shared prefs on the main thread is generally safe since a single instance is cached.
+    return DialerStrictMode.bypass(
         () -> getSharedPrefs(appContext).getLong(PREF_PREFIX + key, defaultValue));
   }
 
   @Override
   public boolean getBoolean(String key, boolean defaultValue) {
-    return bypassStrictMode(
+    // Reading shared prefs on the main thread is generally safe since a single instance is cached.
+    return DialerStrictMode.bypass(
         () -> getSharedPrefs(appContext).getBoolean(PREF_PREFIX + key, defaultValue));
   }
 
   private static SharedPreferences getSharedPrefs(Context appContext) {
     return DialerUtils.getDefaultSharedPreferenceForDeviceProtectedStorageContext(appContext);
   }
-
-  private interface Provider<T> {
-    T get();
-  }
-
-  // Reading shared prefs on the main thread is generally safe since a single instance is cached.
-  private static <T> T bypassStrictMode(Provider<T> provider) {
-    StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
-    try {
-      return provider.get();
-    } finally {
-      StrictMode.setThreadPolicy(oldPolicy);
-    }
-  }
 }
diff --git a/java/com/android/dialer/strictmode/DialerStrictMode.java b/java/com/android/dialer/strictmode/DialerStrictMode.java
new file mode 100644
index 000000000..c9bbeafbf
--- /dev/null
+++ b/java/com/android/dialer/strictmode/DialerStrictMode.java
@@ -0,0 +1,95 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.dialer.strictmode;
+
+import android.os.StrictMode;
+import android.os.StrictMode.ThreadPolicy;
+import android.os.StrictMode.VmPolicy;
+import com.android.dialer.buildtype.BuildType;
+
+/**
+ * Enables strict mode for the application, and provides means of temporarily disabling it.
+ *
+ * <p>NOTE: All methods in this class are stripped by proguard in release builds.
+ */
+public final class DialerStrictMode {
+
+  /** Initializes strict mode on application start. */
+  public static void onApplicationCreate() {
+    enableDeathPenalty();
+  }
+
+  /**
+   * Disables the strict mode death penalty. If strict mode is enabled for the build, warnings are
+   * printed instead of the application crashing.
+   *
+   * <p>You should typically do this only temporarily and restore the death penalty in a finally
+   * block using {@link #enableDeathPenalty()}.
+   */
+  public static void disableDeathPenalty() {
+    if (isStrictModeAllowed()) {
+      StrictMode.setThreadPolicy(threadPolicyTemplate().build());
+      StrictMode.setVmPolicy(vmPolicyTemplate().build());
+    }
+  }
+
+  /**
+   * Restore the death penalty. This should typically be called in a finally block after calling
+   * {@link #disableDeathPenalty()}.
+   */
+  public static void enableDeathPenalty() {
+    if (isStrictModeAllowed()) {
+      StrictMode.setThreadPolicy(threadPolicyTemplate().penaltyDeath().build());
+      StrictMode.setVmPolicy(vmPolicyTemplate().penaltyDeath().build());
+    }
+  }
+
+  private static ThreadPolicy.Builder threadPolicyTemplate() {
+    return new StrictMode.ThreadPolicy.Builder().detectAll().penaltyLog();
+  }
+
+  private static VmPolicy.Builder vmPolicyTemplate() {
+    return new StrictMode.VmPolicy.Builder().detectAll().penaltyLog();
+  }
+
+  private static boolean isStrictModeAllowed() {
+    return BuildType.get() == BuildType.BUGFOOD;
+  }
+
+  /** Functional interface intended to be used with {@link #bypass(Provider)}. */
+  public interface Provider<T> {
+    T get();
+  }
+
+  /**
+   * Convenience method for disabling and enabling the death penalty using lambdas.
+   *
+   * <p>For example:
+   *
+   * <p><code>
+   *   DialerStrictMode.bypass(() -> doDiskAccessOnMainThread());
+   * </code>
+   */
+  public static <T> T bypass(Provider<T> provider) {
+    disableDeathPenalty();
+    try {
+      return provider.get();
+    } finally {
+      enableDeathPenalty();
+    }
+  }
+}
diff --git a/java/com/android/dialer/strictmode/proguard.flags b/java/com/android/dialer/strictmode/proguard.flags
new file mode 100644
index 000000000..785f9ae86
--- /dev/null
+++ b/java/com/android/dialer/strictmode/proguard.flags
@@ -0,0 +1 @@
+-assumenosideeffects class com.android.dialer.strictmode.DialerStrictMode { public * ; }
diff --git a/java/com/android/voicemail/impl/transcribe/TranscriptionService.java b/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
index 3e80a7f59..9a781e6c2 100644
--- a/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
+++ b/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
@@ -24,7 +24,6 @@ import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.net.Uri;
-import android.os.StrictMode;
 import android.support.annotation.MainThread;
 import android.support.annotation.VisibleForTesting;
 import android.support.v4.os.BuildCompat;
@@ -32,6 +31,7 @@ import android.text.TextUtils;
 import com.android.dialer.common.Assert;
 import com.android.dialer.common.LogUtil;
 import com.android.dialer.constants.ScheduledJobIds;
+import com.android.dialer.strictmode.DialerStrictMode;
 import com.android.voicemail.impl.transcribe.grpc.TranscriptionClientFactory;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -48,7 +48,6 @@ public class TranscriptionService extends JobService {
   private JobParameters jobParameters;
   private TranscriptionClientFactory clientFactory;
   private TranscriptionConfigProvider configProvider;
-  private StrictMode.VmPolicy originalPolicy;
 
   /** Callback used by a task to indicate it has finished processing its work item */
   interface JobCallback {
@@ -111,8 +110,7 @@ public class TranscriptionService extends JobService {
       LogUtil.i(
           "TranscriptionService.onStartJob",
           "transcription server address: " + configProvider.getServerAddress());
-      originalPolicy = StrictMode.getVmPolicy();
-      StrictMode.enableDefaults();
+      DialerStrictMode.disableDeathPenalty(); // Re-enabled in cleanup.
       jobParameters = params;
       return checkForWork();
     }
@@ -142,10 +140,7 @@ public class TranscriptionService extends JobService {
       executorService.shutdownNow();
       executorService = null;
     }
-    if (originalPolicy != null) {
-      StrictMode.setVmPolicy(originalPolicy);
-      originalPolicy = null;
-    }
+    DialerStrictMode.enableDeathPenalty();
   }
 
   @MainThread