Fixed cancel() and commit():

- Call removeSelfLocked() on cancelSessionLocked.
- Call removeSelf() after dispatching the PendingSaveRequest.
- Added a finalizer callback to PendingSaveRequest that calls removeSelf().
- Fixed SaveUi SnackBar so its actions are only triggered once.
- Reused removeSelfLocked() when needed.
- Removed unnecessary {} on some lambdas.
- Removed unnecessary mLock on PendingSaveRequest.
- Removed unnecessary mLock usage on PendingFillRequest.

Test: CtsAutoFillServiceTestCases (including new tests) pass
Test: manual verification

Fixes: 35721501
Fixes: 35844249

Change-Id: I9789218777b62a9558a602b8eaed0714d8b77fa0
This commit is contained in:
Felipe Leme
2017-03-16 08:59:50 -07:00
parent 24aae15218
commit 6d0cb1e8eb
4 changed files with 136 additions and 61 deletions

View File

@@ -235,9 +235,8 @@ final class AutofillManagerServiceImpl {
if (!hasService()) {
final int sessionCount = mSessions.size();
for (int i = sessionCount - 1; i >= 0; i--) {
Session session = mSessions.valueAt(i);
session.destroyLocked();
mSessions.removeAt(i);
final Session session = mSessions.valueAt(i);
session.removeSelfLocked();
}
}
sendStateToClients();
@@ -328,7 +327,13 @@ final class AutofillManagerServiceImpl {
return;
}
session.showSaveLocked();
final boolean finished = session.showSaveLocked();
if (DEBUG) {
Log.d(TAG, "finishSessionLocked(): session finished on save? " + finished);
}
if (finished) {
session.removeSelf();
}
}
void cancelSessionLocked(IBinder activityToken) {
@@ -341,8 +346,7 @@ final class AutofillManagerServiceImpl {
Slog.w(TAG, "cancelSessionLocked(): no session for " + activityToken);
return;
}
session.destroyLocked();
session.removeSelfLocked();
}
private Session createSessionByTokenLocked(@NonNull IBinder activityToken,
@@ -754,9 +758,7 @@ final class AutofillManagerServiceImpl {
synchronized (mLock) {
fillInIntent = createAuthFillInIntent(mStructure);
}
mHandlerCaller.getHandler().post(() -> {
startAuthentication(intent, fillInIntent);
});
mHandlerCaller.getHandler().post(() -> startAuthentication(intent, fillInIntent));
}
// FillServiceCallbacks
@@ -776,8 +778,7 @@ final class AutofillManagerServiceImpl {
Binder.restoreCallingIdentity(identity);
}
synchronized (mLock) {
destroyLocked();
mSessions.remove(this);
removeSelfLocked();
}
}
@@ -790,9 +791,7 @@ final class AutofillManagerServiceImpl {
// AutoFillUiCallback
@Override
public void fill(Dataset dataset) {
mHandlerCaller.getHandler().post(() -> {
autoFill(dataset);
});
mHandlerCaller.getHandler().post(() -> autoFill(dataset));
}
// AutoFillUiCallback
@@ -805,17 +804,13 @@ final class AutofillManagerServiceImpl {
// AutoFillUiCallback
@Override
public void cancelSave() {
mHandlerCaller.getHandler().post(() -> {
removeSelf();
});
mHandlerCaller.getHandler().post(() -> removeSelf());
}
// AutoFillUiCallback
@Override
public void onEvent(AutofillId id, int event) {
mHandlerCaller.getHandler().post(() -> {
notifyChangeToClient(id, event);
});
mHandlerCaller.getHandler().post(() -> notifyChangeToClient(id, event));
}
public void setAuthenticationResultLocked(Bundle data) {
@@ -845,12 +840,14 @@ final class AutofillManagerServiceImpl {
}
/**
* Show the save UI, when session can be saved.
* Shows the save UI, when session can be saved.
*
* @return {@code true} if session is done, or {@code false} if it's pending user action.
*/
public void showSaveLocked() {
public boolean showSaveLocked() {
if (mStructure == null) {
Slog.wtf(TAG, "showSaveLocked(): no mStructure");
return;
return true;
}
if (mCurrentResponse == null) {
// Happens when the activity / session was finished before the service replied, or
@@ -858,7 +855,7 @@ final class AutofillManagerServiceImpl {
if (DEBUG) {
Slog.d(TAG, "showSaveLocked(): no mCurrentResponse");
}
return;
return true;
}
final SaveInfo saveInfo = mCurrentResponse.getSaveInfo();
if (DEBUG) {
@@ -874,13 +871,13 @@ final class AutofillManagerServiceImpl {
*/
if (saveInfo == null) {
return;
return true;
}
final AutofillId[] requiredIds = saveInfo.getRequiredIds();
if (requiredIds == null || requiredIds.length == 0) {
Slog.w(TAG, "showSaveLocked(): no required ids on saveInfo");
return;
return true;
}
boolean allRequiredAreNotEmpty = true;
@@ -950,7 +947,7 @@ final class AutofillManagerServiceImpl {
getUiForShowing().showSaveUi(
mInfo.getServiceInfo().loadLabel(mContext.getPackageManager()),
saveInfo, mPackageName);
return;
return false;
}
}
// Nothing changed...
@@ -959,7 +956,7 @@ final class AutofillManagerServiceImpl {
+ "allRequiredAreNotNull=" + allRequiredAreNotEmpty
+ ", atLeastOneChanged=" + atLeastOneChanged);
}
removeSelf();
return true;
}
/**
@@ -1001,7 +998,7 @@ final class AutofillManagerServiceImpl {
mStructure.dump();
}
mRemoteFillService.onSaveRequest(mStructure, extras);
mRemoteFillService.onSaveRequest(mStructure, extras, () -> removeSelf());
}
void updateLocked(AutofillId id, Rect bounds, AutofillValue value, int flags) {
@@ -1255,15 +1252,18 @@ final class AutofillManagerServiceImpl {
mPackageName);
}
private void removeSelf() {
if (VERBOSE) {
Slog.v(TAG, "removeSelf()");
}
void removeSelf() {
synchronized (mLock) {
destroyLocked();
mSessions.remove(mActivityToken);
removeSelfLocked();
}
}
private void removeSelfLocked() {
if (VERBOSE) {
Slog.v(TAG, "removeSelfLocked()");
}
destroyLocked();
mSessions.remove(mActivityToken);
}
}
}

View File

@@ -16,6 +16,8 @@
package com.android.server.autofill;
import static com.android.server.autofill.Helper.DEBUG;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.assist.AssistStructure;
@@ -38,8 +40,10 @@ import android.service.autofill.IFillCallback;
import android.service.autofill.ISaveCallback;
import android.text.format.DateUtils;
import android.util.Slog;
import com.android.internal.os.HandlerCaller;
import com.android.server.FgThread;
import com.android.server.autofill.AutofillManagerServiceImpl.Session;
import java.io.PrintWriter;
import java.lang.ref.WeakReference;
@@ -55,8 +59,6 @@ import java.lang.ref.WeakReference;
final class RemoteFillService implements DeathRecipient {
private static final String LOG_TAG = "RemoteFillService";
private static final boolean DEBUG = Helper.DEBUG;
// How long after the last interaction with the service we would unbind
private static final long TIMEOUT_IDLE_BIND_MILLIS = 5 * DateUtils.SECOND_IN_MILLIS;
@@ -139,9 +141,10 @@ final class RemoteFillService implements DeathRecipient {
mHandler.obtainMessageO(MyHandler.MSG_ON_PENDING_REQUEST, request).sendToTarget();
}
public void onSaveRequest(@NonNull AssistStructure structure, @Nullable Bundle extras) {
public void onSaveRequest(@NonNull AssistStructure structure, @Nullable Bundle extras,
@Nullable Runnable finalizer) {
cancelScheduledUnbind();
final PendingSaveRequest request = new PendingSaveRequest(structure, extras, this);
final PendingSaveRequest request = new PendingSaveRequest(structure, extras, this, finalizer);
mHandler.obtainMessageO(MyHandler.MSG_ON_PENDING_REQUEST, request).sendToTarget();
}
@@ -414,8 +417,8 @@ final class RemoteFillService implements DeathRecipient {
private static final class PendingFillRequest extends PendingRequest {
private final Object mLock = new Object();
private final WeakReference<RemoteFillService> mWeakService;
private AssistStructure mStructure;
private Bundle mExtras;
private final AssistStructure mStructure;
private final Bundle mExtras;
private final IFillCallback mCallback;
private ICancellationSignal mCancellation;
private boolean mCancelled;
@@ -473,10 +476,6 @@ final class RemoteFillService implements DeathRecipient {
try {
remoteService.mAutoFillService.onFillRequest(mStructure,
mExtras, mCallback, mFlags);
synchronized (mLock) {
mStructure = null;
mExtras = null;
}
} catch (RemoteException e) {
Slog.e(LOG_TAG, "Error calling on fill request", e);
cancel();
@@ -506,17 +505,18 @@ final class RemoteFillService implements DeathRecipient {
}
private static final class PendingSaveRequest extends PendingRequest {
private final Object mLock = new Object();
private final WeakReference<RemoteFillService> mWeakService;
private AssistStructure mStructure;
private Bundle mExtras;
private final AssistStructure mStructure;
private final Bundle mExtras;
private final ISaveCallback mCallback;
private final Runnable mFinalizer;
public PendingSaveRequest(@NonNull AssistStructure structure,
@Nullable Bundle extras, @NonNull RemoteFillService service) {
public PendingSaveRequest(@NonNull AssistStructure structure, @Nullable Bundle extras,
@NonNull RemoteFillService service, @Nullable Runnable finalizer) {
mStructure = structure;
mExtras = extras;
mWeakService = new WeakReference<>(service);
mFinalizer = finalizer;
mCallback = new ISaveCallback.Stub() {
@Override
public void onSuccess() {
@@ -540,19 +540,17 @@ final class RemoteFillService implements DeathRecipient {
@Override
public void run() {
RemoteFillService service = mWeakService.get();
final RemoteFillService service = mWeakService.get();
if (service != null) {
try {
service.mAutoFillService.onSaveRequest(mStructure,
mExtras, mCallback);
synchronized (mLock) {
mStructure = null;
mExtras = null;
}
service.mAutoFillService.onSaveRequest(mStructure, mExtras, mCallback);
} catch (RemoteException e) {
Slog.e(LOG_TAG, "Error calling on save request", e);
}
}
if (mFinalizer != null) {
mFinalizer.run();
}
}
@Override

View File

@@ -0,0 +1,26 @@
/*
* Copyright (C) 2017 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.server.autofill.ui;
final class Helper {
static final boolean DEBUG = true; // TODO(b/33197203): set to false when stable
static final boolean VERBOSE = false;
private Helper() {
throw new UnsupportedOperationException("contains static members only");
}
}

View File

@@ -16,6 +16,8 @@
package com.android.server.autofill.ui;
import static com.android.server.autofill.ui.Helper.DEBUG;
import android.annotation.NonNull;
import android.app.Dialog;
import android.content.Context;
@@ -23,6 +25,7 @@ import android.content.IntentSender;
import android.os.Handler;
import android.service.autofill.SaveInfo;
import android.text.format.DateUtils;
import android.util.Slog;
import android.view.Gravity;
import android.view.Window;
import android.view.WindowManager;
@@ -37,23 +40,66 @@ import com.android.server.UiThread;
* Autofill Save Prompt
*/
final class SaveUi {
private static final String TAG = "SaveUi";
public interface OnSaveListener {
void onSave();
void onCancel(IntentSender listener);
void onDestroy();
}
private class OneTimeListener implements OnSaveListener {
private final OnSaveListener mRealListener;
private boolean mDone;
OneTimeListener(OnSaveListener realListener) {
mRealListener = realListener;
}
@Override
public void onSave() {
if (DEBUG) Slog.d(TAG, "onSave(): " + mDone);
if (mDone) {
return;
}
mDone = true;
mRealListener.onSave();
}
@Override
public void onCancel(IntentSender listener) {
if (DEBUG) Slog.d(TAG, "onCancel(): " + mDone);
if (mDone) {
return;
}
mDone = true;
mRealListener.onCancel(listener);
}
@Override
public void onDestroy() {
if (DEBUG) Slog.d(TAG, "onDestroy(): " + mDone);
if (mDone) {
return;
}
mDone = true;
mRealListener.onDestroy();
}
}
private final Handler mHandler = UiThread.getHandler();
private final @NonNull Dialog mDialog;
private final @NonNull OnSaveListener mListener;
private final @NonNull OneTimeListener mListener;
private boolean mDestroyed;
SaveUi(@NonNull Context context, @NonNull CharSequence providerLabel, @NonNull SaveInfo info,
@NonNull OnSaveListener listener, int lifeTimeMs) {
mListener = listener;
mListener = new OneTimeListener(listener);
final LayoutInflater inflater = LayoutInflater.from(context);
final View view = inflater.inflate(R.layout.autofill_save, null);
@@ -118,7 +164,12 @@ final class SaveUi {
mDialog.show();
mHandler.postDelayed(() -> mListener.onCancel(null), lifeTimeMs);
mHandler.postDelayed(() -> {
if (!mListener.mDone) {
mListener.onCancel(null);
Slog.d(TAG, "Save snackbar timed out after " + lifeTimeMs + "ms");
}
}, lifeTimeMs);
}
void destroy() {