Workaround bug where system clobbered StrictMode thread policy.

There is a bug (b/36951662) in many versions of the system where if you set the thread policy in Application.onCreate it could get clobbered.

When this happened, strict mode violations could sometimes go unreported.

This CL works around the problem by re-enabling our custom policy after onCreate by posting to the main thread.

Additionally, it copies most of java/com/google/android/libraries/strictmode/StrictModeUtils.java into our codebase and uses it to better handle known violations.

Finally, fixed or bypassed a few violations which were exposed by this change.

Bug: 64690143,64772057
Test: observed that certain strict mode violations which were flaky before now consistently reproduce
PiperOrigin-RevId: 165653027
Change-Id: If58966cfce251a22945f3aada6570b303c6af0ee

7 files changed, 233 insertions, 76 deletions

diff --git a/java/com/android/contacts/common/preference/ContactsPreferences.java b/java/com/android/contacts/common/preference/ContactsPreferences.java
index 2bdf5c82a..399700e28 100644
--- a/java/com/android/contacts/common/preference/ContactsPreferences.java
+++ b/java/com/android/contacts/common/preference/ContactsPreferences.java
@@ -70,7 +70,10 @@ public class ContactsPreferences implements OnSharedPreferenceChangeListener {
   public ContactsPreferences(Context context) {
     mContext = context;
     mHandler = new Handler();
-    mPreferences = mContext.getSharedPreferences(context.getPackageName(), Context.MODE_PRIVATE);
+    mPreferences =
+        mContext
+            .getApplicationContext()
+            .getSharedPreferences(context.getPackageName(), Context.MODE_PRIVATE);
     mDefaultAccountKey =
         mContext.getResources().getString(R.string.contact_editor_default_account_key);
     mDefaultAccountSavedKey =
@@ -253,7 +256,7 @@ public class ContactsPreferences implements OnSharedPreferenceChangeListener {
 
     if (!mPreferences.contains(mDefaultAccountKey)) {
       final SharedPreferences previousPrefs =
-          PreferenceManager.getDefaultSharedPreferences(mContext);
+          PreferenceManager.getDefaultSharedPreferences(mContext.getApplicationContext());
       final String defaultAccount = previousPrefs.getString(mDefaultAccountKey, null);
       if (!TextUtils.isEmpty(defaultAccount)) {
         final AccountWithDataSet accountWithDataSet =
diff --git a/java/com/android/dialer/app/list/ListsFragment.java b/java/com/android/dialer/app/list/ListsFragment.java
index dbb6c8b5c..86a3d2fbb 100644
--- a/java/com/android/dialer/app/list/ListsFragment.java
+++ b/java/com/android/dialer/app/list/ListsFragment.java
@@ -98,7 +98,7 @@ public class ListsFragment extends Fragment implements OnPageChangeListener, Lis
     LogUtil.d("ListsFragment.onCreate", null);
     Trace.beginSection(TAG + " onCreate");
     super.onCreate(savedInstanceState);
-    mPrefs = PreferenceManager.getDefaultSharedPreferences(getActivity());
+    mPrefs = PreferenceManager.getDefaultSharedPreferences(getActivity().getApplicationContext());
     Trace.endSection();
   }
 
diff --git a/java/com/android/dialer/app/list/RegularSearchFragment.java b/java/com/android/dialer/app/list/RegularSearchFragment.java
index 728948bfc..73120c547 100644
--- a/java/com/android/dialer/app/list/RegularSearchFragment.java
+++ b/java/com/android/dialer/app/list/RegularSearchFragment.java
@@ -18,7 +18,10 @@ package com.android.dialer.app.list;
 import static android.Manifest.permission.READ_CONTACTS;
 
 import android.app.Activity;
+import android.content.Context;
 import android.content.pm.PackageManager;
+import android.os.Bundle;
+import android.support.annotation.Nullable;
 import android.support.v13.app.FragmentCompat;
 import android.view.LayoutInflater;
 import android.view.ViewGroup;
@@ -27,7 +30,11 @@ import com.android.contacts.common.list.PinnedHeaderListView;
 import com.android.dialer.app.R;
 import com.android.dialer.callintent.CallInitiationType;
 import com.android.dialer.common.LogUtil;
+import com.android.dialer.common.concurrent.DialerExecutor;
+import com.android.dialer.common.concurrent.DialerExecutor.Worker;
+import com.android.dialer.common.concurrent.DialerExecutors;
 import com.android.dialer.phonenumbercache.CachedNumberLookupService;
+import com.android.dialer.phonenumbercache.CachedNumberLookupService.CachedContactInfo;
 import com.android.dialer.phonenumbercache.PhoneNumberCache;
 import com.android.dialer.util.PermissionsUtil;
 import com.android.dialer.widget.EmptyContentView;
@@ -43,6 +50,8 @@ public class RegularSearchFragment extends SearchFragment
   private static final int SEARCH_DIRECTORY_RESULT_LIMIT = 5;
   protected String mPermissionToRequest;
 
+  private DialerExecutor<CachedContactInfo> addContactTask;
+
   public RegularSearchFragment() {
     configureDirectorySearch();
   }
@@ -53,6 +62,18 @@ public class RegularSearchFragment extends SearchFragment
   }
 
   @Override
+  public void onCreate(Bundle savedState) {
+    super.onCreate(savedState);
+
+    addContactTask =
+        DialerExecutors.createUiTaskBuilder(
+                getFragmentManager(),
+                "RegularSearchFragment.addContact",
+                new AddContactWorker(getContext().getApplicationContext()))
+            .build();
+  }
+
+  @Override
   protected void onCreateView(LayoutInflater inflater, ViewGroup container) {
     super.onCreateView(inflater, container);
     ((PinnedHeaderListView) getListView()).setScrollToSectionOnHeaderTouch(true);
@@ -73,8 +94,9 @@ public class RegularSearchFragment extends SearchFragment
         PhoneNumberCache.get(getContext()).getCachedNumberLookupService();
     if (cachedNumberLookupService != null) {
       final RegularSearchListAdapter adapter = (RegularSearchListAdapter) getAdapter();
-      cachedNumberLookupService.addContact(
-          getContext(), adapter.getContactInfo(cachedNumberLookupService, position));
+      CachedContactInfo cachedContactInfo =
+          adapter.getContactInfo(cachedNumberLookupService, position);
+      addContactTask.executeSerial(cachedContactInfo);
     }
   }
 
@@ -152,4 +174,24 @@ public class RegularSearchFragment extends SearchFragment
 
     boolean isNearbyPlacesSearchEnabled();
   }
+
+  private static class AddContactWorker implements Worker<CachedContactInfo, Void> {
+
+    private final Context appContext;
+
+    private AddContactWorker(Context appContext) {
+      this.appContext = appContext;
+    }
+
+    @Nullable
+    @Override
+    public Void doInBackground(@Nullable CachedContactInfo contactInfo) throws Throwable {
+      CachedNumberLookupService cachedNumberLookupService =
+          PhoneNumberCache.get(appContext).getCachedNumberLookupService();
+      if (cachedNumberLookupService != null) {
+        cachedNumberLookupService.addContact(appContext, contactInfo);
+      }
+      return null;
+    }
+  }
 }
diff --git a/java/com/android/dialer/smartdial/SmartDialPrefix.java b/java/com/android/dialer/smartdial/SmartDialPrefix.java
index a000e21c5..36f174b33 100644
--- a/java/com/android/dialer/smartdial/SmartDialPrefix.java
+++ b/java/com/android/dialer/smartdial/SmartDialPrefix.java
@@ -73,7 +73,8 @@ public class SmartDialPrefix {
       sUserSimCountryCode = manager.getSimCountryIso();
     }
 
-    final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context);
+    final SharedPreferences prefs =
+        PreferenceManager.getDefaultSharedPreferences(context.getApplicationContext());
 
     if (sUserSimCountryCode != null) {
       /** Updates shared preferences with the latest country obtained from getSimCountryIso. */
diff --git a/java/com/android/dialer/strictmode/DialerStrictMode.java b/java/com/android/dialer/strictmode/DialerStrictMode.java
index ae72c3c0d..4a0336e53 100644
--- a/java/com/android/dialer/strictmode/DialerStrictMode.java
+++ b/java/com/android/dialer/strictmode/DialerStrictMode.java
@@ -18,11 +18,14 @@ package com.android.dialer.strictmode;
 
 import android.app.Application;
 import android.content.Context;
+import android.os.Handler;
 import android.os.Looper;
 import android.os.StrictMode;
 import android.os.StrictMode.ThreadPolicy;
 import android.os.StrictMode.VmPolicy;
 import android.preference.PreferenceManager;
+import android.support.annotation.AnyThread;
+import android.support.annotation.MainThread;
 import android.support.v4.os.UserManagerCompat;
 import com.android.dialer.buildtype.BuildType;
 import com.android.dialer.util.DialerUtils;
@@ -30,10 +33,32 @@ import com.android.dialer.util.DialerUtils;
 /** Enables strict mode for the application, and provides means of temporarily disabling it. */
 public final class DialerStrictMode {
 
+  private static final VmPolicy VM_DEATH_PENALTY =
+      new StrictMode.VmPolicy.Builder().penaltyLog().penaltyDeath().build();
+
+  private static final ThreadPolicy THREAD_LOG_PENALTY =
+      new StrictMode.ThreadPolicy.Builder().penaltyLog().build();
+  private static final ThreadPolicy THREAD_DEATH_PENALTY =
+      new StrictMode.ThreadPolicy.Builder().penaltyLog().penaltyDeath().build();
+
+  private DialerStrictMode() {}
+
   /** Initializes strict mode on application start. */
+  @MainThread
   public static void onApplicationCreate(Application application) {
-    warmupSharedPrefs(application);
-    enableDeathPenalty();
+    if (isStrictModeAllowed()) {
+      warmupSharedPrefs(application);
+      StrictModeUtils.setRecommendedMainThreadPolicy(THREAD_DEATH_PENALTY);
+      StrictModeUtils.setRecommendedVMPolicy(VM_DEATH_PENALTY);
+
+      // Because Android resets StrictMode policies after Application.onCreate is done, we set it
+      // again right after.
+      // See cl/105932355 for the discussion.
+      // See b/36951662 for the public bug.
+      Handler handler = new Handler(Looper.myLooper());
+      handler.postAtFrontOfQueue(
+          () -> StrictModeUtils.setRecommendedMainThreadPolicy(THREAD_DEATH_PENALTY));
+    }
   }
 
   /**
@@ -43,65 +68,22 @@ public final class DialerStrictMode {
    * every point in the application where shared preferences are accessed.
    */
   private static void warmupSharedPrefs(Application application) {
-    if (isStrictModeAllowed()) {
-      // From credential-encrypted (CE) storage, i.e.:
-      //    /data/data/com.android.dialer/shared_prefs
-
-      if (UserManagerCompat.isUserUnlocked(application)) {
-        // <package_name>_preferences.xml
-        PreferenceManager.getDefaultSharedPreferences(application);
-
-        // <package_name>.xml
-        application.getSharedPreferences(application.getPackageName(), Context.MODE_PRIVATE);
-      }
-
-      // From device-encrypted (DE) storage, i.e.:
-      //   /data/user_de/0/com.android.dialer/shared_prefs/
+    // From credential-encrypted (CE) storage, i.e.:
+    //    /data/data/com.android.dialer/shared_prefs
 
+    if (UserManagerCompat.isUserUnlocked(application)) {
       // <package_name>_preferences.xml
-      DialerUtils.getDefaultSharedPreferenceForDeviceProtectedStorageContext(application);
-    }
-  }
-
-  /**
-   * 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()}.
-   *
-   * <p>The thread policy is only mutated if this is called from the main thread.
-   */
-  public static void disableDeathPenalty() {
-    if (isStrictModeAllowed()) {
-      if (onMainThread()) {
-        StrictMode.setThreadPolicy(threadPolicyTemplate().build());
-      }
-      StrictMode.setVmPolicy(vmPolicyTemplate().build());
-    }
-  }
+      PreferenceManager.getDefaultSharedPreferences(application);
 
-  /**
-   * Restore the death penalty. This should typically be called in a finally block after calling
-   * {@link #disableDeathPenalty()}.
-   *
-   * <p>The thread policy is only mutated if this is called from the main thread.
-   */
-  public static void enableDeathPenalty() {
-    if (isStrictModeAllowed()) {
-      if (onMainThread()) {
-        StrictMode.setThreadPolicy(threadPolicyTemplate().penaltyDeath().build());
-      }
-      StrictMode.setVmPolicy(vmPolicyTemplate().penaltyDeath().build());
+      // <package_name>.xml
+      application.getSharedPreferences(application.getPackageName(), Context.MODE_PRIVATE);
     }
-  }
 
-  private static ThreadPolicy.Builder threadPolicyTemplate() {
-    return new StrictMode.ThreadPolicy.Builder().detectAll().penaltyLog();
-  }
+    // From device-encrypted (DE) storage, i.e.:
+    //   /data/user_de/0/com.android.dialer/shared_prefs/
 
-  private static VmPolicy.Builder vmPolicyTemplate() {
-    return new StrictMode.VmPolicy.Builder().detectAll().penaltyLog();
+    // <package_name>_preferences.xml
+    DialerUtils.getDefaultSharedPreferenceForDeviceProtectedStorageContext(application);
   }
 
   private static boolean isStrictModeAllowed() {
@@ -118,7 +100,7 @@ public final class DialerStrictMode {
   }
 
   /**
-   * Convenience method for disabling and enabling the death penalty using lambdas.
+   * Convenience method for disabling and enabling the thread policy death penalty using lambdas.
    *
    * <p>For example:
    *
@@ -128,30 +110,42 @@ public final class DialerStrictMode {
    *
    * <p>The thread policy is only mutated if this is called from the main thread.
    */
+  @AnyThread
   public static <T> T bypass(Provider<T> provider) {
-    disableDeathPenalty();
-    try {
-      return provider.get();
-    } finally {
-      enableDeathPenalty();
+    if (isStrictModeAllowed() && onMainThread()) {
+      ThreadPolicy originalPolicy = StrictMode.getThreadPolicy();
+      StrictModeUtils.setRecommendedMainThreadPolicy(THREAD_LOG_PENALTY);
+      try {
+        return provider.get();
+      } finally {
+        StrictMode.setThreadPolicy(originalPolicy);
+      }
     }
+    return provider.get();
   }
 
   /**
-   * Convenience method for disabling and enabling the death penalty using lambdas.
+   * Convenience method for disabling and enabling the thread policy death penalty using lambdas.
    *
    * <p>For example:
    *
    * <p><code>
    *   DialerStrictMode.bypass(() -> doDiskAccessOnMainThread());
    * </code>
+   *
+   * <p>The thread policy is only mutated if this is called from the main thread.
    */
+  @AnyThread
   public static void bypass(Runnable runnable) {
-    disableDeathPenalty();
-    try {
-      runnable.run();
-    } finally {
-      enableDeathPenalty();
+    if (isStrictModeAllowed() && onMainThread()) {
+      ThreadPolicy originalPolicy = StrictMode.getThreadPolicy();
+      StrictModeUtils.setRecommendedMainThreadPolicy(THREAD_LOG_PENALTY);
+      try {
+        runnable.run();
+      } finally {
+        StrictMode.setThreadPolicy(originalPolicy);
+      }
     }
+    runnable.run();
   }
 }
diff --git a/java/com/android/dialer/strictmode/StrictModeUtils.java b/java/com/android/dialer/strictmode/StrictModeUtils.java
new file mode 100644
index 000000000..6944fd487
--- /dev/null
+++ b/java/com/android/dialer/strictmode/StrictModeUtils.java
@@ -0,0 +1,120 @@
+/*
+ * 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.Build;
+import android.os.StrictMode;
+import android.support.annotation.Nullable;
+import com.android.dialer.common.Assert;
+import com.google.auto.value.AutoValue;
+import java.util.Map;
+import java.util.Map.Entry;
+
+/** Utilities for enforcing strict-mode in an app. */
+final class StrictModeUtils {
+
+  /**
+   * Set the recommended policy for the app.
+   *
+   * @param threadPenalties policy with preferred penalties. Detection bits will be ignored.
+   */
+  static void setRecommendedMainThreadPolicy(StrictMode.ThreadPolicy threadPenalties) {
+    StrictMode.ThreadPolicy threadPolicy =
+        new StrictMode.ThreadPolicy.Builder(threadPenalties).detectAll().build();
+    StrictMode.setThreadPolicy(threadPolicy);
+  }
+
+  /**
+   * Set the recommended policy for the app.
+   *
+   * @param vmPenalties policy with preferred penalties. Detection bits should be unset.
+   */
+  static void setRecommendedVMPolicy(StrictMode.VmPolicy vmPenalties) {
+    setRecommendedVMPolicy(vmPenalties, StrictModeVmConfig.empty());
+  }
+
+  /**
+   * Set the recommended policy for the app.
+   *
+   * @param vmPenalties policy with preferred penalties. Detection bits should be unset.
+   */
+  private static void setRecommendedVMPolicy(
+      StrictMode.VmPolicy vmPenalties, StrictModeVmConfig config) {
+    Assert.isNotNull(config);
+    StrictMode.VmPolicy.Builder vmPolicyBuilder =
+        new StrictMode.VmPolicy.Builder(vmPenalties)
+            .detectLeakedClosableObjects()
+            .detectLeakedSqlLiteObjects();
+    if (Build.VERSION.SDK_INT >= 16) {
+      vmPolicyBuilder.detectLeakedRegistrationObjects();
+    }
+    if (Build.VERSION.SDK_INT >= 18) {
+      vmPolicyBuilder.detectFileUriExposure();
+    }
+    if (Build.VERSION.SDK_INT >= 21) {
+      // Even though this API is available earlier, it did not properly run finalizers.
+      // This avoids lots of false positives.
+
+      // TODO(zachh): Use LeakCanary and remove this line.
+      vmPolicyBuilder.detectActivityLeaks();
+
+      if (config.maxInstanceLimits() != null) {
+        for (Entry<Class<?>, Integer> entry : config.maxInstanceLimits().entrySet()) {
+          vmPolicyBuilder.setClassInstanceLimit(entry.getKey(), entry.getValue());
+        }
+      }
+    }
+    if (Build.VERSION.SDK_INT >= 23) {
+      // TODO(azlatin): Enable clear-text check once b/36730713 is fixed.
+      // vmPolicyBuilder.detectCleartextNetwork();
+    }
+    // Once OC Lands:
+    // .detectContentUriWithoutPermission()
+    // .detectUntaggedSockets()
+    StrictMode.setVmPolicy(vmPolicyBuilder.build());
+  }
+
+  /** VmPolicy configuration. */
+  @AutoValue
+  abstract static class StrictModeVmConfig {
+    /** A map of a class to the maximum number of allowed instances of that class. */
+    @Nullable
+    abstract Map<Class<?>, Integer> maxInstanceLimits();
+
+    public static StrictModeVmConfig empty() {
+      return builder().build();
+    }
+
+    public static Builder builder() {
+      return new AutoValue_StrictModeUtils_StrictModeVmConfig.Builder();
+    }
+
+    /** VmPolicy configuration builder. */
+    @AutoValue.Builder
+    public abstract static class Builder {
+      public abstract Builder setMaxInstanceLimits(Map<Class<?>, Integer> limits);
+
+      public abstract StrictModeVmConfig build();
+
+      Builder() {}
+    }
+
+    StrictModeVmConfig() {}
+  }
+
+  private StrictModeUtils() {}
+}
diff --git a/java/com/android/voicemail/impl/transcribe/TranscriptionService.java b/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
index 85e524302..509f25bed 100644
--- a/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
+++ b/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
@@ -33,7 +33,6 @@ import com.android.dialer.common.LogUtil;
 import com.android.dialer.constants.ScheduledJobIds;
 import com.android.dialer.logging.DialerImpression;
 import com.android.dialer.logging.Logger;
-import com.android.dialer.strictmode.DialerStrictMode;
 import com.android.voicemail.impl.transcribe.grpc.TranscriptionClientFactory;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -122,7 +121,6 @@ public class TranscriptionService extends JobService {
       LogUtil.i(
           "TranscriptionService.onStartJob",
           "transcription server address: " + configProvider.getServerAddress());
-      DialerStrictMode.disableDeathPenalty(); // Re-enabled in cleanup.
       jobParameters = params;
       return checkForWork();
     }
@@ -152,7 +150,6 @@ public class TranscriptionService extends JobService {
       executorService.shutdownNow();
       executorService = null;
     }
-    DialerStrictMode.enableDeathPenalty();
   }
 
   @MainThread