0

Reland "ContentCapture: re-stream onscreen content when the frame show again"

This reverts commit dad827a279.

Reason for revert: Change the test, previous test method caused
DCHECK failure in L, M, N. The new way won't remove the view from view
tree instead notify the window invisible to let the renderer think
the frame is in background.

Verified in N device with debug build, the issue was fixed.

Original change's description:
> Revert "ContentCapture: re-stream onscreen content when the frame show again"
>
> This reverts commit dbe0ea06a5.
>
> Reason for revert: New test fails on the following bots:
> https://ci.chromium.org/p/chromium/builders/ci/Lollipop%20Phone%20Tester
> https://ci.chromium.org/p/chromium/builders/ci/Marshmallow%20Tablet%20Tester
> https://ci.chromium.org/p/chromium/builders/ci/Android%20WebView%20N%20%28dbg%29
>
> Original change's description:
> > ContentCapture: re-stream onscreen content when the frame show again
> >
> > Previously ContentCaptureConsumer had no way to know the the frame
> > shows again because the visible content didn't change.
> >
> > This patch resets the task session when the frame was hidden, so
> > ContentCapture task will stream on-screen content because they
> > aren't streamed from the task point of view when the frame was
> > shown.
> >
> > Adds the test to coverage this scenario.
> >
> > Bug: 1137463
> > Change-Id: I0065ce080101d2149622b0f9f6a3aa196c18d717
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2511030
> > Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> > Commit-Queue: Michael Bai <michaelbai@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#823251}
>
> TBR=michaelbai@chromium.org,wangxianzhu@chromium.org
>
> Change-Id: I219bd94c5dfc36b073590d6e604ce951aef3c966
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1137463
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518401
> Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
> Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#823660}

TBR=michaelbai@chromium.org,nyquist@chromium.org,wangxianzhu@chromium.org

# Not skipping CQ checks because this is a reland.

Bug: 1137463
Change-Id: Ib7be9bf21a5aadcf88b79f4334e5298ee9d41275
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518752
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Michael Bai <michaelbai@chromium.org>
Commit-Queue: Michael Bai <michaelbai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824167}
This commit is contained in:
Michael Bai
2020-11-04 21:55:37 +00:00
committed by Commit Bot
parent ba4b3fd5aa
commit b4e95a6f71
6 changed files with 66 additions and 8 deletions
android_webview/javatests/src/org/chromium/android_webview/test
third_party/blink/renderer/core

@ -6,6 +6,7 @@ package org.chromium.android_webview.test;
import android.graphics.Rect;
import android.net.Uri;
import android.view.View;
import androidx.test.filters.LargeTest;
import androidx.test.filters.SmallTest;
@ -769,4 +770,27 @@ public class AwContentCaptureTest {
public void testCantCreateExperimentConsumer() throws Throwable {
Assert.assertNull(ExperimentContentCaptureConsumer.create(mAwContents.getWebContents()));
}
@Test
@SmallTest
@Feature({"AndroidWebView"})
public void testHideAndShow() throws Throwable {
final String response = "<html><head></head><body>"
+ "<div id='editable_id'>Hello</div>"
+ "</div></body></html>";
final String url = mWebServer.setResponse(MAIN_FRAME_FILE, response, null);
runAndVerifyCallbacks(() -> {
loadUrlSync(url);
}, toIntArray(TestAwContentCaptureConsumer.CONTENT_CAPTURED));
// Hides and shows the WebContent and verifies the content is captured again.
runAndVerifyCallbacks(() -> {
TestThreadUtils.runOnUiThreadBlocking(
() -> { mContainerView.onWindowVisibilityChanged(View.INVISIBLE); });
AwActivityTestRule.pollInstrumentationThread(() -> !mAwContents.isPageVisible());
TestThreadUtils.runOnUiThreadBlocking(
() -> { mContainerView.onWindowVisibilityChanged(View.VISIBLE); });
AwActivityTestRule.pollInstrumentationThread(() -> mAwContents.isPageVisible());
}, toIntArray(TestAwContentCaptureConsumer.CONTENT_CAPTURED));
}
}

@ -34,6 +34,8 @@ ContentCaptureManager::ContentCaptureManager(LocalFrame& local_frame_root)
ContentCaptureManager::~ContentCaptureManager() = default;
void ContentCaptureManager::ScheduleTaskIfNeeded(const Node& node) {
if (!task_session_)
return;
if (first_node_holder_created_) {
ScheduleTask(
UserActivated(node)
@ -58,6 +60,7 @@ bool ContentCaptureManager::UserActivated(const Node& node) const {
void ContentCaptureManager::ScheduleTask(
ContentCaptureTask::ScheduleReason reason) {
DCHECK(task_session_);
if (!content_capture_idle_task_) {
content_capture_idle_task_ = CreateContentCaptureTask();
}
@ -69,12 +72,10 @@ ContentCaptureTask* ContentCaptureManager::CreateContentCaptureTask() {
*task_session_);
}
void ContentCaptureManager::NotifyNodeDetached(const Node& node) {
task_session_->OnNodeDetached(node);
}
void ContentCaptureManager::OnLayoutTextWillBeDestroyed(const Node& node) {
NotifyNodeDetached(node);
if (!task_session_)
return;
task_session_->OnNodeDetached(node);
ScheduleTask(
UserActivated(node)
? ContentCaptureTask::ScheduleReason::kUserActivatedContentChange
@ -82,6 +83,8 @@ void ContentCaptureManager::OnLayoutTextWillBeDestroyed(const Node& node) {
}
void ContentCaptureManager::OnScrollPositionChanged() {
if (!task_session_)
return;
ScheduleTask(ContentCaptureTask::ScheduleReason::kScrolling);
}
@ -101,6 +104,8 @@ void ContentCaptureManager::NotifyInputEvent(WebInputEvent::Type type,
}
void ContentCaptureManager::OnNodeTextChanged(Node& node) {
if (!task_session_)
return;
task_session_->OnNodeChanged(node);
ScheduleTask(
UserActivated(node)
@ -115,11 +120,23 @@ void ContentCaptureManager::Trace(Visitor* visitor) const {
visitor->Trace(latest_user_activation_);
}
void ContentCaptureManager::OnFrameWasShown() {
if (task_session_)
return;
task_session_ = MakeGarbageCollected<TaskSession>();
ScheduleTask(ContentCaptureTask::ScheduleReason::kFirstContentChange);
}
void ContentCaptureManager::OnFrameWasHidden() {
Shutdown();
}
void ContentCaptureManager::Shutdown() {
if (content_capture_idle_task_) {
content_capture_idle_task_->Shutdown();
content_capture_idle_task_ = nullptr;
}
task_session_ = nullptr;
}
} // namespace blink

@ -42,6 +42,10 @@ class CORE_EXPORT ContentCaptureManager
// Invokes when text node content was changed.
void OnNodeTextChanged(Node& node);
// Invokes when the LocalFrameRoot was shown/hidden.
void OnFrameWasShown();
void OnFrameWasHidden();
// Invokes when the local_frame_root shutdown.
void Shutdown();
@ -66,7 +70,6 @@ class CORE_EXPORT ContentCaptureManager
virtual void Trace(Visitor*) const;
};
void NotifyNodeDetached(const Node& node);
void ScheduleTask(ContentCaptureTask::ScheduleReason reason);
// Returns true if the user had the input in last

@ -72,6 +72,7 @@ ContentCaptureTask::~ContentCaptureTask() = default;
void ContentCaptureTask::Shutdown() {
DCHECK(local_frame_root_);
local_frame_root_ = nullptr;
CancelTask();
}
bool ContentCaptureTask::CaptureContent(Vector<cc::NodeInfo>& data) {
@ -306,6 +307,10 @@ bool ContentCaptureTask::ShouldPause() {
return ThreadScheduler::Current()->ShouldYieldForHighPriorityWork();
}
void ContentCaptureTask::CancelTask() {
if (delay_task_ && delay_task_->IsActive())
delay_task_->Stop();
}
void ContentCaptureTask::ClearDocumentSessionsForTesting() {
task_session_->ClearDocumentSessionsForTesting();
}
@ -317,8 +322,7 @@ base::TimeDelta ContentCaptureTask::GetTaskNextFireIntervalForTesting() const {
}
void ContentCaptureTask::CancelTaskForTesting() {
if (delay_task_ && delay_task_->IsActive())
delay_task_->Stop();
CancelTask();
}
void ContentCaptureTask::Trace(Visitor* visitor) const {

@ -135,6 +135,8 @@ class CORE_EXPORT ContentCaptureTask
void ScheduleInternal(ScheduleReason reason);
bool CaptureContent(Vector<cc::NodeInfo>& data);
void CancelTask();
// Indicates if there is content change since last run.
bool has_content_change_ = false;

@ -1838,6 +1838,10 @@ void LocalFrame::WasHidden() {
return;
hidden_ = true;
if (content_capture_manager_) {
content_capture_manager_->OnFrameWasHidden();
}
// An iframe may get a "was hidden" notification before it has been attached
// to the frame tree; in that case, skip further processing.
if (!Owner() || IsProvisional())
@ -1875,6 +1879,10 @@ void LocalFrame::WasShown() {
hidden_ = false;
if (LocalFrameView* frame_view = View())
frame_view->ScheduleAnimation();
if (content_capture_manager_) {
content_capture_manager_->OnFrameWasShown();
}
}
bool LocalFrame::ClipsContent() const {