From 2313b2421596b01a36bd2db0a171bbd050f1ddbc Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Thu, 24 May 2018 10:10:02 -0700 Subject: [PATCH] 2nd attempt to fix Slice strict mode. 1. Use real BluetoothAdapter instead of settingslib version. The settingslib version contains calls that violates strictmode rules. 2. Override StrictMode rules in SettingsSliceProvider when it's called in background thread. When in background, the enforcement from Slice framework (StrictMode#ThreadPolicy) is not useful and can be safely ignored. Change-Id: I68523148f4c1dc88a54e207447d21ec439478cdf Merged-In: I68523148f4c1dc88a54e207447d21ec439478cdf Fixes: 79985175 Test: robotests --- .../bluetooth/BluetoothSliceBuilder.java | 15 ++-- .../slices/SettingsSliceProvider.java | 84 +++++++++++-------- .../slices/SettingsSliceProviderTest.java | 51 +++++++++-- .../testutils/shadow/ShadowThreadUtils.java | 18 ++++ 4 files changed, 116 insertions(+), 52 deletions(-) diff --git a/src/com/android/settings/bluetooth/BluetoothSliceBuilder.java b/src/com/android/settings/bluetooth/BluetoothSliceBuilder.java index da759a33e5e..9a9f45588e7 100644 --- a/src/com/android/settings/bluetooth/BluetoothSliceBuilder.java +++ b/src/com/android/settings/bluetooth/BluetoothSliceBuilder.java @@ -17,8 +17,6 @@ package com.android.settings.bluetooth; import static android.app.slice.Slice.EXTRA_TOGGLE_STATE; -import static androidx.slice.builders.ListBuilder.ICON_IMAGE; - import android.annotation.ColorInt; import android.app.PendingIntent; import android.bluetooth.BluetoothAdapter; @@ -28,6 +26,7 @@ import android.content.Intent; import android.content.IntentFilter; import android.net.Uri; import android.provider.SettingsSlicesContract; +import android.support.v4.graphics.drawable.IconCompat; import com.android.internal.logging.nano.MetricsProto; import com.android.settings.R; @@ -42,8 +41,6 @@ import androidx.slice.Slice; import androidx.slice.builders.ListBuilder; import androidx.slice.builders.SliceAction; -import android.support.v4.graphics.drawable.IconCompat; - public class BluetoothSliceBuilder { private static final String TAG = "BluetoothSliceBuilder"; @@ -71,7 +68,8 @@ public class BluetoothSliceBuilder { INTENT_FILTER.addAction(BluetoothAdapter.ACTION_STATE_CHANGED); } - private BluetoothSliceBuilder() {} + private BluetoothSliceBuilder() { + } /** * Return a Bluetooth Slice bound to {@link #BLUETOOTH_URI}. @@ -80,7 +78,7 @@ public class BluetoothSliceBuilder { * Bluetooth. */ public static Slice getSlice(Context context) { - final boolean isBluetoothEnabled = isBluetoothEnabled(context); + final boolean isBluetoothEnabled = isBluetoothEnabled(); final CharSequence title = context.getText(R.string.bluetooth_settings); final IconCompat icon = IconCompat.createWithResource(context, R.drawable.ic_settings_bluetooth); @@ -115,9 +113,8 @@ public class BluetoothSliceBuilder { // handle it. } - private static boolean isBluetoothEnabled(Context context) { - final LocalBluetoothAdapter adapter = LocalBluetoothManager.getInstance(context, - null /* callback */).getBluetoothAdapter(); + private static boolean isBluetoothEnabled() { + final BluetoothAdapter adapter = BluetoothAdapter.getDefaultAdapter(); return adapter.isEnabled(); } diff --git a/src/com/android/settings/slices/SettingsSliceProvider.java b/src/com/android/settings/slices/SettingsSliceProvider.java index 47736dfac2e..841247b1d64 100644 --- a/src/com/android/settings/slices/SettingsSliceProvider.java +++ b/src/com/android/settings/slices/SettingsSliceProvider.java @@ -23,23 +23,23 @@ import android.content.ContentResolver; import android.content.Intent; import android.content.IntentFilter; import android.net.Uri; +import android.os.StrictMode; import android.provider.Settings; import android.provider.SettingsSlicesContract; import android.support.annotation.VisibleForTesting; -import android.support.v4.graphics.drawable.IconCompat; import android.text.TextUtils; import android.util.ArraySet; import android.util.KeyValueListParser; import android.util.Log; import android.util.Pair; -import com.android.settings.location.LocationSliceBuilder; -import com.android.settings.overlay.FeatureFactory; +import com.android.settings.bluetooth.BluetoothSliceBuilder; import com.android.settings.core.BasePreferenceController; +import com.android.settings.location.LocationSliceBuilder; +import com.android.settings.notification.ZenModeSliceBuilder; +import com.android.settings.overlay.FeatureFactory; import com.android.settings.wifi.WifiSliceBuilder; import com.android.settings.wifi.calling.WifiCallingSliceHelper; -import com.android.settings.bluetooth.BluetoothSliceBuilder; -import com.android.settings.notification.ZenModeSliceBuilder; import com.android.settingslib.SliceBroadcastRelay; import com.android.settingslib.utils.ThreadUtils; @@ -150,7 +150,7 @@ public class SettingsSliceProvider extends SliceProvider { @Override public void onSlicePinned(Uri sliceUri) { if (WifiSliceBuilder.WIFI_URI.equals(sliceUri)) { - registerIntentToUri(WifiSliceBuilder.INTENT_FILTER , sliceUri); + registerIntentToUri(WifiSliceBuilder.INTENT_FILTER, sliceUri); return; } else if (ZenModeSliceBuilder.ZEN_MODE_URI.equals(sliceUri)) { registerIntentToUri(ZenModeSliceBuilder.INTENT_FILTER, sliceUri); @@ -176,41 +176,51 @@ public class SettingsSliceProvider extends SliceProvider { @Override public Slice onBindSlice(Uri sliceUri) { - final Set blockedKeys = getBlockedKeys(); - final String key = sliceUri.getLastPathSegment(); - if (blockedKeys.contains(key)) { - Log.e(TAG, "Requested blocked slice with Uri: " + sliceUri); - return null; - } + final StrictMode.ThreadPolicy oldPolicy = StrictMode.getThreadPolicy(); + try { + if (!ThreadUtils.isMainThread()) { + StrictMode.setThreadPolicy(new StrictMode.ThreadPolicy.Builder() + .permitAll() + .build()); + } + final Set blockedKeys = getBlockedKeys(); + final String key = sliceUri.getLastPathSegment(); + if (blockedKeys.contains(key)) { + Log.e(TAG, "Requested blocked slice with Uri: " + sliceUri); + return null; + } - // If adding a new Slice, do not directly match Slice URIs. - // Use {@link SlicesDatabaseAccessor}. - if (WifiCallingSliceHelper.WIFI_CALLING_URI.equals(sliceUri)) { - return FeatureFactory.getFactory(getContext()) - .getSlicesFeatureProvider() - .getNewWifiCallingSliceHelper(getContext()) - .createWifiCallingSlice(sliceUri); - } else if (WifiSliceBuilder.WIFI_URI.equals(sliceUri)) { - return WifiSliceBuilder.getSlice(getContext()); - } else if (ZenModeSliceBuilder.ZEN_MODE_URI.equals(sliceUri)) { - return ZenModeSliceBuilder.getSlice(getContext()); - } else if (BluetoothSliceBuilder.BLUETOOTH_URI.equals(sliceUri)) { - return BluetoothSliceBuilder.getSlice(getContext()); - } else if (LocationSliceBuilder.LOCATION_URI.equals(sliceUri)) { - return LocationSliceBuilder.getSlice(getContext()); - } + // If adding a new Slice, do not directly match Slice URIs. + // Use {@link SlicesDatabaseAccessor}. + if (WifiCallingSliceHelper.WIFI_CALLING_URI.equals(sliceUri)) { + return FeatureFactory.getFactory(getContext()) + .getSlicesFeatureProvider() + .getNewWifiCallingSliceHelper(getContext()) + .createWifiCallingSlice(sliceUri); + } else if (WifiSliceBuilder.WIFI_URI.equals(sliceUri)) { + return WifiSliceBuilder.getSlice(getContext()); + } else if (ZenModeSliceBuilder.ZEN_MODE_URI.equals(sliceUri)) { + return ZenModeSliceBuilder.getSlice(getContext()); + } else if (BluetoothSliceBuilder.BLUETOOTH_URI.equals(sliceUri)) { + return BluetoothSliceBuilder.getSlice(getContext()); + } else if (LocationSliceBuilder.LOCATION_URI.equals(sliceUri)) { + return LocationSliceBuilder.getSlice(getContext()); + } - SliceData cachedSliceData = mSliceWeakDataCache.get(sliceUri); - if (cachedSliceData == null) { - loadSliceInBackground(sliceUri); - return getSliceStub(sliceUri); - } + SliceData cachedSliceData = mSliceWeakDataCache.get(sliceUri); + if (cachedSliceData == null) { + loadSliceInBackground(sliceUri); + return getSliceStub(sliceUri); + } - // Remove the SliceData from the cache after it has been used to prevent a memory-leak. - if (!mSliceDataCache.containsKey(sliceUri)) { - mSliceWeakDataCache.remove(sliceUri); + // Remove the SliceData from the cache after it has been used to prevent a memory-leak. + if (!mSliceDataCache.containsKey(sliceUri)) { + mSliceWeakDataCache.remove(sliceUri); + } + return SliceBuilderUtils.buildSlice(getContext(), cachedSliceData); + } finally { + StrictMode.setThreadPolicy(oldPolicy); } - return SliceBuilderUtils.buildSlice(getContext(), cachedSliceData); } /** diff --git a/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java b/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java index df960d8a912..21bef6131c9 100644 --- a/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java +++ b/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java @@ -18,9 +18,7 @@ package com.android.settings.slices; import static android.content.ContentResolver.SCHEME_CONTENT; - import static com.google.common.truth.Truth.assertThat; - import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -38,19 +36,24 @@ import android.os.StrictMode; import android.provider.SettingsSlicesContract; import android.util.ArraySet; -import com.android.settings.location.LocationSliceBuilder; -import com.android.settings.wifi.WifiSliceBuilder; import com.android.settings.bluetooth.BluetoothSliceBuilder; +import com.android.settings.location.LocationSliceBuilder; import com.android.settings.notification.ZenModeSliceBuilder; import com.android.settings.testutils.DatabaseTestUtils; import com.android.settings.testutils.FakeToggleController; import com.android.settings.testutils.SettingsRobolectricTestRunner; +import com.android.settings.testutils.shadow.ShadowThreadUtils; +import com.android.settings.wifi.WifiSliceBuilder; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; +import org.robolectric.annotation.Resetter; import java.util.Arrays; import java.util.Collection; @@ -66,6 +69,7 @@ import androidx.slice.Slice; * TODO Investigate using ShadowContentResolver.registerProviderInternal(String, ContentProvider) */ @RunWith(SettingsRobolectricTestRunner.class) +@Config(shadows = ShadowThreadUtils.class) public class SettingsSliceProviderTest { private static final String KEY = "KEY"; @@ -98,6 +102,7 @@ public class SettingsSliceProviderTest { public void setUp() { mContext = spy(RuntimeEnvironment.application); mProvider = spy(new SettingsSliceProvider()); + ShadowStrictMode.reset(); mProvider.mSliceWeakDataCache = new HashMap<>(); mProvider.mSliceDataCache = new HashMap<>(); mProvider.mSlicesDatabaseAccessor = new SlicesDatabaseAccessor(mContext); @@ -112,6 +117,7 @@ public class SettingsSliceProviderTest { @After public void cleanUp() { + ShadowThreadUtils.reset(); DatabaseTestUtils.clearDb(mContext); } @@ -184,7 +190,8 @@ public class SettingsSliceProviderTest { } @Test - public void onBindSlice_shouldNotOverrideStrictMode() { + public void onBindSlice_mainThread_shouldNotOverrideStrictMode() { + ShadowThreadUtils.setIsMainThread(true); final StrictMode.ThreadPolicy oldThreadPolicy = StrictMode.getThreadPolicy(); SliceData data = getDummyData(); mProvider.mSliceWeakDataCache.put(data.getUri(), data); @@ -195,6 +202,18 @@ public class SettingsSliceProviderTest { assertThat(newThreadPolicy.toString()).isEqualTo(oldThreadPolicy.toString()); } + @Test + @Config(shadows = ShadowStrictMode.class) + public void onBindSlice_backgroundThread_shouldOverrideStrictMode() { + ShadowThreadUtils.setIsMainThread(false); + + SliceData data = getDummyData(); + mProvider.mSliceWeakDataCache.put(data.getUri(), data); + mProvider.onBindSlice(data.getUri()); + + assertThat(ShadowStrictMode.isThreadPolicyOverridden()).isTrue(); + } + @Test public void onBindSlice_requestsBlockedSlice_retunsNull() { final String blockedKey = "blocked_key"; @@ -456,7 +475,7 @@ public class SettingsSliceProviderTest { mDb.replaceOrThrow(SlicesDatabaseHelper.Tables.TABLE_SLICES_INDEX, null, values); } - private SliceData getDummyData() { + private static SliceData getDummyData() { return new SliceData.Builder() .setKey(KEY) .setTitle(TITLE) @@ -468,4 +487,24 @@ public class SettingsSliceProviderTest { .setPreferenceControllerClassName(PREF_CONTROLLER) .build(); } + + @Implements(value = StrictMode.class, inheritImplementationMethods = true) + public static class ShadowStrictMode { + + private static int sSetThreadPolicyCount; + + @Resetter + public static void reset() { + sSetThreadPolicyCount = 0; + } + + @Implementation + public static void setThreadPolicy(final StrictMode.ThreadPolicy policy) { + sSetThreadPolicyCount++; + } + + public static boolean isThreadPolicyOverridden() { + return sSetThreadPolicyCount != 0; + } + } } \ No newline at end of file diff --git a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowThreadUtils.java b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowThreadUtils.java index 6b0411e9ab0..9513098e862 100644 --- a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowThreadUtils.java +++ b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowThreadUtils.java @@ -20,10 +20,18 @@ import com.android.settingslib.utils.ThreadUtils; import org.robolectric.annotation.Implementation; import org.robolectric.annotation.Implements; +import org.robolectric.annotation.Resetter; @Implements(ThreadUtils.class) public class ShadowThreadUtils { + private static boolean sIsMainThread = true; + + @Resetter + public static void reset() { + sIsMainThread = true; + } + @Implementation public static void postOnBackgroundThread(Runnable runnable) { runnable.run(); @@ -33,4 +41,14 @@ public class ShadowThreadUtils { public static void postOnMainThread(Runnable runnable) { runnable.run(); } + + @Implementation + public static boolean isMainThread() { + return sIsMainThread; + } + + public static void setIsMainThread(boolean isMainThread) { + sIsMainThread = isMainThread; + } + }