diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java b/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java index 1acddabcec0..b9b2a1c784d 100644 --- a/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java +++ b/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java @@ -16,17 +16,13 @@ package com.android.settings.homepage.contextualcards; -import static android.app.slice.Slice.HINT_ERROR; - import static com.android.settings.slices.CustomSliceRegistry.BLUETOOTH_DEVICES_SLICE_URI; import static com.android.settings.slices.CustomSliceRegistry.CONTEXTUAL_WIFI_SLICE_URI; import static com.android.settings.slices.CustomSliceRegistry.NOTIFICATION_CHANNEL_SLICE_URI; -import android.content.ContentResolver; import android.content.Context; import android.database.ContentObserver; import android.database.Cursor; -import android.net.Uri; import android.os.Handler; import android.os.Looper; import android.text.format.DateUtils; @@ -34,8 +30,6 @@ import android.util.Log; import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; -import androidx.slice.Slice; -import androidx.slice.SliceViewManager; import com.android.settings.R; import com.android.settings.overlay.FeatureFactory; @@ -43,9 +37,12 @@ import com.android.settingslib.utils.AsyncLoaderCompat; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; +import java.util.concurrent.TimeoutException; public class ContextualCardLoader extends AsyncLoaderCompat> { @@ -55,8 +52,9 @@ public class ContextualCardLoader extends AsyncLoaderCompat static final long CARD_CONTENT_LOADER_TIMEOUT_MS = DateUtils.SECOND_IN_MILLIS * 3; private static final String TAG = "ContextualCardLoader"; - private static final long LATCH_TIMEOUT_MS = 200; + private static final long ELIGIBILITY_CHECKER_TIMEOUT_MS = 250; + private final ExecutorService mExecutorService; private final ContentObserver mObserver = new ContentObserver( new Handler(Looper.getMainLooper())) { @Override @@ -72,6 +70,7 @@ public class ContextualCardLoader extends AsyncLoaderCompat ContextualCardLoader(Context context) { super(context); mContext = context.getApplicationContext(); + mExecutorService = Executors.newCachedThreadPool(); } @Override @@ -170,68 +169,27 @@ public class ContextualCardLoader extends AsyncLoaderCompat @VisibleForTesting List filterEligibleCards(List candidates) { - return candidates.stream().filter(card -> isCardEligibleToDisplay(card)) - .collect(Collectors.toList()); - } + final List cards = new ArrayList<>(); + final List> eligibleCards = new ArrayList<>(); - @VisibleForTesting - boolean isCardEligibleToDisplay(ContextualCard card) { - final long startTime = System.currentTimeMillis(); - if (card.isCustomCard()) { - return true; + for (ContextualCard card : candidates) { + final EligibleCardChecker future = new EligibleCardChecker(mContext, card); + eligibleCards.add(mExecutorService.submit(future)); } - - final Uri uri = card.getSliceUri(); - - if (!ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) { - return false; + // Collect future and eligible cards + for (Future cardFuture : eligibleCards) { + try { + //TODO(b/124492762): Log latency and timeout occurrence. + final ContextualCard card = cardFuture.get(ELIGIBILITY_CHECKER_TIMEOUT_MS, + TimeUnit.MILLISECONDS); + if (card != null) { + cards.add(card); + } + } catch (ExecutionException | InterruptedException | TimeoutException e) { + Log.w(TAG, "Failed to get eligible state for card, likely timeout. Skipping", e); + } } - - final Slice slice = bindSlice(uri); - //TODO(b/123668403): remove the log here once we do the change with FutureTask - final long bindTime = System.currentTimeMillis() - startTime; - Log.d(TAG, "Binding time for " + uri + " = " + bindTime); - - if (slice == null || slice.hasHint(HINT_ERROR)) { - Log.w(TAG, "Failed to bind slice, not eligible for display " + uri); - return false; - } - - return true; - } - - @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]; + return cards; } private int getNumberOfLargeCard(List cards) { diff --git a/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java b/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java new file mode 100644 index 00000000000..5423ce3ae0c --- /dev/null +++ b/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.homepage.contextualcards; + +import static android.app.slice.Slice.HINT_ERROR; + +import android.content.ContentResolver; +import android.content.Context; +import android.net.Uri; +import android.util.Log; + +import androidx.annotation.VisibleForTesting; +import androidx.slice.Slice; +import androidx.slice.SliceViewManager; + +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 = 200; + + private final Context mContext; + private final ContextualCard mCard; + + EligibleCardChecker(Context context, ContextualCard card) { + mContext = context; + mCard = card; + } + + @Override + public ContextualCard call() throws Exception { + return isCardEligibleToDisplay(mCard) ? mCard : null; + } + + @VisibleForTesting + boolean isCardEligibleToDisplay(ContextualCard card) { + if (card.isCustomCard()) { + return true; + } + + final Uri uri = card.getSliceUri(); + if (!ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) { + return false; + } + + final Slice slice = bindSlice(uri); + if (slice == null || slice.hasHint(HINT_ERROR)) { + Log.w(TAG, "Failed to bind slice, not eligible for display " + uri); + return false; + } + return true; + } + + @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]; + } +} diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java index beb2d407c62..18aa1c4720b 100644 --- a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java @@ -16,6 +16,8 @@ package com.android.settings.homepage.contextualcards; +import static android.app.slice.Slice.HINT_ERROR; + import static com.android.settings.homepage.contextualcards.ContextualCardLoader.DEFAULT_CARD_COUNT; import static com.google.common.truth.Truth.assertThat; @@ -27,6 +29,8 @@ import static org.mockito.Mockito.spy; import android.content.Context; import android.net.Uri; +import androidx.slice.Slice; + import com.android.settings.R; import com.android.settings.slices.CustomSliceRegistry; @@ -43,13 +47,18 @@ import java.util.stream.Collectors; @RunWith(RobolectricTestRunner.class) public class ContextualCardLoaderTest { + private static final String TEST_SLICE_URI = "content://test/test"; + private Context mContext; private ContextualCardLoader mContextualCardLoader; + private EligibleCardChecker mEligibleCardChecker; @Before public void setUp() { mContext = RuntimeEnvironment.application; mContextualCardLoader = spy(new ContextualCardLoader(mContext)); + mEligibleCardChecker = + spy(new EligibleCardChecker(mContext, getContextualCard(TEST_SLICE_URI))); } @Test @@ -61,16 +70,33 @@ public class ContextualCardLoaderTest { .setSummaryText("custom_summary") .build(); - assertThat(mContextualCardLoader.isCardEligibleToDisplay(customCard)).isTrue(); + assertThat(mEligibleCardChecker.isCardEligibleToDisplay(customCard)).isTrue(); } @Test public void isCardEligibleToDisplay_invalidScheme_returnFalse() { final String sliceUri = "contet://com.android.settings.slices/action/flashlight"; - assertThat( - mContextualCardLoader.isCardEligibleToDisplay( - getContextualCard(sliceUri))).isFalse(); + assertThat(mEligibleCardChecker.isCardEligibleToDisplay(getContextualCard(sliceUri))) + .isFalse(); + } + + @Test + public void isCardEligibleToDisplay_nullSlice_returnFalse() { + doReturn(null).when(mEligibleCardChecker).bindSlice(Uri.parse(TEST_SLICE_URI)); + + assertThat(mEligibleCardChecker.isCardEligibleToDisplay(getContextualCard(TEST_SLICE_URI))) + .isFalse(); + } + + @Test + public void isCardEligibleToDisplay_errorSlice_returnFalse() { + final Slice slice = new Slice.Builder(Uri.parse(TEST_SLICE_URI)) + .addHints(HINT_ERROR).build(); + doReturn(slice).when(mEligibleCardChecker).bindSlice(Uri.parse(TEST_SLICE_URI)); + + assertThat(mEligibleCardChecker.isCardEligibleToDisplay(getContextualCard(TEST_SLICE_URI))) + .isFalse(); } @Test diff --git a/tests/unit/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java b/tests/unit/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java index 55ad1ff6eb7..447e2b45e7b 100644 --- a/tests/unit/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java +++ b/tests/unit/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java @@ -37,13 +37,17 @@ import java.util.List; @RunWith(AndroidJUnit4.class) public class ContextualCardLoaderTest { + private static final Uri TEST_URI = Uri.parse("content://test/test"); + private Context mContext; private ContextualCardLoader mContextualCardLoader; + private EligibleCardChecker mEligibleCardChecker; @Before public void setUp() { mContext = InstrumentationRegistry.getTargetContext(); mContextualCardLoader = new ContextualCardLoader(mContext); + mEligibleCardChecker = new EligibleCardChecker(mContext, getContextualCard(TEST_URI)); } @Test @@ -53,9 +57,9 @@ public class ContextualCardLoaderTest { final String sliceUri3 = "cotent://com.android.settings.slices/action/flashlight"; final List cards = new ArrayList<>(); - cards.add(getContextualCard(sliceUri1)); - cards.add(getContextualCard(sliceUri2)); - cards.add(getContextualCard(sliceUri3)); + cards.add(getContextualCard(Uri.parse(sliceUri1))); + cards.add(getContextualCard(Uri.parse(sliceUri2))); + cards.add(getContextualCard(Uri.parse(sliceUri3))); final List result = mContextualCardLoader.filterEligibleCards(cards); @@ -65,7 +69,7 @@ public class ContextualCardLoaderTest { @Test public void bindSlice_flashlightUri_shouldReturnFlashlightSlice() { final Slice loadedSlice = - mContextualCardLoader.bindSlice(CustomSliceRegistry.FLASHLIGHT_SLICE_URI); + mEligibleCardChecker.bindSlice(CustomSliceRegistry.FLASHLIGHT_SLICE_URI); assertThat(loadedSlice.getUri()).isEqualTo(CustomSliceRegistry.FLASHLIGHT_SLICE_URI); } @@ -74,16 +78,16 @@ public class ContextualCardLoaderTest { public void bindSlice_noProvider_shouldReturnNull() { final String sliceUri = "content://com.android.settings.test.slices/action/flashlight"; - final Slice loadedSlice = mContextualCardLoader.bindSlice(Uri.parse(sliceUri)); + final Slice loadedSlice = mEligibleCardChecker.bindSlice(Uri.parse(sliceUri)); assertThat(loadedSlice).isNull(); } - private ContextualCard getContextualCard(String sliceUri) { + private ContextualCard getContextualCard(Uri sliceUri) { return new ContextualCard.Builder() .setName("test_card") .setCardType(ContextualCard.CardType.SLICE) - .setSliceUri(Uri.parse(sliceUri)) + .setSliceUri(sliceUri) .build(); } }