Merge "Fixed FillCallback.onFailure() and SaveCallback.onFailure() behavior."

This commit is contained in:
TreeHugger Robot
2018-09-24 18:48:13 +00:00
committed by Android (Google) Code Review
8 changed files with 94 additions and 27 deletions

View File

@@ -651,14 +651,11 @@ public abstract class AutofillService extends Service {
/**
* Called when the user requests the service to save the contents of a screen.
*
* <p>Service must call one of the {@link SaveCallback} methods (like
* {@link SaveCallback#onSuccess()} or {@link SaveCallback#onFailure(CharSequence)})
* to notify the Android System of the result of the request.
*
* <p>If the service could not handle the request right away&mdash;for example, because it must
* launch an activity asking the user to authenticate first or because the network is
* down&mdash;the service could keep the {@link SaveRequest request} and reuse it later,
* but the service must call {@link SaveCallback#onSuccess()} right away.
* but the service <b>must always</b> call {@link SaveCallback#onSuccess()} or
* {@link SaveCallback#onSuccess(android.content.IntentSender)} right away.
*
* <p><b>Note:</b> To retrieve the actual value of fields input by the user, the service
* should call

View File

@@ -83,19 +83,24 @@ public final class FillCallback {
* fulfill the request; in this case, the service should call {@link #onSuccess(FillResponse)
* onSuccess(null)} instead.
*
* <p><b>Note: </b>on Android versions up to {@link android.os.Build.VERSION_CODES#P}, this
* method is not working as intended, and the service should call
* <p><b>Note: </b>prior to {@link android.os.Build.VERSION_CODES#Q}, this
* method was not working as intended and the service should always call
* {@link #onSuccess(FillResponse) onSuccess(null)} instead.
*
* @param message error message to be displayed to the user. <b>Note: </b> this message is
* displayed on {@code logcat} logs and should not contain PII (Personally Identifiable
* Information, such as username or email address).
* <p><b>Note: </b>for apps targeting {@link android.os.Build.VERSION_CODES#Q} or higher, this
* method just logs the message on {@code logcat}; for apps targetting older SDKs, it also
* displays the message to user using a {@link android.widget.Toast}. Generally speaking, you
* should not display an error to the user if the request failed, unless the request had the
* {@link FillRequest#FLAG_MANUAL_REQUEST} flag.
*
* @param message error message. <b>Note: </b> this message should <b>not</b> contain PII
* (Personally Identifiable Information, such as username or email address).
*
* @throws IllegalStateException if this method or {@link #onSuccess(FillResponse)} was already
* called.
*/
public void onFailure(@Nullable CharSequence message) {
Log.w(TAG, "onFailure(): " + (message == null ? null : message.length() + "_chars"));
Log.w(TAG, "onFailure(): " + message);
assertNotCalled();
mCalled = true;
try {

View File

@@ -83,28 +83,30 @@ public final class SaveCallback {
}
}
/**
* Notifies the Android System that an
* {@link AutofillService#onSaveRequest(SaveRequest, SaveCallback)} could not be handled
* by the service.
*
* <p>This method should only be called when the service could not handle the request right away
* and could not recover or retry it. If the service could retry or recover, it could keep
* the {@link SaveRequest} and call {@link #onSuccess()} instead.
* <p>This method is just used for logging purposes, the Android System won't call the service
* again in case of failures&mdash;if you need to recover from the failure, just save the
* {@link SaveRequest} and try again later.
*
* <p><b>Note:</b> The Android System displays an UI with the supplied error message; if
* you prefer to show your own message, call {@link #onSuccess()} or
* {@link #onSuccess(IntentSender)} instead.
* <p><b>Note: </b>for apps targeting {@link android.os.Build.VERSION_CODES#Q} or higher, this
* method just logs the message on {@code logcat}; for apps targetting older SDKs, it also
* displays the message to user using a {@link android.widget.Toast}.
*
* @param message error message to be displayed to the user. <b>Note: </b> this message is
* displayed on {@code logcat} logs and should not contain PII (Personally Identifiable
* Information, such as username or email address).
* @param message error message. <b>Note: </b> this message should <b>not</b> contain PII
* (Personally Identifiable Information, such as username or email address).
*
* @throws IllegalStateException if this method, {@link #onSuccess()},
* or {@link #onSuccess(IntentSender)} was already called.
*/
public void onFailure(CharSequence message) {
Log.w(TAG, "onFailure(): " + (message == null ? null : message.length() + "_chars"));
Log.w(TAG, "onFailure(): " + message);
assertNotCalled();
mCalled = true;
try {

View File

@@ -301,6 +301,14 @@ public final class AutofillManager {
*/
public static final int STATE_UNKNOWN_COMPAT_MODE = 5;
/**
* Same as {@link #STATE_UNKNOWN}, but used on
* {@link AutofillManagerClient#setSessionFinished(int)} when the session was finished because
* the service failed to fullfil a request.
*
* @hide
*/
public static final int STATE_UNKNOWN_FAILED = 6;
/**
* Timeout in ms for calls to the field classification service.
@@ -2023,8 +2031,10 @@ public final class AutofillManager {
* @param newState {@link #STATE_FINISHED} (because the autofill service returned a {@code null}
* FillResponse), {@link #STATE_UNKNOWN} (because the session was removed),
* {@link #STATE_UNKNOWN_COMPAT_MODE} (beucase the session was finished when the URL bar
* changed on compat mode), or {@link #STATE_DISABLED_BY_SERVICE} (because the autofill service
* disabled further autofill requests for the activity).
* changed on compat mode), {@link #STATE_UNKNOWN_FAILED} (because the session was finished
* when the service failed to fullfil the request, or {@link #STATE_DISABLED_BY_SERVICE}
* (because the autofill service or {@link #STATE_DISABLED_BY_SERVICE} (because the autofill
* service disabled further autofill requests for the activity).
*/
private void setSessionFinished(int newState) {
synchronized (mLock) {
@@ -2032,7 +2042,7 @@ public final class AutofillManager {
Log.v(TAG, "setSessionFinished(): from " + getStateAsStringLocked() + " to "
+ getStateAsString(newState));
}
if (newState == STATE_UNKNOWN_COMPAT_MODE) {
if (newState == STATE_UNKNOWN_COMPAT_MODE || newState == STATE_UNKNOWN_FAILED) {
resetSessionLocked(/* resetEnteredIds= */ true);
mState = STATE_UNKNOWN;
} else {
@@ -2229,6 +2239,8 @@ public final class AutofillManager {
return "DISABLED_BY_SERVICE";
case STATE_UNKNOWN_COMPAT_MODE:
return "UNKNOWN_COMPAT_MODE";
case STATE_UNKNOWN_FAILED:
return "UNKNOWN_FAILED";
default:
return "INVALID:" + state;
}

View File

@@ -4026,6 +4026,8 @@ message MetricsEvent {
// - AUTOFILL_DATASET_AUTHENTICATED
// - AUTOFILL_INVALID_AUTHENTICATION
// - AUTOFILL_INVALID_DATASET_AUTHENTICATION
// NOTE: starting on OS Q, it also added the following fields:
// Tag FIELD_AUTOFILL_TEXT_LEN: length of the error message provided by the service
AUTOFILL_REQUEST = 907;
// Tag of a field for a package of an autofill service
@@ -4102,6 +4104,8 @@ message MetricsEvent {
// Tag FIELD_CLASS_NAME: Class name of the activity that is autofilled.
// Tag FIELD_AUTOFILL_SESSION_ID: id of the autofill session associated with this metric.
// Tag FIELD_AUTOFILL_COMPAT_MODE: package is being autofilled on compatibility mode.
// NOTE: starting on OS Q, it also added the following fields:
// Tag FIELD_AUTOFILL_TEXT_LEN: length of the error message provided by the service
AUTOFILL_DATA_SAVE_REQUEST = 918;
// An auto-fill session was finished
@@ -6517,6 +6521,9 @@ message MetricsEvent {
// OS: Q
MOBILE_NETWORK = 1571;
// Tag of a field for the length of a text
FIELD_AUTOFILL_TEXT_LEN = 1572;
// ---- End Q Constants, all Q constants go above this line ----
// Add new aosp constants above this line.

View File

@@ -235,6 +235,13 @@ final class AutofillManagerServiceImpl {
}
}
int getTargedSdkLocked() {
if (mInfo == null) {
return 0;
}
return mInfo.getServiceInfo().applicationInfo.targetSdkVersion;
}
private boolean isSetupCompletedLocked() {
final String setupComplete = Settings.Secure.getStringForUser(
mContext.getContentResolver(), Settings.Secure.USER_SETUP_COMPLETE, mUserId);
@@ -953,6 +960,7 @@ final class AutofillManagerServiceImpl {
pw.println();
mInfo.dump(prefix2, pw);
pw.print(prefix); pw.print("Service Label: "); pw.println(getServiceLabel());
pw.print(prefix); pw.print("Target SDK: "); pw.println(getTargedSdkLocked());
}
pw.print(prefix); pw.print("Component from settings: ");
pw.println(getComponentNameFromSettings());

View File

@@ -29,6 +29,7 @@ import android.content.Context;
import android.content.Intent;
import android.content.IntentSender;
import android.content.ServiceConnection;
import android.os.Build;
import android.os.Handler;
import android.os.IBinder;
import android.os.IBinder.DeathRecipient;

View File

@@ -50,6 +50,7 @@ import android.graphics.Bitmap;
import android.graphics.Rect;
import android.metrics.LogMaker;
import android.os.Binder;
import android.os.Build;
import android.os.Bundle;
import android.os.Handler;
import android.os.IBinder;
@@ -63,6 +64,7 @@ import android.service.autofill.AutofillService;
import android.service.autofill.Dataset;
import android.service.autofill.FieldClassification;
import android.service.autofill.FieldClassification.Match;
import android.text.TextUtils;
import android.service.autofill.FillContext;
import android.service.autofill.FillRequest;
import android.service.autofill.FillResponse;
@@ -744,12 +746,17 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
private void onFillRequestFailureOrTimeout(int requestId, boolean timedOut,
@Nullable CharSequence message) {
boolean showMessage = !TextUtils.isEmpty(message);
synchronized (mLock) {
if (mDestroyed) {
Slog.w(TAG, "Call to Session#onFillRequestFailureOrTimeout(req=" + requestId
+ ") rejected - session: " + id + " destroyed");
return;
}
if (sDebug) {
Slog.d(TAG, "finishing session due to service "
+ (timedOut ? "timeout" : "failure"));
}
mService.resetLastResponse();
final LogMaker requestLog = mRequestLogs.get(requestId);
if (requestLog == null) {
@@ -757,8 +764,21 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
} else {
requestLog.setType(timedOut ? MetricsEvent.TYPE_CLOSE : MetricsEvent.TYPE_FAILURE);
}
if (showMessage) {
final int targetSdk = mService.getTargedSdkLocked();
if (targetSdk >= Build.VERSION_CODES.Q) {
showMessage = false;
Slog.w(TAG, "onFillRequestFailureOrTimeout(): not showing '" + message
+ "' because service's targetting API " + targetSdk);
}
if (message != null) {
requestLog.addTaggedData(MetricsEvent.FIELD_AUTOFILL_TEXT_LEN,
message.length());
}
}
}
if (message != null) {
notifyUnavailableToClient(AutofillManager.STATE_UNKNOWN_FAILED);
if (showMessage) {
getUiForShowing().showError(message, this);
}
removeSelf();
@@ -793,6 +813,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
@Override
public void onSaveRequestFailure(@Nullable CharSequence message,
@NonNull String servicePackageName) {
boolean showMessage = !TextUtils.isEmpty(message);
synchronized (mLock) {
mIsSaving = false;
@@ -801,12 +822,26 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
+ id + " destroyed");
return;
}
if (showMessage) {
final int targetSdk = mService.getTargedSdkLocked();
if (targetSdk >= Build.VERSION_CODES.Q) {
showMessage = false;
Slog.w(TAG, "onSaveRequestFailure(): not showing '" + message
+ "' because service's targetting API " + targetSdk);
}
}
}
LogMaker log = newLogMaker(MetricsEvent.AUTOFILL_DATA_SAVE_REQUEST, servicePackageName)
final LogMaker log =
newLogMaker(MetricsEvent.AUTOFILL_DATA_SAVE_REQUEST, servicePackageName)
.setType(MetricsEvent.TYPE_FAILURE);
if (message != null) {
log.addTaggedData(MetricsEvent.FIELD_AUTOFILL_TEXT_LEN, message.length());
}
mMetricsLogger.write(log);
getUiForShowing().showError(message, this);
if (showMessage) {
getUiForShowing().showError(message, this);
}
removeSelf();
}