Merge "Add logging field for direct share selection" into qt-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
fb3c3679c1
@@ -219,6 +219,8 @@ public class ChooserActivity extends ResolverActivity {
|
|||||||
private static final int SHORTCUT_MANAGER_SHARE_TARGET_RESULT_COMPLETED = 4;
|
private static final int SHORTCUT_MANAGER_SHARE_TARGET_RESULT_COMPLETED = 4;
|
||||||
private static final int LIST_VIEW_UPDATE_MESSAGE = 5;
|
private static final int LIST_VIEW_UPDATE_MESSAGE = 5;
|
||||||
|
|
||||||
|
private static final int MAX_LOG_RANK_POSITION = 12;
|
||||||
|
|
||||||
@VisibleForTesting
|
@VisibleForTesting
|
||||||
public static final int LIST_VIEW_UPDATE_INTERVAL_IN_MILLIS = 250;
|
public static final int LIST_VIEW_UPDATE_INTERVAL_IN_MILLIS = 250;
|
||||||
|
|
||||||
@@ -1015,6 +1017,7 @@ public class ChooserActivity extends ResolverActivity {
|
|||||||
// Lower values mean the ranking was better.
|
// Lower values mean the ranking was better.
|
||||||
int cat = 0;
|
int cat = 0;
|
||||||
int value = which;
|
int value = which;
|
||||||
|
int directTargetAlsoRanked = -1;
|
||||||
HashedStringCache.HashResult directTargetHashed = null;
|
HashedStringCache.HashResult directTargetHashed = null;
|
||||||
switch (mChooserListAdapter.getPositionTargetType(which)) {
|
switch (mChooserListAdapter.getPositionTargetType(which)) {
|
||||||
case ChooserListAdapter.TARGET_CALLER:
|
case ChooserListAdapter.TARGET_CALLER:
|
||||||
@@ -1034,6 +1037,7 @@ public class ChooserActivity extends ResolverActivity {
|
|||||||
target.getComponentName().getPackageName()
|
target.getComponentName().getPackageName()
|
||||||
+ target.getTitle().toString(),
|
+ target.getTitle().toString(),
|
||||||
mMaxHashSaltDays);
|
mMaxHashSaltDays);
|
||||||
|
directTargetAlsoRanked = getRankedPosition((SelectableTargetInfo) targetInfo);
|
||||||
break;
|
break;
|
||||||
case ChooserListAdapter.TARGET_STANDARD:
|
case ChooserListAdapter.TARGET_STANDARD:
|
||||||
cat = MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_STANDARD_TARGET;
|
cat = MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_STANDARD_TARGET;
|
||||||
@@ -1056,6 +1060,8 @@ public class ChooserActivity extends ResolverActivity {
|
|||||||
targetLogMaker.addTaggedData(
|
targetLogMaker.addTaggedData(
|
||||||
MetricsEvent.FIELD_HASHED_TARGET_SALT_GEN,
|
MetricsEvent.FIELD_HASHED_TARGET_SALT_GEN,
|
||||||
directTargetHashed.saltGeneration);
|
directTargetHashed.saltGeneration);
|
||||||
|
targetLogMaker.addTaggedData(MetricsEvent.FIELD_RANKED_POSITION,
|
||||||
|
directTargetAlsoRanked);
|
||||||
}
|
}
|
||||||
getMetricsLogger().write(targetLogMaker);
|
getMetricsLogger().write(targetLogMaker);
|
||||||
MetricsLogger.action(this, cat, value);
|
MetricsLogger.action(this, cat, value);
|
||||||
@@ -1074,6 +1080,21 @@ public class ChooserActivity extends ResolverActivity {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private int getRankedPosition(SelectableTargetInfo targetInfo) {
|
||||||
|
String targetPackageName =
|
||||||
|
targetInfo.getChooserTarget().getComponentName().getPackageName();
|
||||||
|
int maxRankedResults = Math.min(mChooserListAdapter.mDisplayList.size(),
|
||||||
|
MAX_LOG_RANK_POSITION);
|
||||||
|
|
||||||
|
for (int i = 0; i < maxRankedResults; i++) {
|
||||||
|
if (mChooserListAdapter.mDisplayList.get(i)
|
||||||
|
.getResolveInfo().activityInfo.packageName.equals(targetPackageName)) {
|
||||||
|
return i;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
|
||||||
void queryTargetServices(ChooserListAdapter adapter) {
|
void queryTargetServices(ChooserListAdapter adapter) {
|
||||||
final PackageManager pm = getPackageManager();
|
final PackageManager pm = getPackageManager();
|
||||||
ShortcutManager sm = (ShortcutManager) getSystemService(ShortcutManager.class);
|
ShortcutManager sm = (ShortcutManager) getSystemService(ShortcutManager.class);
|
||||||
|
|||||||
@@ -80,6 +80,17 @@ public class HashedStringCacheTest {
|
|||||||
assertThat(cachedResult2.hashedString, is(cachedResult.hashedString));
|
assertThat(cachedResult2.hashedString, is(cachedResult.hashedString));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testThatMultipleInputResultInDifferentHash() {
|
||||||
|
HashedStringCache cache = HashedStringCache.getInstance();
|
||||||
|
HashedStringCache.HashResult cachedResult =
|
||||||
|
cache.hashString(mContext, TAG, TEST_STRING, 7);
|
||||||
|
HashedStringCache.HashResult cachedResult2 =
|
||||||
|
cache.hashString(mContext, TAG, "different_test", 7);
|
||||||
|
assertThat(cachedResult2.hashedString, is(not(cachedResult.hashedString)));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testThatZeroDaysResultsInNewHash() {
|
public void testThatZeroDaysResultsInNewHash() {
|
||||||
HashedStringCache cache = HashedStringCache.getInstance();
|
HashedStringCache cache = HashedStringCache.getInstance();
|
||||||
|
|||||||
@@ -62,9 +62,6 @@ import com.android.internal.app.ResolverActivity.ResolvedComponentInfo;
|
|||||||
import com.android.internal.logging.MetricsLogger;
|
import com.android.internal.logging.MetricsLogger;
|
||||||
import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
|
import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
|
||||||
|
|
||||||
import java.util.Arrays;
|
|
||||||
import java.util.Collection;
|
|
||||||
import java.util.function.Function;
|
|
||||||
import org.junit.Before;
|
import org.junit.Before;
|
||||||
import org.junit.Rule;
|
import org.junit.Rule;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
@@ -74,7 +71,10 @@ import org.mockito.ArgumentCaptor;
|
|||||||
import org.mockito.Mockito;
|
import org.mockito.Mockito;
|
||||||
|
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
|
import java.util.Arrays;
|
||||||
|
import java.util.Collection;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
import java.util.function.Function;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Chooser activity instrumentation tests
|
* Chooser activity instrumentation tests
|
||||||
@@ -770,8 +770,6 @@ public class ChooserActivityTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// This test is too long and too slow and should not be taken as an example for future tests.
|
// This test is too long and too slow and should not be taken as an example for future tests.
|
||||||
// This is necessary because it tests that multiple calls result in the same result but
|
|
||||||
// normally a test this long should be broken into smaller tests testing individual components.
|
|
||||||
@Test
|
@Test
|
||||||
public void testDirectTargetSelectionLogging() throws InterruptedException {
|
public void testDirectTargetSelectionLogging() throws InterruptedException {
|
||||||
Intent sendIntent = createSendTextIntent();
|
Intent sendIntent = createSendTextIntent();
|
||||||
@@ -785,7 +783,7 @@ public class ChooserActivityTest {
|
|||||||
MetricsLogger mockLogger = sOverrides.metricsLogger;
|
MetricsLogger mockLogger = sOverrides.metricsLogger;
|
||||||
ArgumentCaptor<LogMaker> logMakerCaptor = ArgumentCaptor.forClass(LogMaker.class);
|
ArgumentCaptor<LogMaker> logMakerCaptor = ArgumentCaptor.forClass(LogMaker.class);
|
||||||
// Create direct share target
|
// Create direct share target
|
||||||
List<ChooserTarget> serviceTargets = createDirectShareTargets(1);
|
List<ChooserTarget> serviceTargets = createDirectShareTargets(1, "");
|
||||||
ResolveInfo ri = ResolverDataProvider.createResolveInfo(3, 0);
|
ResolveInfo ri = ResolverDataProvider.createResolveInfo(3, 0);
|
||||||
|
|
||||||
// Start activity
|
// Start activity
|
||||||
@@ -808,7 +806,7 @@ public class ChooserActivityTest {
|
|||||||
// TODO: restructure the tests b/129870719
|
// TODO: restructure the tests b/129870719
|
||||||
Thread.sleep(ChooserActivity.LIST_VIEW_UPDATE_INTERVAL_IN_MILLIS);
|
Thread.sleep(ChooserActivity.LIST_VIEW_UPDATE_INTERVAL_IN_MILLIS);
|
||||||
|
|
||||||
assertThat("Chooser should have 3 targets (2apps, 1 direct)",
|
assertThat("Chooser should have 3 targets (2 apps, 1 direct)",
|
||||||
activity.getAdapter().getCount(), is(3));
|
activity.getAdapter().getCount(), is(3));
|
||||||
assertThat("Chooser should have exactly one selectable direct target",
|
assertThat("Chooser should have exactly one selectable direct target",
|
||||||
activity.getAdapter().getSelectableServiceTargetCount(), is(1));
|
activity.getAdapter().getSelectableServiceTargetCount(), is(1));
|
||||||
@@ -832,20 +830,36 @@ public class ChooserActivityTest {
|
|||||||
.getAllValues().get(2).getTaggedData(MetricsEvent.FIELD_HASHED_TARGET_NAME);
|
.getAllValues().get(2).getTaggedData(MetricsEvent.FIELD_HASHED_TARGET_NAME);
|
||||||
assertThat("Hash is not predictable but must be obfuscated",
|
assertThat("Hash is not predictable but must be obfuscated",
|
||||||
hashedName, is(not(name)));
|
hashedName, is(not(name)));
|
||||||
|
assertThat("The packages shouldn't match for app target and direct target", logMakerCaptor
|
||||||
|
.getAllValues().get(2).getTaggedData(MetricsEvent.FIELD_RANKED_POSITION), is(-1));
|
||||||
|
}
|
||||||
|
|
||||||
// Running the same again to check if the hashed name is the same as before.
|
// This test is too long and too slow and should not be taken as an example for future tests.
|
||||||
|
@Test
|
||||||
|
public void testDirectTargetLoggingWithRankedAppTarget() throws InterruptedException {
|
||||||
|
Intent sendIntent = createSendTextIntent();
|
||||||
|
// We need app targets for direct targets to get displayed
|
||||||
|
List<ResolvedComponentInfo> resolvedComponentInfos = createResolvedComponentsForTest(2);
|
||||||
|
when(sOverrides.resolverListController.getResolversForIntent(Mockito.anyBoolean(),
|
||||||
|
Mockito.anyBoolean(),
|
||||||
|
Mockito.isA(List.class))).thenReturn(resolvedComponentInfos);
|
||||||
|
|
||||||
Intent sendIntent2 = createSendTextIntent();
|
// Set up resources
|
||||||
|
MetricsLogger mockLogger = sOverrides.metricsLogger;
|
||||||
|
ArgumentCaptor<LogMaker> logMakerCaptor = ArgumentCaptor.forClass(LogMaker.class);
|
||||||
|
// Create direct share target
|
||||||
|
List<ChooserTarget> serviceTargets = createDirectShareTargets(1,
|
||||||
|
resolvedComponentInfos.get(0).getResolveInfoAt(0).activityInfo.packageName);
|
||||||
|
ResolveInfo ri = ResolverDataProvider.createResolveInfo(3, 0);
|
||||||
|
|
||||||
// Start activity
|
// Start activity
|
||||||
final ChooserWrapperActivity activity2 = mActivityRule
|
final ChooserWrapperActivity activity = mActivityRule
|
||||||
.launchActivity(Intent.createChooser(sendIntent2, null));
|
.launchActivity(Intent.createChooser(sendIntent, null));
|
||||||
waitForIdle();
|
|
||||||
|
|
||||||
// Insert the direct share target
|
// Insert the direct share target
|
||||||
InstrumentationRegistry.getInstrumentation().runOnMainSync(
|
InstrumentationRegistry.getInstrumentation().runOnMainSync(
|
||||||
() -> activity2.getAdapter().addServiceResults(
|
() -> activity.getAdapter().addServiceResults(
|
||||||
activity2.createTestDisplayResolveInfo(sendIntent,
|
activity.createTestDisplayResolveInfo(sendIntent,
|
||||||
ri,
|
ri,
|
||||||
"testLabel",
|
"testLabel",
|
||||||
"testInfo",
|
"testInfo",
|
||||||
@@ -858,29 +872,89 @@ public class ChooserActivityTest {
|
|||||||
// TODO: restructure the tests b/129870719
|
// TODO: restructure the tests b/129870719
|
||||||
Thread.sleep(ChooserActivity.LIST_VIEW_UPDATE_INTERVAL_IN_MILLIS);
|
Thread.sleep(ChooserActivity.LIST_VIEW_UPDATE_INTERVAL_IN_MILLIS);
|
||||||
|
|
||||||
assertThat("Chooser should have 3 targets (2apps, 1 direct)",
|
assertThat("Chooser should have 3 targets (2 apps, 1 direct)",
|
||||||
activity2.getAdapter().getCount(), is(3));
|
activity.getAdapter().getCount(), is(3));
|
||||||
assertThat("Chooser should have exactly one selectable direct target",
|
assertThat("Chooser should have exactly one selectable direct target",
|
||||||
activity2.getAdapter().getSelectableServiceTargetCount(), is(1));
|
activity.getAdapter().getSelectableServiceTargetCount(), is(1));
|
||||||
assertThat("The resolver info must match the resolver info used to create the target",
|
assertThat("The resolver info must match the resolver info used to create the target",
|
||||||
activity2.getAdapter().getItem(0).getResolveInfo(), is(ri));
|
activity.getAdapter().getItem(0).getResolveInfo(), is(ri));
|
||||||
|
|
||||||
// Click on the direct target
|
// Click on the direct target
|
||||||
|
String name = serviceTargets.get(0).getTitle().toString();
|
||||||
onView(withText(name))
|
onView(withText(name))
|
||||||
.perform(click());
|
.perform(click());
|
||||||
waitForIdle();
|
waitForIdle();
|
||||||
|
|
||||||
// Currently we're seeing 6 invocations (3 from above, doubled up)
|
// Currently we're seeing 3 invocations
|
||||||
// 4. ChooserActivity.onCreate()
|
// 1. ChooserActivity.onCreate()
|
||||||
// 5. ChooserActivity$ChooserRowAdapter.createContentPreviewView()
|
// 2. ChooserActivity$ChooserRowAdapter.createContentPreviewView()
|
||||||
// 6. ChooserActivity.startSelected -- which is the one we're after
|
// 3. ChooserActivity.startSelected -- which is the one we're after
|
||||||
verify(mockLogger, Mockito.times(6)).write(logMakerCaptor.capture());
|
verify(mockLogger, Mockito.times(3)).write(logMakerCaptor.capture());
|
||||||
assertThat(logMakerCaptor.getAllValues().get(5).getCategory(),
|
assertThat(logMakerCaptor.getAllValues().get(2).getCategory(),
|
||||||
is(MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET));
|
is(MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET));
|
||||||
String hashedName2 = (String) logMakerCaptor
|
assertThat("The packages should match for app target and direct target", logMakerCaptor
|
||||||
.getAllValues().get(5).getTaggedData(MetricsEvent.FIELD_HASHED_TARGET_NAME);
|
.getAllValues().get(2).getTaggedData(MetricsEvent.FIELD_RANKED_POSITION), is(0));
|
||||||
assertThat("Hashing the same name should result in the same hashed value",
|
}
|
||||||
hashedName2, is(hashedName));
|
|
||||||
|
// This test is too long and too slow and should not be taken as an example for future tests.
|
||||||
|
@Test
|
||||||
|
public void testDirectTargetLoggingWithAppTargetNotRanked() throws InterruptedException {
|
||||||
|
Intent sendIntent = createSendTextIntent();
|
||||||
|
// We need app targets for direct targets to get displayed
|
||||||
|
List<ResolvedComponentInfo> resolvedComponentInfos = createResolvedComponentsForTest(15);
|
||||||
|
when(sOverrides.resolverListController.getResolversForIntent(Mockito.anyBoolean(),
|
||||||
|
Mockito.anyBoolean(),
|
||||||
|
Mockito.isA(List.class))).thenReturn(resolvedComponentInfos);
|
||||||
|
|
||||||
|
// Set up resources
|
||||||
|
MetricsLogger mockLogger = sOverrides.metricsLogger;
|
||||||
|
ArgumentCaptor<LogMaker> logMakerCaptor = ArgumentCaptor.forClass(LogMaker.class);
|
||||||
|
// Create direct share target
|
||||||
|
List<ChooserTarget> serviceTargets = createDirectShareTargets(1,
|
||||||
|
resolvedComponentInfos.get(14).getResolveInfoAt(0).activityInfo.packageName);
|
||||||
|
ResolveInfo ri = ResolverDataProvider.createResolveInfo(16, 0);
|
||||||
|
|
||||||
|
// Start activity
|
||||||
|
final ChooserWrapperActivity activity = mActivityRule
|
||||||
|
.launchActivity(Intent.createChooser(sendIntent, null));
|
||||||
|
// Insert the direct share target
|
||||||
|
InstrumentationRegistry.getInstrumentation().runOnMainSync(
|
||||||
|
() -> activity.getAdapter().addServiceResults(
|
||||||
|
activity.createTestDisplayResolveInfo(sendIntent,
|
||||||
|
ri,
|
||||||
|
"testLabel",
|
||||||
|
"testInfo",
|
||||||
|
sendIntent),
|
||||||
|
serviceTargets,
|
||||||
|
false)
|
||||||
|
);
|
||||||
|
// Thread.sleep shouldn't be a thing in an integration test but it's
|
||||||
|
// necessary here because of the way the code is structured
|
||||||
|
// TODO: restructure the tests b/129870719
|
||||||
|
Thread.sleep(ChooserActivity.LIST_VIEW_UPDATE_INTERVAL_IN_MILLIS);
|
||||||
|
|
||||||
|
assertThat("Chooser should have 20 targets (4 apps, 1 direct, 15 A-Z)",
|
||||||
|
activity.getAdapter().getCount(), is(20));
|
||||||
|
assertThat("Chooser should have exactly one selectable direct target",
|
||||||
|
activity.getAdapter().getSelectableServiceTargetCount(), is(1));
|
||||||
|
assertThat("The resolver info must match the resolver info used to create the target",
|
||||||
|
activity.getAdapter().getItem(0).getResolveInfo(), is(ri));
|
||||||
|
|
||||||
|
// Click on the direct target
|
||||||
|
String name = serviceTargets.get(0).getTitle().toString();
|
||||||
|
onView(withText(name))
|
||||||
|
.perform(click());
|
||||||
|
waitForIdle();
|
||||||
|
|
||||||
|
// Currently we're seeing 3 invocations
|
||||||
|
// 1. ChooserActivity.onCreate()
|
||||||
|
// 2. ChooserActivity$ChooserRowAdapter.createContentPreviewView()
|
||||||
|
// 3. ChooserActivity.startSelected -- which is the one we're after
|
||||||
|
verify(mockLogger, Mockito.times(3)).write(logMakerCaptor.capture());
|
||||||
|
assertThat(logMakerCaptor.getAllValues().get(2).getCategory(),
|
||||||
|
is(MetricsEvent.ACTION_ACTIVITY_CHOOSER_PICKED_SERVICE_TARGET));
|
||||||
|
assertThat("The packages shouldn't match for app target and direct target", logMakerCaptor
|
||||||
|
.getAllValues().get(2).getTaggedData(MetricsEvent.FIELD_RANKED_POSITION), is(-1));
|
||||||
}
|
}
|
||||||
|
|
||||||
private Intent createSendTextIntent() {
|
private Intent createSendTextIntent() {
|
||||||
@@ -946,12 +1020,17 @@ public class ChooserActivityTest {
|
|||||||
return infoList;
|
return infoList;
|
||||||
}
|
}
|
||||||
|
|
||||||
private List<ChooserTarget> createDirectShareTargets(int numberOfResults) {
|
private List<ChooserTarget> createDirectShareTargets(int numberOfResults, String packageName) {
|
||||||
Icon icon = Icon.createWithBitmap(createBitmap());
|
Icon icon = Icon.createWithBitmap(createBitmap());
|
||||||
String testTitle = "testTitle";
|
String testTitle = "testTitle";
|
||||||
List<ChooserTarget> targets = new ArrayList<>();
|
List<ChooserTarget> targets = new ArrayList<>();
|
||||||
for (int i = 0; i < numberOfResults; i++) {
|
for (int i = 0; i < numberOfResults; i++) {
|
||||||
ComponentName componentName = ResolverDataProvider.createComponentName(i);
|
ComponentName componentName;
|
||||||
|
if (packageName.isEmpty()) {
|
||||||
|
componentName = ResolverDataProvider.createComponentName(i);
|
||||||
|
} else {
|
||||||
|
componentName = new ComponentName(packageName, packageName + ".class");
|
||||||
|
}
|
||||||
ChooserTarget tempTarget = new ChooserTarget(
|
ChooserTarget tempTarget = new ChooserTarget(
|
||||||
testTitle + i,
|
testTitle + i,
|
||||||
icon,
|
icon,
|
||||||
|
|||||||
Reference in New Issue
Block a user