Commit f715b16c authored by Michael Bai's avatar Michael Bai Committed by Commit Bot

ContentCapture: Suppress platform exception

Framework has a bug which causes a exception in a race condition,
the exception make the app crash.

This patch catches this specific exception to prevent crash.

Adds a new set of tests for NotificationTask, its subclasses and
PlatformAPIWrapper to verify the capture exception working
correctly.

Bug: 1131430
Change-Id: Ia6ef93c47421700c0e52b1de267c6be00d0dd51f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521224Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarChangwan Ryu <changwan@chromium.org>
Commit-Queue: Michael Bai <michaelbai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826020}
parent 62f1fc8e
......@@ -794,6 +794,7 @@ if (is_android) {
"//components/browser_ui/util/android:junit",
"//components/browser_ui/webshare/android:junit",
"//components/browser_ui/widget/android:junit",
"//components/content_capture/android/junit:components_content_capture_junit_tests",
"//components/embedder_support/android:components_embedder_support_junit_tests",
"//components/gcm_driver/android:components_gcm_driver_junit_tests",
"//components/media_router/browser/android:junit",
......
......@@ -41,6 +41,8 @@ android_library("java") {
"java/src/org/chromium/components/content_capture/ExperimentContentCaptureConsumer.java",
"java/src/org/chromium/components/content_capture/FrameSession.java",
"java/src/org/chromium/components/content_capture/NotificationTask.java",
"java/src/org/chromium/components/content_capture/PlatformAPIWrapper.java",
"java/src/org/chromium/components/content_capture/PlatformAPIWrapperImpl.java",
"java/src/org/chromium/components/content_capture/PlatformSession.java",
"java/src/org/chromium/components/content_capture/ProcessContentCaptureDataTask.java",
"java/src/org/chromium/components/content_capture/SessionRemovedTask.java",
......
......@@ -19,16 +19,16 @@ class ContentRemovedTask extends NotificationTask {
}
@Override
protected Boolean doInBackground() {
protected void runTask() {
removeContent();
return true;
}
private void removeContent() {
log("ContentRemovedTask.removeContent");
PlatformSessionData platformSessionData = buildCurrentSession();
if (platformSessionData == null) return;
platformSessionData.contentCaptureSession.notifyViewsDisappeared(
PlatformAPIWrapper.getInstance().notifyViewsDisappeared(
platformSessionData.contentCaptureSession,
mPlatformSession.getRootPlatformSessionData().autofillId, mRemovedIds);
}
}
......@@ -25,10 +25,11 @@ class ContentUpdateTask extends ProcessContentCaptureDataTask {
private AutofillId notifyViewTextChanged(
PlatformSessionData parentPlatformSessionData, ContentCaptureData data) {
AutofillId autofillId = parentPlatformSessionData.contentCaptureSession.newAutofillId(
AutofillId autofillId = PlatformAPIWrapper.getInstance().newAutofillId(
parentPlatformSessionData.contentCaptureSession,
mPlatformSession.getRootPlatformSessionData().autofillId, data.getId());
parentPlatformSessionData.contentCaptureSession.notifyViewTextChanged(
autofillId, data.getValue());
PlatformAPIWrapper.getInstance().notifyViewTextChanged(
parentPlatformSessionData.contentCaptureSession, autofillId, data.getValue());
return autofillId;
}
}
......@@ -5,15 +5,15 @@
package org.chromium.components.content_capture;
import android.annotation.TargetApi;
import android.content.LocusId;
import android.graphics.Rect;
import android.os.Build;
import android.text.TextUtils;
import android.view.ViewStructure;
import android.view.autofill.AutofillId;
import android.view.contentcapture.ContentCaptureContext;
import android.view.contentcapture.ContentCaptureSession;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.Log;
import org.chromium.base.annotations.VerifiesOnQ;
import org.chromium.base.task.AsyncTask;
......@@ -31,6 +31,26 @@ abstract class NotificationTask extends AsyncTask<Boolean> {
protected final FrameSession mSession;
protected final PlatformSession mPlatformSession;
private boolean mHasPlatformExceptionForTesting;
/**
* A specific framework ContentCapture exception in Android Q and R shall be caught to prevent
* the crash, the current NotificationTask can't be recovered from exception and has to
* exit, the next task shall continue to run even it could cause the inconsistent state in
* Android framework and aiai service who shall bear with it.
*
* Refer to crbug.com/1131430 for details.
*/
private static boolean isMainContentCaptureSesionSentEventException(NullPointerException e) {
for (StackTraceElement s : e.getStackTrace()) {
if (s.getClassName().startsWith("android.view.contentcapture.MainContentCaptureSession")
&& s.getMethodName().startsWith("sendEvent")) {
return true;
}
}
return false;
}
public NotificationTask(FrameSession session, PlatformSession platformSession) {
mSession = session;
mPlatformSession = platformSession;
......@@ -54,15 +74,16 @@ abstract class NotificationTask extends AsyncTask<Boolean> {
protected AutofillId notifyViewAppeared(
PlatformSessionData parentPlatformSessionData, ContentCaptureData data) {
ViewStructure viewStructure =
parentPlatformSessionData.contentCaptureSession.newVirtualViewStructure(
parentPlatformSessionData.autofillId, data.getId());
ViewStructure viewStructure = PlatformAPIWrapper.getInstance().newVirtualViewStructure(
parentPlatformSessionData.contentCaptureSession,
parentPlatformSessionData.autofillId, data.getId());
if (!data.hasChildren()) viewStructure.setText(data.getValue());
Rect rect = data.getBounds();
// Always set scroll as (0, 0).
viewStructure.setDimens(rect.left, rect.top, 0, 0, rect.width(), rect.height());
parentPlatformSessionData.contentCaptureSession.notifyViewAppeared(viewStructure);
PlatformAPIWrapper.getInstance().notifyViewAppeared(
parentPlatformSessionData.contentCaptureSession, viewStructure);
return viewStructure.getAutofillId();
}
......@@ -72,10 +93,10 @@ abstract class NotificationTask extends AsyncTask<Boolean> {
mPlatformSession.getFrameIdToPlatformSessionData().get(frame.getId());
if (platformSessionData == null && !TextUtils.isEmpty(frame.getValue())) {
ContentCaptureSession session =
parentPlatformSessionData.contentCaptureSession.createContentCaptureSession(
new ContentCaptureContext.Builder(new LocusId(frame.getValue()))
.build());
AutofillId autofillId = parentPlatformSessionData.contentCaptureSession.newAutofillId(
PlatformAPIWrapper.getInstance().createContentCaptureSession(
parentPlatformSessionData.contentCaptureSession, frame.getValue());
AutofillId autofillId = PlatformAPIWrapper.getInstance().newAutofillId(
parentPlatformSessionData.contentCaptureSession,
mPlatformSession.getRootPlatformSessionData().autofillId, frame.getId());
autofillId = notifyViewAppeared(parentPlatformSessionData, frame);
platformSessionData = new PlatformSessionData(session, autofillId);
......@@ -85,10 +106,32 @@ abstract class NotificationTask extends AsyncTask<Boolean> {
return platformSessionData;
}
@VisibleForTesting
public boolean hasPlatformExceptionForTesting() {
return mHasPlatformExceptionForTesting;
}
protected void log(String message) {
if (sDump.booleanValue()) Log.i(TAG, message);
}
@Override
protected void onPostExecute(Boolean result) {}
@Override
public final Boolean doInBackground() {
try {
runTask();
} catch (NullPointerException e) {
if (isMainContentCaptureSesionSentEventException(e)) {
mHasPlatformExceptionForTesting = true;
Log.e(TAG, "PlatformException", e);
} else {
throw e;
}
}
return true;
}
protected abstract void runTask();
}
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.components.content_capture;
import android.annotation.TargetApi;
import android.os.Build;
import android.view.ViewStructure;
import android.view.autofill.AutofillId;
import android.view.contentcapture.ContentCaptureSession;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.annotations.VerifiesOnQ;
/**
* The class to wrap ContentCapture platform APIs, catches the exception from platform, and
* re-throws with PlatformAPIException, so the call sites can catch platform exception to avoid
* the crash.
*/
@VerifiesOnQ
@TargetApi(Build.VERSION_CODES.Q)
public abstract class PlatformAPIWrapper {
private static PlatformAPIWrapper sImpl;
public static PlatformAPIWrapper getInstance() {
if (sImpl == null) {
sImpl = new PlatformAPIWrapperImpl();
}
return sImpl;
}
public abstract ContentCaptureSession createContentCaptureSession(
ContentCaptureSession parent, String url);
public abstract void destroyContentCaptureSession(ContentCaptureSession session);
public abstract AutofillId newAutofillId(
ContentCaptureSession parent, AutofillId rootAutofillId, long id);
public abstract ViewStructure newVirtualViewStructure(
ContentCaptureSession parent, AutofillId parentAutofillId, long id);
public abstract void notifyViewAppeared(
ContentCaptureSession session, ViewStructure viewStructure);
public abstract void notifyViewDisappeared(
ContentCaptureSession session, AutofillId autofillId);
public abstract void notifyViewsDisappeared(
ContentCaptureSession session, AutofillId autofillId, long[] ids);
public abstract void notifyViewTextChanged(
ContentCaptureSession session, AutofillId autofillId, String newContent);
@VisibleForTesting
public static void setPlatformAPIWrapperImplForTesting(PlatformAPIWrapper impl) {
sImpl = impl;
}
}
\ No newline at end of file
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.components.content_capture;
import android.annotation.TargetApi;
import android.content.LocusId;
import android.os.Build;
import android.view.ViewStructure;
import android.view.autofill.AutofillId;
import android.view.contentcapture.ContentCaptureContext;
import android.view.contentcapture.ContentCaptureSession;
import org.chromium.base.annotations.VerifiesOnQ;
/**
* The implementation of PlatformAPIWrapper.
*/
@VerifiesOnQ
@TargetApi(Build.VERSION_CODES.Q)
public class PlatformAPIWrapperImpl extends PlatformAPIWrapper {
@Override
public ContentCaptureSession createContentCaptureSession(
ContentCaptureSession parent, String url) {
return parent.createContentCaptureSession(
new ContentCaptureContext.Builder(new LocusId(url)).build());
}
@Override
public void destroyContentCaptureSession(ContentCaptureSession session) {
session.destroy();
}
@Override
public AutofillId newAutofillId(
ContentCaptureSession parent, AutofillId rootAutofillId, long id) {
return parent.newAutofillId(rootAutofillId, id);
}
@Override
public ViewStructure newVirtualViewStructure(
ContentCaptureSession parent, AutofillId parentAutofillId, long id) {
return parent.newVirtualViewStructure(parentAutofillId, id);
}
@Override
public void notifyViewAppeared(ContentCaptureSession session, ViewStructure viewStructure) {
session.notifyViewAppeared(viewStructure);
}
@Override
public void notifyViewDisappeared(ContentCaptureSession session, AutofillId autofillId) {
session.notifyViewDisappeared(autofillId);
}
@Override
public void notifyViewsDisappeared(
ContentCaptureSession session, AutofillId autofillId, long[] ids) {
session.notifyViewsDisappeared(autofillId, ids);
}
@Override
public void notifyViewTextChanged(
ContentCaptureSession session, AutofillId autofillId, String newContent) {
session.notifyViewTextChanged(autofillId, newContent);
}
}
......@@ -27,9 +27,8 @@ abstract class ProcessContentCaptureDataTask extends NotificationTask {
}
@Override
protected Boolean doInBackground() {
protected void runTask() {
processContent();
return true;
}
private void processContent() {
......
......@@ -15,9 +15,8 @@ class SessionRemovedTask extends NotificationTask {
}
@Override
protected Boolean doInBackground() {
protected void runTask() {
removeSession();
return true;
}
private void removeSession() {
......@@ -25,7 +24,8 @@ class SessionRemovedTask extends NotificationTask {
PlatformSessionData removedPlatformSessionData =
mPlatformSession.getFrameIdToPlatformSessionData().remove(mSession.get(0).getId());
if (removedPlatformSessionData == null) return;
removedPlatformSessionData.contentCaptureSession.destroy();
PlatformAPIWrapper.getInstance().destroyContentCaptureSession(
removedPlatformSessionData.contentCaptureSession);
PlatformSessionData parentPlatformSessionData =
mPlatformSession.getRootPlatformSessionData();
// We need to notify the view disappeared through the removed session's parent,
......@@ -36,7 +36,8 @@ class SessionRemovedTask extends NotificationTask {
mPlatformSession.getFrameIdToPlatformSessionData().get(mSession.get(1).getId());
}
if (parentPlatformSessionData == null) return;
parentPlatformSessionData.contentCaptureSession.notifyViewDisappeared(
PlatformAPIWrapper.getInstance().notifyViewDisappeared(
parentPlatformSessionData.contentCaptureSession,
removedPlatformSessionData.autofillId);
}
}
# Copyright 2020 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import("//build/config/android/config.gni")
import("//build/config/android/rules.gni")
java_library("components_content_capture_junit_tests") {
# Platform checks are broken for Robolectric. See https://crbug.com/1071638.
bypass_platform_checks = true
testonly = true
sources = [
"src/org/chromium/components/content_capture/PlatformAPIWrapperTest.java",
]
deps = [
"//base:base_java",
"//base:base_java_test_support",
"//base:base_junit_test_support",
"//components/content_capture/android:java",
"//content/public/android:content_java",
"//third_party/android_deps:robolectric_all_java",
"//third_party/junit",
"//third_party/mockito:mockito_java",
]
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment