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
Bug: 79985175
Test: robotests
This commit is contained in:
Fan Zhang
2018-05-24 10:10:02 -07:00
parent cf84bf0d6a
commit 934de22622
4 changed files with 116 additions and 52 deletions

View File

@@ -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;
@@ -83,7 +81,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);
@@ -119,9 +117,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();
}

View File

@@ -23,6 +23,7 @@ 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.text.TextUtils;
@@ -31,13 +32,13 @@ 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;
@@ -53,7 +54,6 @@ import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentHashMap;
import androidx.annotation.VisibleForTesting;
import androidx.core.graphics.drawable.IconCompat;
import androidx.slice.Slice;
import androidx.slice.SliceProvider;
@@ -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);
mRegisteredUris.add(sliceUri);
return;
} else if (ZenModeSliceBuilder.ZEN_MODE_URI.equals(sliceUri)) {
@@ -177,6 +177,13 @@ public class SettingsSliceProvider extends SliceProvider {
@Override
public Slice onBindSlice(Uri sliceUri) {
final StrictMode.ThreadPolicy oldPolicy = StrictMode.getThreadPolicy();
try {
if (!ThreadUtils.isMainThread()) {
StrictMode.setThreadPolicy(new StrictMode.ThreadPolicy.Builder()
.permitAll()
.build());
}
final Set<String> blockedKeys = getBlockedKeys();
final String key = sliceUri.getLastPathSegment();
if (blockedKeys.contains(key)) {
@@ -212,6 +219,9 @@ public class SettingsSliceProvider extends SliceProvider {
mSliceWeakDataCache.remove(sliceUri);
}
return SliceBuilderUtils.buildSlice(getContext(), cachedSliceData);
} finally {
StrictMode.setThreadPolicy(oldPolicy);
}
}
/**

View File

@@ -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);
@@ -196,7 +203,19 @@ public class SettingsSliceProviderTest {
}
@Test
public void onBindSlice_requestsBlockedSlice_retunsNull() {
@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_returnsNull() {
final String blockedKey = "blocked_key";
final Set<String> blockedSet = new ArraySet<>();
blockedSet.add(blockedKey);
@@ -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;
}
}
}

View File

@@ -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;
}
}