Merge changes from topic "homepage_latency"
* changes: Make slices precheck executes in parallel. Use a better way to bind slice for slice precheck.
This commit is contained in:
committed by
Android (Google) Code Review
commit
7e1bd433e6
@@ -16,20 +16,13 @@
|
||||
|
||||
package com.android.settings.homepage.contextualcards;
|
||||
|
||||
import static android.app.slice.Slice.HINT_ERROR;
|
||||
|
||||
import static androidx.slice.widget.SliceLiveData.SUPPORTED_SPECS;
|
||||
|
||||
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.ContentProviderClient;
|
||||
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;
|
||||
@@ -37,7 +30,6 @@ import android.util.Log;
|
||||
|
||||
import androidx.annotation.NonNull;
|
||||
import androidx.annotation.VisibleForTesting;
|
||||
import androidx.slice.Slice;
|
||||
|
||||
import com.android.settings.R;
|
||||
import com.android.settings.overlay.FeatureFactory;
|
||||
@@ -45,7 +37,12 @@ import com.android.settingslib.utils.AsyncLoaderCompat;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.stream.Collectors;
|
||||
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.concurrent.TimeoutException;
|
||||
|
||||
public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>> {
|
||||
|
||||
@@ -55,7 +52,9 @@ public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>
|
||||
static final long CARD_CONTENT_LOADER_TIMEOUT_MS = DateUtils.SECOND_IN_MILLIS * 3;
|
||||
|
||||
private static final String TAG = "ContextualCardLoader";
|
||||
private static final long ELIGIBILITY_CHECKER_TIMEOUT_MS = 250;
|
||||
|
||||
private final ExecutorService mExecutorService;
|
||||
private final ContentObserver mObserver = new ContentObserver(
|
||||
new Handler(Looper.getMainLooper())) {
|
||||
@Override
|
||||
@@ -71,6 +70,7 @@ public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>
|
||||
ContextualCardLoader(Context context) {
|
||||
super(context);
|
||||
mContext = context.getApplicationContext();
|
||||
mExecutorService = Executors.newCachedThreadPool();
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -169,43 +169,27 @@ public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>
|
||||
|
||||
@VisibleForTesting
|
||||
List<ContextualCard> filterEligibleCards(List<ContextualCard> candidates) {
|
||||
return candidates.stream().filter(card -> isCardEligibleToDisplay(card))
|
||||
.collect(Collectors.toList());
|
||||
}
|
||||
final List<ContextualCard> cards = new ArrayList<>();
|
||||
final List<Future<ContextualCard>> 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<ContextualCard> 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);
|
||||
}
|
||||
}
|
||||
|
||||
//check if the uri has a provider associated with.
|
||||
final ContentProviderClient provider =
|
||||
mContext.getContentResolver().acquireContentProviderClient(uri);
|
||||
if (provider == null) {
|
||||
return false;
|
||||
}
|
||||
//release contentProviderClient to prevent from memory leak.
|
||||
provider.release();
|
||||
|
||||
final Slice slice = Slice.bindSlice(mContext, uri, SUPPORTED_SPECS);
|
||||
//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;
|
||||
return cards;
|
||||
}
|
||||
|
||||
private int getNumberOfLargeCard(List<ContextualCard> cards) {
|
||||
|
||||
@@ -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<ContextualCard> {
|
||||
|
||||
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];
|
||||
}
|
||||
}
|
||||
@@ -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,25 +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_noProvider_returnFalse() {
|
||||
final String sliceUri = "content://com.android.settings.test.slices/action/flashlight";
|
||||
public void isCardEligibleToDisplay_nullSlice_returnFalse() {
|
||||
doReturn(null).when(mEligibleCardChecker).bindSlice(Uri.parse(TEST_SLICE_URI));
|
||||
|
||||
assertThat(
|
||||
mContextualCardLoader.isCardEligibleToDisplay(
|
||||
getContextualCard(sliceUri))).isFalse();
|
||||
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
|
||||
|
||||
@@ -21,9 +21,12 @@ import static com.google.common.truth.Truth.assertThat;
|
||||
import android.content.Context;
|
||||
import android.net.Uri;
|
||||
|
||||
import androidx.slice.Slice;
|
||||
import androidx.test.InstrumentationRegistry;
|
||||
import androidx.test.runner.AndroidJUnit4;
|
||||
|
||||
import com.android.settings.slices.CustomSliceRegistry;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
@@ -34,36 +37,57 @@ 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
|
||||
public void filter_twoInvalidCards_shouldReturnOneCard() {
|
||||
public void filterEligibleCards_twoInvalidCards_shouldReturnOneCard() {
|
||||
final String sliceUri1 = "content://com.android.settings.slices/action/flashlight"; //valid
|
||||
final String sliceUri2 = "content://com.android.settings.test.slices/action/flashlight";
|
||||
final String sliceUri3 = "cotent://com.android.settings.slices/action/flashlight";
|
||||
|
||||
final List<ContextualCard> 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<ContextualCard> result = mContextualCardLoader.filterEligibleCards(cards);
|
||||
|
||||
assertThat(result).hasSize(1);
|
||||
}
|
||||
|
||||
private ContextualCard getContextualCard(String sliceUri) {
|
||||
@Test
|
||||
public void bindSlice_flashlightUri_shouldReturnFlashlightSlice() {
|
||||
final Slice loadedSlice =
|
||||
mEligibleCardChecker.bindSlice(CustomSliceRegistry.FLASHLIGHT_SLICE_URI);
|
||||
|
||||
assertThat(loadedSlice.getUri()).isEqualTo(CustomSliceRegistry.FLASHLIGHT_SLICE_URI);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void bindSlice_noProvider_shouldReturnNull() {
|
||||
final String sliceUri = "content://com.android.settings.test.slices/action/flashlight";
|
||||
|
||||
final Slice loadedSlice = mEligibleCardChecker.bindSlice(Uri.parse(sliceUri));
|
||||
|
||||
assertThat(loadedSlice).isNull();
|
||||
}
|
||||
|
||||
private ContextualCard getContextualCard(Uri sliceUri) {
|
||||
return new ContextualCard.Builder()
|
||||
.setName("test_card")
|
||||
.setCardType(ContextualCard.CardType.SLICE)
|
||||
.setSliceUri(Uri.parse(sliceUri))
|
||||
.setSliceUri(sliceUri)
|
||||
.build();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user