Fix crash on Wi-Fi Slice when it's continuously pinned and unpinned

It's because WifiTracker's initialization and onStop is on different
thread. Fine tune the thread logic in SliceBackgroundWorker.

Fixes: 118165942
Test: manual
Change-Id: Icc86b5df7ec3c6fd0e4a79a62ea0c84465e9528d

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

@@ -52,6 +52,7 @@ 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,6 +135,7 @@ public class SettingsSliceProvider extends SliceProvider {
     final Set<Uri> mRegisteredUris = new ArraySet<>();
 
     final Map<Uri, SliceBackgroundWorker> mWorkerMap = new ArrayMap<>();
+    final Set<SliceBackgroundWorker> mLiveWorkers = new ArraySet<>();
 
     public SettingsSliceProvider() {
         super(READ_SEARCH_INDEXABLES);
@@ -169,7 +171,9 @@ public class SettingsSliceProvider extends SliceProvider {
             if (filter != null) {
                 registerIntentToUri(filter, sliceUri);
             }
+            ThreadUtils.postOnMainThread(() -> {
                 startBackgroundWorker(sliceable);
+            });
             return;
         }
 
@@ -198,7 +202,9 @@ public class SettingsSliceProvider extends SliceProvider {
             SliceBroadcastRelay.unregisterReceivers(getContext(), sliceUri);
             mRegisteredUris.remove(sliceUri);
         }
+        ThreadUtils.postOnMainThread(() -> {
             stopBackgroundWorker(sliceUri);
+        });
         mSliceDataCache.remove(sliceUri);
     }
 
@@ -365,12 +371,15 @@ public class SettingsSliceProvider extends SliceProvider {
         }
 
         final Uri uri = sliceable.getUri();
-        Log.d(TAG, "Starting background worker for: " + uri);
         if (mWorkerMap.containsKey(uri)) {
             return;
         }
 
+        Log.d(TAG, "Starting background worker for: " + uri);
         mWorkerMap.put(uri, worker);
+        if (!mLiveWorkers.contains(worker)) {
+            mLiveWorkers.add(worker);
+        }
         worker.onSlicePinned();
     }
 
@@ -383,6 +392,20 @@ public class SettingsSliceProvider extends SliceProvider {
         }
     }
 
+    @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();
+    }
+
     private List<Uri> buildUrisFromKeys(List<String> keys, String authority) {
         final List<Uri> descendants = new ArrayList<>();
 

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

@@ -16,9 +16,11 @@
 
 package com.android.settings.slices;
 
+import android.annotation.MainThread;
 import android.content.ContentResolver;
 import android.net.Uri;
 
+import java.io.Closeable;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -32,7 +34,7 @@ import java.util.List;
  * {@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.
  */
-public abstract class SliceBackgroundWorker<E> {
+public abstract class SliceBackgroundWorker<E> implements Closeable {
 
     private final ContentResolver mContentResolver;
     private final Uri mUri;
@@ -48,12 +50,14 @@ public abstract class SliceBackgroundWorker<E> {
      * Called when the Slice is pinned. This is the place to register callbacks or initialize scan
      * tasks.
      */
+    @MainThread
     protected abstract void onSlicePinned();
 
     /**
      * Called when the Slice is unpinned. This is the place to unregister callbacks or perform any
      * final cleanup.
      */
+    @MainThread
     protected abstract void onSliceUnpinned();
 
     /**

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

@@ -30,8 +30,6 @@ import android.net.wifi.WifiInfo;
 import android.net.wifi.WifiManager;
 import android.net.wifi.WifiSsid;
 import android.os.Bundle;
-import android.os.Handler;
-import android.os.Looper;
 import android.provider.SettingsSlicesContract;
 import android.text.TextUtils;
 
@@ -62,7 +60,6 @@ import java.util.List;
  */
 public class WifiSlice implements CustomSliceable {
 
-
     /**
      * Backing Uri for the Wifi Slice.
      */
@@ -124,16 +121,16 @@ public class WifiSlice implements CustomSliceable {
             return listBuilder.build();
         }
 
-        List<AccessPoint> result = getBackgroundWorker().getResults();
+        List<AccessPoint> results = getBackgroundWorker().getResults();
-        if (result == null) {
+        if (results == null) {
-            result = new ArrayList<>();
+            results = new ArrayList<>();
         }
-        final int apCount = result.size();
+        final int apCount = results.size();
         // Add AP rows
         final CharSequence placeholder = mContext.getText(R.string.summary_placeholder);
         for (int i = 0; i < DEFAULT_EXPANDED_ROW_COUNT; i++) {
             if (i < apCount) {
-                listBuilder.addRow(getAccessPointRow(result.get(i)));
+                listBuilder.addRow(getAccessPointRow(results.get(i)));
             } else {
                 listBuilder.addRow(new RowBuilder()
                         .setTitle(placeholder)
@@ -277,12 +274,12 @@ public class WifiSlice implements CustomSliceable {
     private 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 WifiManager mWifiManager;
 
         private WifiScanWorker(Context context, Uri uri) {
             super(context.getContentResolver(), uri);
@@ -298,17 +295,20 @@ public class WifiSlice implements CustomSliceable {
 
         @Override
         protected void onSlicePinned() {
-            new Handler(Looper.getMainLooper()).post(() -> {
+            if (mWifiTracker == null) {
                 mWifiTracker = new WifiTracker(mContext, this, true, true);
-                mWifiManager = mWifiTracker.getManager();
+            }
             mWifiTracker.onStart();
             onAccessPointsChanged();
-            });
         }
 
         @Override
         protected void onSliceUnpinned() {
             mWifiTracker.onStop();
+        }
+
+        @Override
+        public void close() {
             mWifiTracker.onDestroy();
             mWifiScanWorker = null;
         }
@@ -324,7 +324,7 @@ public class WifiSlice implements CustomSliceable {
         @Override
         public void onAccessPointsChanged() {
             // in case state has changed
-            if (!mWifiManager.isWifiEnabled()) {
+            if (!mWifiTracker.getManager().isWifiEnabled()) {
                 updateResults(null);
                 return;
             }

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

@@ -67,6 +67,7 @@ import org.robolectric.annotation.Implementation;
 import org.robolectric.annotation.Implements;
 import org.robolectric.annotation.Resetter;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -485,11 +486,15 @@ public class SettingsSliceProviderTest {
         final SliceBackgroundWorker worker = spy(new SliceBackgroundWorker(
                 mContext.getContentResolver(), uri) {
             @Override
-            public void onSlicePinned() {
+            protected void onSlicePinned() {
             }
 
             @Override
-            public void onSliceUnpinned() {
+            protected void onSliceUnpinned() {
+            }
+
+            @Override
+            public void close() {
             }
         });
         final WifiSlice wifiSlice = spy(new WifiSlice(mContext));
@@ -519,6 +524,17 @@ public class SettingsSliceProviderTest {
         verify(worker).onSliceUnpinned();
     }
 
+    @Test
+    public void shutdown_backgroundWorker_closed() throws IOException {
+        final Uri uri = WifiSlice.WIFI_URI;
+        final SliceBackgroundWorker worker = initBackgroundWorker(uri);
+
+        mProvider.onSlicePinned(uri);
+        mProvider.shutdown();
+
+        verify(worker).close();
+    }
+
     @Test
     public void grantWhitelistedPackagePermissions_noWhitelist_shouldNotGrant() {
         final List<Uri> uris = new ArrayList<>();