Add exp flag for min number of sys gen smart replies in notifications

For system (assistant) generated smart replies we want to ensure the
replies don't seem biased (see example [1]). Therefore we here add an
experiment flag that determines the minimum number of replies N to show
in a notification. If we can't fit N replies into the notification we
remove all replies from that notification (i.e. we show zero replies).

[1] Example of biased replies:
Message: "Hi, how are you?"
Replies: "Good", "Great!", "Bad"
using only one reply here ("Good") might be seen as biased.

Bug: 120779592
Test: atest SystemUITests
Test: call "adb shell settings put global
smart_replies_in_notifications_flags enabled=true,min_num_replies=X" for
different values of X and ensure the behaviour is correct.
Change-Id: I898850f7895d0fd43ec6788095162f3138cd62cb
This commit is contained in:
Gustav Sennton
2019-01-08 11:20:49 +00:00
parent 96583d40d1
commit a31f6aede1
6 changed files with 233 additions and 34 deletions

View File

@@ -13883,11 +13883,12 @@ public final class Settings {
* The following keys are supported:
*
* <pre>
* enabled (boolean)
* requires_targeting_p (boolean)
* max_squeeze_remeasure_attempts (int)
* edit_choices_before_sending (boolean)
* show_in_heads_up (boolean)
* enabled (boolean)
* requires_targeting_p (boolean)
* max_squeeze_remeasure_attempts (int)
* edit_choices_before_sending (boolean)
* show_in_heads_up (boolean)
* min_num_system_generated_replies (int)
* </pre>
* @see com.android.systemui.statusbar.policy.SmartReplyConstants
* @hide

View File

@@ -458,6 +458,11 @@
heads-up notifications. -->
<bool name="config_smart_replies_in_notifications_show_in_heads_up">true</bool>
<!-- Smart replies in notifications: Minimum number of system generated smart replies that
should be shown in a notification. If we cannot show at least this many replies we instead
show none. -->
<integer name="config_smart_replies_in_notifications_min_num_system_generated_replies">0</integer>
<!-- Screenshot editing default activity. Must handle ACTION_EDIT image/png intents.
Blank sends the user to the Chooser first.
This name is in the ComponentName flattened format (package/class) -->

View File

@@ -46,18 +46,21 @@ public final class SmartReplyConstants extends ContentObserver {
private static final String KEY_EDIT_CHOICES_BEFORE_SENDING =
"edit_choices_before_sending";
private static final String KEY_SHOW_IN_HEADS_UP = "show_in_heads_up";
private static final String KEY_MIN_NUM_REPLIES = "min_num_system_generated_replies";
private final boolean mDefaultEnabled;
private final boolean mDefaultRequiresP;
private final int mDefaultMaxSqueezeRemeasureAttempts;
private final boolean mDefaultEditChoicesBeforeSending;
private final boolean mDefaultShowInHeadsUp;
private final int mDefaultMinNumSystemGeneratedReplies;
private boolean mEnabled;
private boolean mRequiresTargetingP;
private int mMaxSqueezeRemeasureAttempts;
private boolean mEditChoicesBeforeSending;
private boolean mShowInHeadsUp;
private int mMinNumSystemGeneratedReplies;
private final Context mContext;
private final KeyValueListParser mParser = new KeyValueListParser(',');
@@ -78,6 +81,8 @@ public final class SmartReplyConstants extends ContentObserver {
R.bool.config_smart_replies_in_notifications_edit_choices_before_sending);
mDefaultShowInHeadsUp = resources.getBoolean(
R.bool.config_smart_replies_in_notifications_show_in_heads_up);
mDefaultMinNumSystemGeneratedReplies = resources.getInteger(
R.integer.config_smart_replies_in_notifications_min_num_system_generated_replies);
mContext.getContentResolver().registerContentObserver(
Settings.Global.getUriFor(Settings.Global.SMART_REPLIES_IN_NOTIFICATIONS_FLAGS),
@@ -105,6 +110,8 @@ public final class SmartReplyConstants extends ContentObserver {
mEditChoicesBeforeSending = mParser.getBoolean(
KEY_EDIT_CHOICES_BEFORE_SENDING, mDefaultEditChoicesBeforeSending);
mShowInHeadsUp = mParser.getBoolean(KEY_SHOW_IN_HEADS_UP, mDefaultShowInHeadsUp);
mMinNumSystemGeneratedReplies =
mParser.getInt(KEY_MIN_NUM_REPLIES, mDefaultMinNumSystemGeneratedReplies);
}
}
@@ -155,4 +162,12 @@ public final class SmartReplyConstants extends ContentObserver {
public boolean getShowInHeadsUp() {
return mShowInHeadsUp;
}
/**
* Returns the minimum number of system generated replies to show in a notification.
* If we cannot show at least this many system generated replies we should show none.
*/
public int getMinNumSystemGeneratedReplies() {
return mMinNumSystemGeneratedReplies;
}
}

View File

@@ -88,6 +88,12 @@ public class SmartReplyView extends ViewGroup {
private View mSmartReplyContainer;
/**
* Whether the smart replies in this view were generated by the notification assistant. If not
* they're provided by the app.
*/
private boolean mSmartRepliesGeneratedByAssistant = false;
@ColorInt
private int mCurrentBackgroundColor;
@ColorInt
@@ -202,6 +208,7 @@ public class SmartReplyView extends ViewGroup {
getContext(), this, i, smartReplies, smartReplyController, entry);
addView(replyButton);
}
this.mSmartRepliesGeneratedByAssistant = smartReplies.fromAssistant;
}
}
reallocateCandidateButtonQueueForSqueezing();
@@ -344,10 +351,11 @@ public class SmartReplyView extends ViewGroup {
mCandidateButtonQueueForSqueezing.clear();
}
int measuredWidth = mPaddingLeft + mPaddingRight;
int maxChildHeight = 0;
SmartSuggestionMeasures accumulatedMeasures = new SmartSuggestionMeasures(
mPaddingLeft + mPaddingRight,
0 /* maxChildHeight */,
mSingleLineButtonPaddingHorizontal);
int displayedChildCount = 0;
int buttonPaddingHorizontal = mSingleLineButtonPaddingHorizontal;
// Set up a list of suggestions where actions come before replies. Note that the Buttons
// themselves have already been added to the view hierarchy in an order such that Smart
@@ -360,11 +368,15 @@ public class SmartReplyView extends ViewGroup {
smartSuggestions.addAll(smartReplies);
List<View> coveredSuggestions = new ArrayList<>();
// SmartSuggestionMeasures for all action buttons, this will be filled in when the first
// reply button is added.
SmartSuggestionMeasures actionsMeasures = null;
for (View child : smartSuggestions) {
final LayoutParams lp = (LayoutParams) child.getLayoutParams();
child.setPadding(buttonPaddingHorizontal, child.getPaddingTop(),
buttonPaddingHorizontal, child.getPaddingBottom());
child.setPadding(accumulatedMeasures.mButtonPaddingHorizontal, child.getPaddingTop(),
accumulatedMeasures.mButtonPaddingHorizontal, child.getPaddingBottom());
child.measure(MEASURE_SPEC_ANY_LENGTH, heightMeasureSpec);
coveredSuggestions.add(child);
@@ -380,45 +392,52 @@ public class SmartReplyView extends ViewGroup {
}
// Remember the current measurements in case the current button doesn't fit in.
final int originalMaxChildHeight = maxChildHeight;
final int originalMeasuredWidth = measuredWidth;
final int originalButtonPaddingHorizontal = buttonPaddingHorizontal;
SmartSuggestionMeasures originalMeasures = accumulatedMeasures.clone();
if (actionsMeasures == null && lp.buttonType == SmartButtonType.REPLY) {
// We've added all actions (we go through actions first), now add their
// measurements.
actionsMeasures = accumulatedMeasures.clone();
}
final int spacing = displayedChildCount == 0 ? 0 : mSpacing;
final int childWidth = child.getMeasuredWidth();
final int childHeight = child.getMeasuredHeight();
measuredWidth += spacing + childWidth;
maxChildHeight = Math.max(maxChildHeight, childHeight);
accumulatedMeasures.mMeasuredWidth += spacing + childWidth;
accumulatedMeasures.mMaxChildHeight =
Math.max(accumulatedMeasures.mMaxChildHeight, childHeight);
// Do we need to increase the number of lines in smart reply buttons to two?
final boolean increaseToTwoLines =
buttonPaddingHorizontal == mSingleLineButtonPaddingHorizontal
&& (lineCount == 2 || measuredWidth > targetWidth);
(accumulatedMeasures.mButtonPaddingHorizontal
== mSingleLineButtonPaddingHorizontal)
&& (lineCount == 2 || accumulatedMeasures.mMeasuredWidth > targetWidth);
if (increaseToTwoLines) {
measuredWidth += (displayedChildCount + 1) * mSingleToDoubleLineButtonWidthIncrease;
buttonPaddingHorizontal = mDoubleLineButtonPaddingHorizontal;
accumulatedMeasures.mMeasuredWidth +=
(displayedChildCount + 1) * mSingleToDoubleLineButtonWidthIncrease;
accumulatedMeasures.mButtonPaddingHorizontal =
mDoubleLineButtonPaddingHorizontal;
}
// If the last button doesn't fit into the remaining width, try squeezing preceding
// smart reply buttons.
if (measuredWidth > targetWidth) {
if (accumulatedMeasures.mMeasuredWidth > targetWidth) {
// Keep squeezing preceding and current smart reply buttons until they all fit.
while (measuredWidth > targetWidth
while (accumulatedMeasures.mMeasuredWidth > targetWidth
&& !mCandidateButtonQueueForSqueezing.isEmpty()) {
final Button candidate = mCandidateButtonQueueForSqueezing.poll();
final int squeezeReduction = squeezeButton(candidate, heightMeasureSpec);
if (squeezeReduction != SQUEEZE_FAILED) {
maxChildHeight = Math.max(maxChildHeight, candidate.getMeasuredHeight());
measuredWidth -= squeezeReduction;
accumulatedMeasures.mMaxChildHeight =
Math.max(accumulatedMeasures.mMaxChildHeight,
candidate.getMeasuredHeight());
accumulatedMeasures.mMeasuredWidth -= squeezeReduction;
}
}
// If the current button still doesn't fit after squeezing all buttons, undo the
// last squeezing round.
if (measuredWidth > targetWidth) {
measuredWidth = originalMeasuredWidth;
maxChildHeight = originalMaxChildHeight;
buttonPaddingHorizontal = originalButtonPaddingHorizontal;
if (accumulatedMeasures.mMeasuredWidth > targetWidth) {
accumulatedMeasures = originalMeasures;
// Mark all buttons from the last squeezing round as "failed to squeeze", so
// that they're re-measured without squeezing later.
@@ -440,16 +459,75 @@ public class SmartReplyView extends ViewGroup {
displayedChildCount++;
}
if (mSmartRepliesGeneratedByAssistant) {
if (!gotEnoughSmartReplies(smartReplies)) {
// We don't have enough smart replies - hide all of them.
for (View smartReplyButton : smartReplies) {
final LayoutParams lp = (LayoutParams) smartReplyButton.getLayoutParams();
lp.show = false;
}
// Reset our measures back to when we had only added actions (before adding
// replies).
accumulatedMeasures = actionsMeasures;
}
}
// We're done squeezing buttons, so we can clear the priority queue.
mCandidateButtonQueueForSqueezing.clear();
// Finally, we need to re-measure some buttons.
remeasureButtonsIfNecessary(buttonPaddingHorizontal, maxChildHeight);
remeasureButtonsIfNecessary(accumulatedMeasures.mButtonPaddingHorizontal,
accumulatedMeasures.mMaxChildHeight);
setMeasuredDimension(
resolveSize(Math.max(getSuggestedMinimumWidth(), measuredWidth), widthMeasureSpec),
resolveSize(Math.max(getSuggestedMinimumHeight(),
mPaddingTop + maxChildHeight + mPaddingBottom), heightMeasureSpec));
resolveSize(Math.max(getSuggestedMinimumWidth(),
accumulatedMeasures.mMeasuredWidth),
widthMeasureSpec),
resolveSize(Math.max(getSuggestedMinimumHeight(), mPaddingTop
+ accumulatedMeasures.mMaxChildHeight + mPaddingBottom),
heightMeasureSpec));
}
/**
* Fields we keep track of inside onMeasure() to correctly measure the SmartReplyView depending
* on which suggestions are added.
*/
private static class SmartSuggestionMeasures {
int mMeasuredWidth = -1;
int mMaxChildHeight = -1;
int mButtonPaddingHorizontal = -1;
SmartSuggestionMeasures(int measuredWidth, int maxChildHeight,
int buttonPaddingHorizontal) {
this.mMeasuredWidth = measuredWidth;
this.mMaxChildHeight = maxChildHeight;
this.mButtonPaddingHorizontal = buttonPaddingHorizontal;
}
public SmartSuggestionMeasures clone() {
return new SmartSuggestionMeasures(
mMeasuredWidth, mMaxChildHeight, mButtonPaddingHorizontal);
}
}
/**
* Returns whether our notification contains at least N smart replies (or 0) where N is
* determined by {@link SmartReplyConstants}.
*/
private boolean gotEnoughSmartReplies(List<View> smartReplies) {
int numShownReplies = 0;
for (View smartReplyButton : smartReplies) {
final LayoutParams lp = (LayoutParams) smartReplyButton.getLayoutParams();
if (lp.show) {
numShownReplies++;
}
}
if (numShownReplies == 0
|| numShownReplies >= mConstants.getMinNumSystemGeneratedReplies()) {
// We have enough replies, yay!
return true;
}
return false;
}
private List<View> filterActionsOrReplies(SmartButtonType buttonType) {

View File

@@ -55,6 +55,9 @@ public class SmartReplyConstantsTest extends SysuiTestCase {
resources.addOverride(
R.bool.config_smart_replies_in_notifications_edit_choices_before_sending, false);
resources.addOverride(R.bool.config_smart_replies_in_notifications_show_in_heads_up, true);
resources.addOverride(
R.integer.config_smart_replies_in_notifications_min_num_system_generated_replies,
2);
mConstants = new SmartReplyConstants(Handler.createAsync(Looper.myLooper()), mContext);
}
@@ -178,6 +181,19 @@ public class SmartReplyConstantsTest extends SysuiTestCase {
Settings.Global.SMART_REPLIES_IN_NOTIFICATIONS_FLAGS, flags);
}
@Test
public void testGetMinNumSystemGeneratedRepliesWithNoConfig() {
assertTrue(mConstants.isEnabled());
assertEquals(2, mConstants.getMinNumSystemGeneratedReplies());
}
@Test
public void testGetMinNumSystemGeneratedRepliesWithValidConfig() {
overrideSetting("enabled=true,min_num_system_generated_replies=5");
triggerConstantsOnChange();
assertEquals(5, mConstants.getMinNumSystemGeneratedReplies());
}
private void triggerConstantsOnChange() {
// Since Settings.Global is mocked in TestableContext, we need to manually trigger the
// content observer.

View File

@@ -96,6 +96,7 @@ public class SmartReplyViewTest extends SysuiTestCase {
@Mock private SmartReplyController mLogger;
private NotificationEntry mEntry;
private Notification mNotification;
@Mock private SmartReplyConstants mConstants;
@Mock ActivityStarter mActivityStarter;
@Mock HeadsUpManager mHeadsUpManager;
@@ -108,10 +109,14 @@ public class SmartReplyViewTest extends SysuiTestCase {
mDependency.get(KeyguardDismissUtil.class).setDismissHandler(action -> action.onDismiss());
mDependency.injectMockDependency(ShadeController.class);
mDependency.injectTestDependency(ActivityStarter.class, mActivityStarter);
mDependency.injectTestDependency(SmartReplyConstants.class, mConstants);
mContainer = new View(mContext, null);
mView = SmartReplyView.inflate(mContext, null);
// Any number of replies are fine.
when(mConstants.getMinNumSystemGeneratedReplies()).thenReturn(0);
when(mConstants.getMaxSqueezeRemeasureAttempts()).thenReturn(3);
final Resources res = mContext.getResources();
mSingleLinePaddingHorizontal = res.getDimensionPixelSize(
@@ -403,7 +408,7 @@ public class SmartReplyViewTest extends SysuiTestCase {
}
private void setSmartReplies(CharSequence[] choices) {
setSmartReplies(choices, false);
setSmartReplies(choices, false /* fromAssistant */);
}
private void setSmartReplies(CharSequence[] choices, boolean fromAssistant) {
@@ -440,9 +445,14 @@ public class SmartReplyViewTest extends SysuiTestCase {
}
private void setSmartRepliesAndActions(CharSequence[] choices, String[] actionTitles) {
setSmartReplies(choices);
setSmartRepliesAndActions(choices, actionTitles, false /* fromAssistant */);
}
private void setSmartRepliesAndActions(
CharSequence[] choices, String[] actionTitles, boolean fromAssistant) {
setSmartReplies(choices, fromAssistant);
mView.addSmartActions(
new SmartReplyView.SmartActions(createActions(actionTitles), false),
new SmartReplyView.SmartActions(createActions(actionTitles), fromAssistant),
mLogger,
mEntry,
mHeadsUpManager);
@@ -943,4 +953,78 @@ public class SmartReplyViewTest extends SysuiTestCase {
expectedView.getChildAt(3), mView.getChildAt(4)); // a1
assertReplyButtonHidden(mView.getChildAt(5)); // long action
}
@Test
public void testMeasure_minNumSystemGeneratedSmartReplies_notEnoughReplies() {
when(mConstants.getMinNumSystemGeneratedReplies()).thenReturn(3);
// Add 2 replies when the minimum is 3 -> we should end up with 0 replies.
String[] choices = new String[] {"reply1", "reply2"};
String[] actions = new String[] {"action1"};
ViewGroup expectedView = buildExpectedView(new String[] {}, 1,
createActions(new String[] {"action1"}));
expectedView.measure(MeasureSpec.UNSPECIFIED, MeasureSpec.UNSPECIFIED);
setSmartRepliesAndActions(choices, actions, true /* fromAssistant */);
mView.measure(MeasureSpec.UNSPECIFIED, MeasureSpec.UNSPECIFIED);
assertEqualMeasures(expectedView, mView);
// smart replies
assertReplyButtonHidden(mView.getChildAt(0));
assertReplyButtonHidden(mView.getChildAt(1));
// smart actions
assertReplyButtonShownWithEqualMeasures(expectedView.getChildAt(0), mView.getChildAt(2));
}
@Test
public void testMeasure_minNumSystemGeneratedSmartReplies_enoughReplies() {
when(mConstants.getMinNumSystemGeneratedReplies()).thenReturn(2);
// Add 2 replies when the minimum is 3 -> we should end up with 0 replies.
String[] choices = new String[] {"reply1", "reply2"};
String[] actions = new String[] {"action1"};
ViewGroup expectedView = buildExpectedView(new String[] {"reply1", "reply2"}, 1,
createActions(new String[] {"action1"}));
expectedView.measure(MeasureSpec.UNSPECIFIED, MeasureSpec.UNSPECIFIED);
setSmartRepliesAndActions(choices, actions, true /* fromAssistant */);
mView.measure(MeasureSpec.UNSPECIFIED, MeasureSpec.UNSPECIFIED);
assertEqualMeasures(expectedView, mView);
// smart replies
assertReplyButtonShownWithEqualMeasures(expectedView.getChildAt(0), mView.getChildAt(0));
assertReplyButtonShownWithEqualMeasures(expectedView.getChildAt(1), mView.getChildAt(1));
// smart actions
assertReplyButtonShownWithEqualMeasures(expectedView.getChildAt(2), mView.getChildAt(2));
}
/**
* Ensure actions that are squeezed when shown together with smart replies are unsqueezed if the
* replies are never added (because of the SmartReplyConstants.getMinNumSystemGeneratedReplies()
* flag).
*/
@Test
public void testMeasure_minNumSystemGeneratedSmartReplies_unSqueezeActions() {
when(mConstants.getMinNumSystemGeneratedReplies()).thenReturn(2);
// Add 2 replies when the minimum is 3 -> we should end up with 0 replies.
String[] choices = new String[] {"This is a very long two-line reply."};
String[] actions = new String[] {"Short action"};
// The action should be displayed on one line only - since it fits!
ViewGroup expectedView = buildExpectedView(new String[] {}, 1 /* lineCount */,
createActions(new String[] {"Short action"}));
expectedView.measure(MeasureSpec.UNSPECIFIED, MeasureSpec.UNSPECIFIED);
setSmartRepliesAndActions(choices, actions, true /* fromAssistant */);
mView.measure(MeasureSpec.UNSPECIFIED, MeasureSpec.UNSPECIFIED);
assertEqualMeasures(expectedView, mView);
// smart replies
assertReplyButtonHidden(mView.getChildAt(0));
// smart actions
assertReplyButtonShownWithEqualMeasures(expectedView.getChildAt(0), mView.getChildAt(1));
}
}