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

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);
-}
+}
+
+# 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);
+}

src/com/android/settings/slices/CustomSliceable.java

@@ -91,11 +91,12 @@ public interface CustomSliceable {
 
     /**
      * 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;
     }
 

src/com/android/settings/slices/SettingsSliceProvider.java

@@ -52,7 +52,6 @@ import com.android.settings.wifi.calling.WifiCallingSliceHelper;
 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 @@ public class SettingsSliceProvider extends SliceProvider {
 
     final Set<Uri> mRegisteredUris = new ArraySet<>();
 
-    final Map<Uri, SliceBackgroundWorker> mWorkerMap = new ArrayMap<>();
+    final Map<Uri, SliceBackgroundWorker> mPinnedWorkers = new ArrayMap<>();
-    final Set<SliceBackgroundWorker> mLiveWorkers = new ArraySet<>();
 
     public SettingsSliceProvider() {
         super(READ_SEARCH_INDEXABLES);
@@ -365,45 +363,37 @@ public class SettingsSliceProvider extends SliceProvider {
     }
 
     private void startBackgroundWorker(CustomSliceable sliceable) {
-        final SliceBackgroundWorker worker = sliceable.getBackgroundWorker();
+        final Class workerClass = sliceable.getBackgroundWorkerClass();
-        if (worker == null) {
+        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);
+        final SliceBackgroundWorker worker = SliceBackgroundWorker.getInstance(
-        if (!mLiveWorkers.contains(worker)) {
+                getContext(), sliceable);
-            mLiveWorkers.add(worker);
+        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(() -> {
-            ThreadUtils.postOnMainThread(() -> {
+            SliceBackgroundWorker.shutdown();
-                try {
+        });
-                    worker.close();
-                } catch (IOException e) {
-                    Log.w(TAG, "Exception when shutting down worker", e);
-                }
-            });
-        }
-        mLiveWorkers.clear();
     }
 
     private List<Uri> buildUrisFromKeys(List<String> keys, String authority) {
@@ -532,4 +522,4 @@ public class SettingsSliceProvider extends SliceProvider {
         }
         return new String[0];
     }
-}
+}

src/com/android/settings/slices/SliceBackgroundWorker.java

@@ -17,35 +17,86 @@
 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
+ * The background worker will be started at {@link SettingsSliceProvider#onSlicePinned(Uri)}, be
- * stopped at {@link SettingsSliceProvider#onSliceUnpinned(Uri)}.
+ * 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) {
+    protected SliceBackgroundWorker(Context context, Uri uri) {
-        mContentResolver = cr;
+        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 @@ public abstract class SliceBackgroundWorker<E> implements Closeable {
 
         if (needNotify) {
             mCachedResults = results;
-            mContentResolver.notifyChange(mUri, null);
+            mContext.getContentResolver().notifyChange(mUri, null);
         }
     }
 }

src/com/android/settings/wifi/WifiSlice.java

@@ -121,7 +121,7 @@ public class WifiSlice implements CustomSliceable {
             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 @@ public class WifiSlice implements CustomSliceable {
     }
 
     @Override
-    public SliceBackgroundWorker getBackgroundWorker() {
+    public Class getBackgroundWorkerClass() {
-        return WifiScanWorker.getInstance(mContext, WIFI_URI);
+        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) {
+        public WifiScanWorker(Context context, Uri uri) {
-            super(context.getContentResolver(), 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 @@ public class WifiSlice implements CustomSliceable {
         @Override
         public void close() {
             mWifiTracker.onDestroy();
-            mWifiScanWorker = null;
         }
 
         @Override

tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java

@@ -25,6 +25,7 @@ import static org.mockito.ArgumentMatchers.any;
 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.ShadowThreadUtils;
 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.annotation.Resetter;
 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 @@ import java.util.Set;
 @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 @@ public class SettingsSliceProviderTest {
         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 @@ public class SettingsSliceProviderTest {
         mProvider.onSlicePinned(uri);
     }
 
-    private SliceBackgroundWorker initBackgroundWorker(Uri uri) {
+    @Implements(WifiSlice.WifiScanWorker.class)
-        final SliceBackgroundWorker worker = spy(new SliceBackgroundWorker(
+    public static class ShadowWifiScanWorker {
-                mContext.getContentResolver(), uri) {
+        private static WifiTracker mWifiTracker;
-            @Override
-            protected void onSlicePinned() {
-            }
 
-            @Override
+        @Implementation
-            protected void onSliceUnpinned() {
+        protected void onSlicePinned() {
-            }
+            mWifiTracker = mock(WifiTracker.class);
+            mWifiTracker.onStart();
+        }
 
-            @Override
+        @Implementation
-            public void close() {
+        protected void onSliceUnpinned() {
-            }
+            mWifiTracker.onStop();
-        });
+        }
-        final WifiSlice wifiSlice = spy(new WifiSlice(mContext));
+
-        when(wifiSlice.getBackgroundWorker()).thenReturn(worker);
+        @Implementation
-        when(mProvider.mCustomSliceManager.getSliceableFromUri(uri)).thenReturn(wifiSlice);
+        public void close() {
-        return worker;
+            mWifiTracker.onDestroy();
+        }
+
+        static WifiTracker getWifiTracker() {
+            return mWifiTracker;
+        }
     }
 
     @Test
     public void onSlicePinned_backgroundWorker_started() {
-        final Uri uri = WifiSlice.WIFI_URI;
+        mProvider.onSlicePinned(WifiSlice.WIFI_URI);
-        final SliceBackgroundWorker worker = initBackgroundWorker(uri);
 
-        mProvider.onSlicePinned(uri);
+        verify(ShadowWifiScanWorker.getWifiTracker()).onStart();
-
-        verify(worker).onSlicePinned();
     }
 
     @Test
     public void onSlicePinned_backgroundWorker_stopped() {
-        final Uri uri = WifiSlice.WIFI_URI;
+        mProvider.onSlicePinned(WifiSlice.WIFI_URI);
-        final SliceBackgroundWorker worker = initBackgroundWorker(uri);
+        mProvider.onSliceUnpinned(WifiSlice.WIFI_URI);
 
-        mProvider.onSlicePinned(uri);
+        verify(ShadowWifiScanWorker.getWifiTracker()).onStop();
-        mProvider.onSliceUnpinned(uri);
-
-        verify(worker).onSliceUnpinned();
     }
 
     @Test
-    public void shutdown_backgroundWorker_closed() throws IOException {
+    public void shutdown_backgroundWorker_closed() {
-        final Uri uri = WifiSlice.WIFI_URI;
+        mProvider.onSlicePinned(WifiSlice.WIFI_URI);
-        final SliceBackgroundWorker worker = initBackgroundWorker(uri);
-
-        mProvider.onSlicePinned(uri);
         mProvider.shutdown();
 
-        verify(worker).close();
+        verify(ShadowWifiScanWorker.getWifiTracker()).onDestroy();
     }
 
     @Test
@@ -630,4 +627,4 @@ public class SettingsSliceProviderTest {
             return sSetThreadPolicyCount != 0;
         }
     }
-}
+}