Skip to content

Commit

Permalink
ContentCapture: Add OnscreenContentProvider
Browse files Browse the repository at this point in the history
This patch is 3rd patch of the refactoring, it has below changes
- ContentCaptureConsumer became a interface, it stops to handle the
  WebContents change.
- Rename ContentCaptureConsumerImpl to PlatformContentCaptureConsumer.
- Rename ContentCaptureReceiverManager to OnscreenContentProvider.
- OnscreenContentProvider adds PlatformContentCaptureConsumer and
  ExperimentContentCaptureConsumer as needed, so each embedder's
  interaction to ContentCapture becomes simple.
- The onscreen content streaming won't start if there is no
  consumer.
- OnscreenContentProvider handles the WebContents change, it creates
  native ContentCaptureReceiverManagerAndroid as needed and switches
  to current WebContents by removing/adding itself as consumer.
- ContentCaptureReceiverManagerAndroid as a native consumer is owned
  by OnscreenContentProvider, it forwards the contentcapture events
  to OnscreenContentProvider and does the actual consumer switch.
  It will be renamed to OnscreenContentProviderAndroid in next patch.

No new test, all cases shall already be covered by existing tests.

Bug: 1191672, 1119663
Change-Id: I83533884f1bd4527c71a2dd612a709429c729ee5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2794728
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Michael Bai <michaelbai@chromium.org>
Commit-Queue: Michael Bai <michaelbai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#868120}
  • Loading branch information
Michael Bai authored and Chromium LUCI CQ committed Mar 31, 2021
1 parent 5f67138 commit 89355e6
Show file tree
Hide file tree
Showing 15 changed files with 263 additions and 216 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.ScopedSysTraceEvent;
import org.chromium.components.autofill.AutofillProvider;
import org.chromium.components.content_capture.ContentCaptureConsumerImpl;
import org.chromium.components.content_capture.ContentCaptureFeatures;
import org.chromium.components.content_capture.OnscreenContentProvider;
import org.chromium.components.embedder_support.application.ClassLoaderContextWrapperFactory;
import org.chromium.content_public.browser.NavigationHistory;
import org.chromium.content_public.browser.SmartClipProvider;
Expand Down Expand Up @@ -1832,7 +1832,7 @@ public void onProvideContentCaptureStructure(ViewStructure structure, int flags)
if (ContentCaptureFeatures.isDumpForTestingEnabled()) {
Log.i("ContentCapture", "onProvideContentCaptureStructure");
}
mAwContents.setContentCaptureConsumer(ContentCaptureConsumerImpl.create(
mAwContents.setOnscreenContentProvider(new OnscreenContentProvider(
ClassLoaderContextWrapperFactory.get(mWebView.getContext()), mWebView, structure,
mAwContents.getWebContents()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
import org.chromium.base.task.PostTask;
import org.chromium.components.autofill.AutofillActionModeCallback;
import org.chromium.components.autofill.AutofillProvider;
import org.chromium.components.content_capture.ContentCaptureConsumer;
import org.chromium.components.content_capture.OnscreenContentProvider;
import org.chromium.components.embedder_support.util.WebResourceResponseInfo;
import org.chromium.components.navigation_interception.InterceptNavigationDelegate;
import org.chromium.components.navigation_interception.NavigationParams;
Expand Down Expand Up @@ -479,7 +479,7 @@ public abstract static class VisualStateCallback {

private JavascriptInjector mJavascriptInjector;

private ContentCaptureConsumer mContentCaptureConsumer;
private OnscreenContentProvider mOnscreenContentProvider;

private AwDisplayCutoutController mDisplayCutoutController;
private final AwDisplayModeController mDisplayModeController;
Expand Down Expand Up @@ -1347,8 +1347,8 @@ private void setNewAwContents(long newAwContentsPtr) {
setNewAwContentsPreO(newAwContentsPtr);
if (textClassifier != null) setTextClassifier(textClassifier);
}
if (mContentCaptureConsumer != null) {
mContentCaptureConsumer.onWebContentsChanged(mWebContents);
if (mOnscreenContentProvider != null) {
mOnscreenContentProvider.onWebContentsChanged(mWebContents);
}
}

Expand Down Expand Up @@ -1540,9 +1540,9 @@ public void destroy() {
if (TRACE) Log.i(TAG, "%s destroy", this);
if (isDestroyed(NO_WARN)) return;

if (mContentCaptureConsumer != null) {
mContentCaptureConsumer.onWebContentsChanged(null);
mContentCaptureConsumer = null;
if (mOnscreenContentProvider != null) {
mOnscreenContentProvider.destroy();
mOnscreenContentProvider = null;
}

// Remove pending messages
Expand Down Expand Up @@ -1688,8 +1688,8 @@ private Rect getGlobalVisibleRect() {
return sLocalGlobalVisibleRect;
}

public void setContentCaptureConsumer(ContentCaptureConsumer consumer) {
mContentCaptureConsumer = consumer;
public void setOnscreenContentProvider(OnscreenContentProvider onscreenContentProvider) {
mOnscreenContentProvider = onscreenContentProvider;
}

//--------------------------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@
import org.chromium.components.content_capture.ContentCaptureData;
import org.chromium.components.content_capture.ContentCaptureDataBase;
import org.chromium.components.content_capture.ContentCaptureFrame;
import org.chromium.components.content_capture.ExperimentContentCaptureConsumer;
import org.chromium.components.content_capture.FrameSession;
import org.chromium.components.content_capture.OnscreenContentProvider;
import org.chromium.components.content_capture.UrlAllowlist;
import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.net.test.util.TestWebServer;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;

Expand All @@ -49,7 +49,7 @@
@RunWith(AwJUnit4ClassRunner.class)
@CommandLineFlags.Add({"enable-features=ContentCapture"})
public class AwContentCaptureTest {
private static class TestAwContentCaptureConsumer extends ContentCaptureConsumer {
private static class TestAwContentCaptureConsumer implements ContentCaptureConsumer {
private static final long DEFAULT_TIMEOUT_IN_SECONDS = 30;

public static final int CONTENT_CAPTURED = 1;
Expand All @@ -58,8 +58,7 @@ private static class TestAwContentCaptureConsumer extends ContentCaptureConsumer
public static final int SESSION_REMOVED = 4;
public static final int TITLE_UPDATED = 5;

public TestAwContentCaptureConsumer(WebContents webContents) {
super(webContents);
public TestAwContentCaptureConsumer() {
mCapturedContentIds = new HashSet<Long>();
}

Expand Down Expand Up @@ -213,6 +212,7 @@ public int[] getCallbacks() {
private AwTestContainerView mContainerView;
private TestAwContentCaptureConsumer mConsumer;
private TestAwContentCaptureConsumer mSecondConsumer;
private OnscreenContentProvider mOnscreenContentProvider;

private void loadUrlSync(String url) {
try {
Expand All @@ -236,8 +236,12 @@ public void setUp() throws Exception {
mAwContents = mContainerView.getAwContents();
AwActivityTestRule.enableJavaScriptOnUiThread(mAwContents);
TestThreadUtils.runOnUiThreadBlocking(() -> {
mConsumer = new TestAwContentCaptureConsumer(mAwContents.getWebContents());
mAwContents.setContentCaptureConsumer(mConsumer);
mConsumer = new TestAwContentCaptureConsumer();
mOnscreenContentProvider = new OnscreenContentProvider(
mRule.getActivity(), mContainerView, mAwContents.getWebContents());
mOnscreenContentProvider.addConsumer(mConsumer);
mOnscreenContentProvider.removePlatformConsumerForTesting();
mAwContents.setOnscreenContentProvider(mOnscreenContentProvider);
});
}

Expand Down Expand Up @@ -679,9 +683,8 @@ public void testRemoveIframe() throws Throwable {
@FlakyTest(message = "https://crbug.com/1126950")
@Feature({"AndroidWebView"})
public void testMultipleConsumers() throws Throwable {
TestThreadUtils.runOnUiThreadBlocking(() -> {
mSecondConsumer = new TestAwContentCaptureConsumer(mAwContents.getWebContents());
});
TestThreadUtils.runOnUiThreadBlocking(
() -> { mSecondConsumer = new TestAwContentCaptureConsumer(); });
final String response = "<html><head></head><body>"
+ "<div id='place_holder'>"
+ "<p style=\"height: 100vh\">Hello</p>"
Expand All @@ -701,9 +704,8 @@ public void testMultipleConsumers() throws Throwable {
@Feature({"AndroidWebView"})
@CommandLineFlags.Add({"enable-features=ContentCaptureTriggeringForExperiment"})
public void testHostNotAllowed() throws Throwable {
TestThreadUtils.runOnUiThreadBlocking(() -> {
mSecondConsumer = new TestAwContentCaptureConsumer(mAwContents.getWebContents());
});
TestThreadUtils.runOnUiThreadBlocking(
() -> { mSecondConsumer = new TestAwContentCaptureConsumer(); });
final String response = "<html><head></head><body>"
+ "<div id='place_holder'>"
+ "<p style=\"height: 100vh\">Hello</p>"
Expand Down Expand Up @@ -752,7 +754,9 @@ public void testHostAllowedForExperiment() throws Throwable {
@Feature({"AndroidWebView"})
@CommandLineFlags.Add({"disable-features=ContentCaptureTriggeringForExperiment"})
public void testCantCreateExperimentConsumer() throws Throwable {
Assert.assertNull(ExperimentContentCaptureConsumer.create(mAwContents.getWebContents()));
List<ContentCaptureConsumer> consumers = mOnscreenContentProvider.getConsumersForTesting();
Assert.assertEquals(1, consumers.size());
Assert.assertTrue(consumers.get(0) instanceof TestAwContentCaptureConsumer);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@
import org.chromium.chrome.browser.ui.TabObscuringHandler;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.components.browser_ui.widget.InsetObserverView;
import org.chromium.components.content_capture.ContentCaptureConsumer;
import org.chromium.components.content_capture.ContentCaptureConsumerImpl;
import org.chromium.components.content_capture.ExperimentContentCaptureConsumer;
import org.chromium.components.content_capture.OnscreenContentProvider;
import org.chromium.components.embedder_support.view.ContentView;
import org.chromium.content_public.browser.ImeAdapter;
import org.chromium.content_public.browser.WebContents;
Expand Down Expand Up @@ -215,11 +213,7 @@ public interface TouchEventObserver {
*/
private boolean mHasKeyboardGeometryChangeFired;

// Indicates if ContentCaptureConsumer should be created, we only try to create it once.
private boolean mShouldCreateContentCaptureConsumer = true;
// TODO: (crbug.com/1119663) Move consumers out of this class while support multiple consumers.
private ArrayList<ContentCaptureConsumer> mContentCaptureConsumers =
new ArrayList<ContentCaptureConsumer>();
private OnscreenContentProvider mOnscreenContentProvider;

private Set<Runnable> mOnCompositorLayoutCallbacks = new HashSet<>();
private Set<Runnable> mDidSwapFrameCallbacks = new HashSet<>();
Expand Down Expand Up @@ -572,10 +566,7 @@ public void shutDown() {
mInsetObserverView.removeObserver(this);
mInsetObserverView = null;
}
for (ContentCaptureConsumer consumer : mContentCaptureConsumers) {
consumer.onWebContentsChanged(null);
}
mContentCaptureConsumers.clear();
if (mOnscreenContentProvider != null) mOnscreenContentProvider.destroy();
if (mContentView != null) {
mContentView.removeOnHierarchyChangeListener(this);
}
Expand Down Expand Up @@ -1433,20 +1424,11 @@ private void setTab(Tab tab) {

if (mTabVisible != null) initializeTab(mTabVisible);

if (mShouldCreateContentCaptureConsumer) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
ContentCaptureConsumer consumer =
ContentCaptureConsumerImpl.create(getContext(), this, getWebContents());
if (consumer != null) mContentCaptureConsumers.add(consumer);
}
ContentCaptureConsumer consumer =
ExperimentContentCaptureConsumer.create(getWebContents());
if (consumer != null) mContentCaptureConsumers.add(consumer);
mShouldCreateContentCaptureConsumer = false;
if (mOnscreenContentProvider == null) {
mOnscreenContentProvider =
new OnscreenContentProvider(getContext(), this, getWebContents());
} else {
for (ContentCaptureConsumer consumer : mContentCaptureConsumers) {
consumer.onWebContentsChanged(getWebContents());
}
mOnscreenContentProvider.onWebContentsChanged(getWebContents());
}
}

Expand Down
6 changes: 3 additions & 3 deletions components/content_capture/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@ android_library("java") {
annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
sources = [
"java/src/org/chromium/components/content_capture/ContentCaptureConsumer.java",
"java/src/org/chromium/components/content_capture/ContentCaptureConsumerImpl.java",
"java/src/org/chromium/components/content_capture/ContentCaptureData.java",
"java/src/org/chromium/components/content_capture/ContentCaptureDataBase.java",
"java/src/org/chromium/components/content_capture/ContentCaptureFeatures.java",
"java/src/org/chromium/components/content_capture/ContentCaptureFrame.java",
"java/src/org/chromium/components/content_capture/ContentCaptureReceiverManager.java",
"java/src/org/chromium/components/content_capture/ContentCapturedTask.java",
"java/src/org/chromium/components/content_capture/ContentRemovedTask.java",
"java/src/org/chromium/components/content_capture/ContentUpdateTask.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/OnscreenContentProvider.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/PlatformContentCaptureConsumer.java",
"java/src/org/chromium/components/content_capture/PlatformContentCaptureController.java",
"java/src/org/chromium/components/content_capture/PlatformSession.java",
"java/src/org/chromium/components/content_capture/ProcessContentCaptureDataTask.java",
Expand All @@ -54,6 +54,6 @@ generate_jni("jni_headers") {
"java/src/org/chromium/components/content_capture/ContentCaptureData.java",
"java/src/org/chromium/components/content_capture/ContentCaptureFeatures.java",
"java/src/org/chromium/components/content_capture/ContentCaptureFrame.java",
"java/src/org/chromium/components/content_capture/ContentCaptureReceiverManager.java",
"java/src/org/chromium/components/content_capture/OnscreenContentProvider.java",
]
}

0 comments on commit 89355e6

Please sign in to comment.