Enforce all the SliceBackgroundWorkers being singletons at syntax level
- Create workers via reflection in SliceBackgroundWorker
- Store the workers in a static container and release then at shutdown()
Fixes: 118228009
Test: robolectric
Change-Id: I564277d3a12b2d7d3b50cef091bdfedb3397c145
diff --git a/proguard.flags b/proguard.flags
index 82e8e58..b66a786 100644
--- a/proguard.flags
+++ b/proguard.flags
@@ -60,4 +60,9 @@
# Keep classes that implements CustomSliceable, which are used by reflection.
-keepclasseswithmembers class * implements com.android.settings.slices.CustomSliceable {
public <init>(android.content.Context);
-}
\ No newline at end of file
+}
+
+# Keep classes that extends SliceBackgroundWorker, which are used by reflection.
+-keepclasseswithmembers class * extends com.android.settings.slices.SliceBackgroundWorker {
+ public <init>(android.content.Context, android.net.Uri);
+}
diff --git a/src/com/android/settings/slices/CustomSliceable.java b/src/com/android/settings/slices/CustomSliceable.java
index e09cc73..b538b89 100644
--- a/src/com/android/settings/slices/CustomSliceable.java
+++ b/src/com/android/settings/slices/CustomSliceable.java
@@ -91,11 +91,12 @@
/**
* Settings Slices which can represent component lists that are updatable by the
- * {@link SliceBackgroundWorker} returned here.
+ * {@link SliceBackgroundWorker} class returned here.
*
- * @return a {@link SliceBackgroundWorker} for fetching the list of results in the background.
+ * @return a {@link SliceBackgroundWorker} class for fetching the list of results in the
+ * background.
*/
- default SliceBackgroundWorker getBackgroundWorker() {
+ default Class<? extends SliceBackgroundWorker> getBackgroundWorkerClass() {
return null;
}
diff --git a/src/com/android/settings/slices/SettingsSliceProvider.java b/src/com/android/settings/slices/SettingsSliceProvider.java
index 3911603..33576f5 100644
--- a/src/com/android/settings/slices/SettingsSliceProvider.java
+++ b/src/com/android/settings/slices/SettingsSliceProvider.java
@@ -52,7 +52,6 @@
import com.android.settingslib.SliceBroadcastRelay;
import com.android.settingslib.utils.ThreadUtils;
-import java.io.IOException;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Arrays;
@@ -134,8 +133,7 @@
final Set<Uri> mRegisteredUris = new ArraySet<>();
- final Map<Uri, SliceBackgroundWorker> mWorkerMap = new ArrayMap<>();
- final Set<SliceBackgroundWorker> mLiveWorkers = new ArraySet<>();
+ final Map<Uri, SliceBackgroundWorker> mPinnedWorkers = new ArrayMap<>();
public SettingsSliceProvider() {
super(READ_SEARCH_INDEXABLES);
@@ -365,45 +363,37 @@
}
private void startBackgroundWorker(CustomSliceable sliceable) {
- final SliceBackgroundWorker worker = sliceable.getBackgroundWorker();
- if (worker == null) {
+ final Class workerClass = sliceable.getBackgroundWorkerClass();
+ if (workerClass == null) {
return;
}
final Uri uri = sliceable.getUri();
- if (mWorkerMap.containsKey(uri)) {
+ if (mPinnedWorkers.containsKey(uri)) {
return;
}
Log.d(TAG, "Starting background worker for: " + uri);
- mWorkerMap.put(uri, worker);
- if (!mLiveWorkers.contains(worker)) {
- mLiveWorkers.add(worker);
- }
+ final SliceBackgroundWorker worker = SliceBackgroundWorker.getInstance(
+ getContext(), sliceable);
+ mPinnedWorkers.put(uri, worker);
worker.onSlicePinned();
}
private void stopBackgroundWorker(Uri uri) {
- final SliceBackgroundWorker worker = mWorkerMap.get(uri);
+ final SliceBackgroundWorker worker = mPinnedWorkers.get(uri);
if (worker != null) {
Log.d(TAG, "Stopping background worker for: " + uri);
worker.onSliceUnpinned();
- mWorkerMap.remove(uri);
+ mPinnedWorkers.remove(uri);
}
}
@Override
public void shutdown() {
- for (SliceBackgroundWorker worker : mLiveWorkers) {
- ThreadUtils.postOnMainThread(() -> {
- try {
- worker.close();
- } catch (IOException e) {
- Log.w(TAG, "Exception when shutting down worker", e);
- }
- });
- }
- mLiveWorkers.clear();
+ ThreadUtils.postOnMainThread(() -> {
+ SliceBackgroundWorker.shutdown();
+ });
}
private List<Uri> buildUrisFromKeys(List<String> keys, String authority) {
@@ -532,4 +522,4 @@
}
return new String[0];
}
-}
\ No newline at end of file
+}
diff --git a/src/com/android/settings/slices/SliceBackgroundWorker.java b/src/com/android/settings/slices/SliceBackgroundWorker.java
index 422fcc7..a663ece 100644
--- a/src/com/android/settings/slices/SliceBackgroundWorker.java
+++ b/src/com/android/settings/slices/SliceBackgroundWorker.java
@@ -17,36 +17,87 @@
package com.android.settings.slices;
import android.annotation.MainThread;
-import android.content.ContentResolver;
+import android.content.Context;
import android.net.Uri;
+import android.util.ArrayMap;
+import android.util.Log;
import java.io.Closeable;
+import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.List;
+import java.util.Map;
/**
* The Slice background worker is used to make Settings Slices be able to work with data that is
* changing continuously, e.g. available Wi-Fi networks.
*
- * The background worker will be started at {@link SettingsSliceProvider#onSlicePinned(Uri)}, and be
- * stopped at {@link SettingsSliceProvider#onSliceUnpinned(Uri)}.
+ * The background worker will be started at {@link SettingsSliceProvider#onSlicePinned(Uri)}, be
+ * stopped at {@link SettingsSliceProvider#onSliceUnpinned(Uri)}, and be closed at {@link
+ * SettingsSliceProvider#shutdown()}.
*
* {@link SliceBackgroundWorker} caches the results, uses the cache to compare if there is any data
* changed, and then notifies the Slice {@link Uri} to update.
+ *
+ * It also stores all instances of all workers to ensure each worker is a Singleton.
*/
public abstract class SliceBackgroundWorker<E> implements Closeable {
- private final ContentResolver mContentResolver;
+ private static final String TAG = "SliceBackgroundWorker";
+
+ private static final Map<Uri, SliceBackgroundWorker> LIVE_WORKERS = new ArrayMap<>();
+
+ private final Context mContext;
private final Uri mUri;
private List<E> mCachedResults;
- protected SliceBackgroundWorker(ContentResolver cr, Uri uri) {
- mContentResolver = cr;
+ protected SliceBackgroundWorker(Context context, Uri uri) {
+ mContext = context;
mUri = uri;
}
/**
+ * Returns the singleton instance of the {@link SliceBackgroundWorker} for specified {@link
+ * CustomSliceable}
+ */
+ public static SliceBackgroundWorker getInstance(Context context, CustomSliceable sliceable) {
+ final Uri uri = sliceable.getUri();
+ final Class<? extends SliceBackgroundWorker> workerClass =
+ sliceable.getBackgroundWorkerClass();
+ SliceBackgroundWorker worker = LIVE_WORKERS.get(uri);
+ if (worker == null) {
+ worker = createInstance(context, uri, workerClass);
+ LIVE_WORKERS.put(uri, worker);
+ }
+ return worker;
+ }
+
+ private static SliceBackgroundWorker createInstance(Context context, Uri uri,
+ Class<? extends SliceBackgroundWorker> clazz) {
+ Log.d(TAG, "create instance: " + clazz);
+ try {
+ return clazz.getConstructor(Context.class, Uri.class).newInstance(context, uri);
+ } catch (NoSuchMethodException | IllegalAccessException | InstantiationException |
+ InvocationTargetException e) {
+ throw new IllegalStateException(
+ "Invalid slice background worker: " + clazz, e);
+ }
+ }
+
+ static void shutdown() {
+ for (SliceBackgroundWorker worker : LIVE_WORKERS.values()) {
+ try {
+ worker.close();
+ } catch (IOException e) {
+ Log.w(TAG, "Shutting down worker failed", e);
+ }
+ }
+ LIVE_WORKERS.clear();
+ }
+
+ /**
* Called when the Slice is pinned. This is the place to register callbacks or initialize scan
* tasks.
*/
@@ -83,7 +134,7 @@
if (needNotify) {
mCachedResults = results;
- mContentResolver.notifyChange(mUri, null);
+ mContext.getContentResolver().notifyChange(mUri, null);
}
}
}
diff --git a/src/com/android/settings/wifi/WifiSlice.java b/src/com/android/settings/wifi/WifiSlice.java
index 43f5372..d06d830 100644
--- a/src/com/android/settings/wifi/WifiSlice.java
+++ b/src/com/android/settings/wifi/WifiSlice.java
@@ -121,7 +121,7 @@
return listBuilder.build();
}
- List<AccessPoint> results = getBackgroundWorker().getResults();
+ List<AccessPoint> results = SliceBackgroundWorker.getInstance(mContext, this).getResults();
if (results == null) {
results = new ArrayList<>();
}
@@ -264,36 +264,27 @@
}
@Override
- public SliceBackgroundWorker getBackgroundWorker() {
- return WifiScanWorker.getInstance(mContext, WIFI_URI);
+ public Class getBackgroundWorkerClass() {
+ return WifiScanWorker.class;
}
- private static class WifiScanWorker extends SliceBackgroundWorker<AccessPoint>
+ public static class WifiScanWorker extends SliceBackgroundWorker<AccessPoint>
implements WifiTracker.WifiListener {
- // TODO: enforce all the SliceBackgroundWorkers being singletons at syntax level
- private static WifiScanWorker mWifiScanWorker;
-
private final Context mContext;
private WifiTracker mWifiTracker;
- private WifiScanWorker(Context context, Uri uri) {
- super(context.getContentResolver(), uri);
+ public WifiScanWorker(Context context, Uri uri) {
+ super(context, uri);
mContext = context;
}
- public static WifiScanWorker getInstance(Context context, Uri uri) {
- if (mWifiScanWorker == null) {
- mWifiScanWorker = new WifiScanWorker(context, uri);
- }
- return mWifiScanWorker;
- }
-
@Override
protected void onSlicePinned() {
if (mWifiTracker == null) {
- mWifiTracker = new WifiTracker(mContext, this, true, true);
+ mWifiTracker = new WifiTracker(mContext, this /* wifiListener */,
+ true /* includeSaved */, true /* includeScans */);
}
mWifiTracker.onStart();
onAccessPointsChanged();
@@ -307,7 +298,6 @@
@Override
public void close() {
mWifiTracker.onDestroy();
- mWifiScanWorker = null;
}
@Override
diff --git a/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java b/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java
index a0fd21b..3c2cbdb 100644
--- a/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java
+++ b/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java
@@ -25,6 +25,7 @@
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
@@ -60,6 +61,7 @@
import com.android.settings.testutils.shadow.ShadowUserManager;
import com.android.settings.testutils.shadow.ShadowUtils;
import com.android.settings.wifi.WifiSlice;
+import com.android.settingslib.wifi.WifiTracker;
import org.junit.After;
import org.junit.Before;
@@ -75,7 +77,6 @@
import org.robolectric.shadow.api.Shadow;
import org.robolectric.shadows.ShadowAccessibilityManager;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -91,7 +92,8 @@
@RunWith(SettingsRobolectricTestRunner.class)
@Config(shadows = {ShadowUserManager.class, ShadowThreadUtils.class, ShadowUtils.class,
SlicesDatabaseAccessorTest.ShadowApplicationPackageManager.class,
- ShadowBluetoothAdapter.class, ShadowLockPatternUtils.class})
+ ShadowBluetoothAdapter.class, ShadowLockPatternUtils.class,
+ SettingsSliceProviderTest.ShadowWifiScanWorker.class})
public class SettingsSliceProviderTest {
private static final String KEY = "KEY";
@@ -135,7 +137,7 @@
mProvider.mSliceWeakDataCache = new HashMap<>();
mProvider.mSliceDataCache = new HashMap<>();
mProvider.mSlicesDatabaseAccessor = new SlicesDatabaseAccessor(mContext);
- mProvider.mCustomSliceManager = spy(new CustomSliceManager(mContext));
+ mProvider.mCustomSliceManager = new CustomSliceManager(mContext);
when(mProvider.getContext()).thenReturn(mContext);
SlicesDatabaseHelper.getInstance(mContext).setIndexedState();
@@ -497,57 +499,52 @@
mProvider.onSlicePinned(uri);
}
- private SliceBackgroundWorker initBackgroundWorker(Uri uri) {
- final SliceBackgroundWorker worker = spy(new SliceBackgroundWorker(
- mContext.getContentResolver(), uri) {
- @Override
- protected void onSlicePinned() {
- }
+ @Implements(WifiSlice.WifiScanWorker.class)
+ public static class ShadowWifiScanWorker {
+ private static WifiTracker mWifiTracker;
- @Override
- protected void onSliceUnpinned() {
- }
+ @Implementation
+ protected void onSlicePinned() {
+ mWifiTracker = mock(WifiTracker.class);
+ mWifiTracker.onStart();
+ }
- @Override
- public void close() {
- }
- });
- final WifiSlice wifiSlice = spy(new WifiSlice(mContext));
- when(wifiSlice.getBackgroundWorker()).thenReturn(worker);
- when(mProvider.mCustomSliceManager.getSliceableFromUri(uri)).thenReturn(wifiSlice);
- return worker;
+ @Implementation
+ protected void onSliceUnpinned() {
+ mWifiTracker.onStop();
+ }
+
+ @Implementation
+ public void close() {
+ mWifiTracker.onDestroy();
+ }
+
+ static WifiTracker getWifiTracker() {
+ return mWifiTracker;
+ }
}
@Test
public void onSlicePinned_backgroundWorker_started() {
- final Uri uri = WifiSlice.WIFI_URI;
- final SliceBackgroundWorker worker = initBackgroundWorker(uri);
+ mProvider.onSlicePinned(WifiSlice.WIFI_URI);
- mProvider.onSlicePinned(uri);
-
- verify(worker).onSlicePinned();
+ verify(ShadowWifiScanWorker.getWifiTracker()).onStart();
}
@Test
public void onSlicePinned_backgroundWorker_stopped() {
- final Uri uri = WifiSlice.WIFI_URI;
- final SliceBackgroundWorker worker = initBackgroundWorker(uri);
+ mProvider.onSlicePinned(WifiSlice.WIFI_URI);
+ mProvider.onSliceUnpinned(WifiSlice.WIFI_URI);
- mProvider.onSlicePinned(uri);
- mProvider.onSliceUnpinned(uri);
-
- verify(worker).onSliceUnpinned();
+ verify(ShadowWifiScanWorker.getWifiTracker()).onStop();
}
@Test
- public void shutdown_backgroundWorker_closed() throws IOException {
- final Uri uri = WifiSlice.WIFI_URI;
- final SliceBackgroundWorker worker = initBackgroundWorker(uri);
-
- mProvider.onSlicePinned(uri);
+ public void shutdown_backgroundWorker_closed() {
+ mProvider.onSlicePinned(WifiSlice.WIFI_URI);
mProvider.shutdown();
- verify(worker).close();
+ verify(ShadowWifiScanWorker.getWifiTracker()).onDestroy();
}
@Test
@@ -630,4 +627,4 @@
return sSetThreadPolicyCount != 0;
}
}
-}
\ No newline at end of file
+}