From f7afded1d1eb7642daf1edfc06589401e3e745c0 Mon Sep 17 00:00:00 2001 From: Jason Chiu Date: Thu, 11 Jun 2020 16:28:58 +0800 Subject: [PATCH] Fix Slice not pinned error Slices should be pinned before being bound. The original design calls registerSliceCallback() to pin a slice, and then calls bindSlice() and passes the result to the callback directly. When the callback is called, it executes unregisterSliceCallback() and unpins the slice. However, registerSliceCallback() starts to observe the slice change and then rebind it in an AsyncTask. If the slice is updating via its background worker and the timing of the binding overlaps the callback execution, it's possible to bind the slice right after unpinning it and causes the error. The solution is to remove the callback mechanism, and just to pin, bind and unpin the slice directly. Fixes: 157387583 Test: robotest Change-Id: I8748dd3038a3662599935f07420d07cf254a4073 --- .../contextualcards/EligibleCardChecker.java | 38 +++---------------- .../EligibleCardCheckerTest.java | 4 +- .../SliceContextualCardRendererTest.java | 4 +- 3 files changed, 9 insertions(+), 37 deletions(-) diff --git a/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java b/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java index 4e8c61f8fad..017bdb53fdb 100644 --- a/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java +++ b/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java @@ -35,13 +35,10 @@ import com.android.settingslib.core.instrumentation.MetricsFeatureProvider; import java.util.List; import java.util.concurrent.Callable; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; public class EligibleCardChecker implements Callable { private static final String TAG = "EligibleCardChecker"; - private static final long LATCH_TIMEOUT_MS = 300; private final Context mContext; @@ -54,7 +51,7 @@ public class EligibleCardChecker implements Callable { } @Override - public ContextualCard call() throws Exception { + public ContextualCard call() { final long startTime = System.currentTimeMillis(); final MetricsFeatureProvider metricsFeatureProvider = FeatureFactory.getFactory(mContext).getMetricsFeatureProvider(); @@ -113,35 +110,10 @@ public class EligibleCardChecker implements Callable { @VisibleForTesting Slice bindSlice(Uri uri) { final SliceViewManager manager = SliceViewManager.getInstance(mContext); - final Slice[] returnSlice = new Slice[1]; - final CountDownLatch latch = new CountDownLatch(1); - final SliceViewManager.SliceCallback callback = - new SliceViewManager.SliceCallback() { - @Override - public void onSliceUpdated(Slice slice) { - try { - // We are just making sure the existence of the slice, so ignore - // slice loading state here. - returnSlice[0] = slice; - latch.countDown(); - } catch (Exception e) { - Log.w(TAG, uri + " cannot be indexed", e); - } finally { - manager.unregisterSliceCallback(uri, this); - } - } - }; - // Register a callback until we get a loaded slice. - manager.registerSliceCallback(uri, callback); - // Trigger the binding. - callback.onSliceUpdated(manager.bindSlice(uri)); - try { - latch.await(LATCH_TIMEOUT_MS, TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - Log.w(TAG, "Error waiting for slice binding for uri" + uri, e); - manager.unregisterSliceCallback(uri, callback); - } - return returnSlice[0]; + manager.pinSlice(uri); + final Slice slice = manager.bindSlice(uri); + manager.unpinSlice(uri); + return slice; } @VisibleForTesting diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java index 613062fea14..67f0462c823 100644 --- a/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java @@ -77,9 +77,9 @@ public class EligibleCardCheckerTest { @Test public void isCardEligibleToDisplay_invalidScheme_returnFalse() { - final Uri sliceUri = Uri.parse("contet://com.android.settings.slices/action/flashlight"); + final Uri invalidUri = Uri.parse("contet://com.android.settings.slices/action/flashlight"); - assertThat(mEligibleCardChecker.isCardEligibleToDisplay(getContextualCard(sliceUri))) + assertThat(mEligibleCardChecker.isCardEligibleToDisplay(getContextualCard(invalidUri))) .isFalse(); } diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRendererTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRendererTest.java index 722833c8b04..06edf2baa4d 100644 --- a/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRendererTest.java +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRendererTest.java @@ -86,10 +86,10 @@ public class SliceContextualCardRendererTest { @Test public void bindView_invalidScheme_sliceShouldBeNull() { - final Uri sliceUri = Uri.parse("contet://com.android.settings.slices/action/flashlight"); + final Uri invalidUri = Uri.parse("contet://com.android.settings.slices/action/flashlight"); final RecyclerView.ViewHolder viewHolder = getSliceViewHolder(); - mRenderer.bindView(viewHolder, buildContextualCard(sliceUri)); + mRenderer.bindView(viewHolder, buildContextualCard(invalidUri)); assertThat( ((SliceFullCardRendererHelper.SliceViewHolder) viewHolder).sliceView.getSlice())