Skip to content

GitLab

T
tangled
Commit dad827a2 authored by Tommy Nyquist's avatar Tommy Nyquist Committed by Commit Bot
Browse files
Options

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

This reverts commit dbe0ea06.

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/+/2518401Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823660}
parent 74bebd78
Hide whitespace changes
Inline Side-by-side
Showing with 8 additions and 64 deletions
android_webview/javatests/src/org/chromium/android_webview/test/AwContentCaptureTest.java
View file @ dad827a2
...@@ -769,26 +769,4 @@ public class AwContentCaptureTest { ...@@ -769,26 +769,4 @@ public class AwContentCaptureTest {
public void testCantCreateExperimentConsumer() throws Throwable { public void testCantCreateExperimentConsumer() throws Throwable {
Assert.assertNull(ExperimentContentCaptureConsumer.create(mAwContents.getWebContents())); 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(() -> {
// Removes the view and adds it back, this essentially hides and shows the frame.
mRule.getActivity().removeAllViews();
mRule.getActivity().addView(mContainerView);
});
}, toIntArray(TestAwContentCaptureConsumer.CONTENT_CAPTURED));
}
} }
third_party/blink/renderer/core/content_capture/content_capture_manager.cc
View file @ dad827a2
...@@ -34,8 +34,6 @@ ContentCaptureManager::ContentCaptureManager(LocalFrame& local_frame_root) ...@@ -34,8 +34,6 @@ ContentCaptureManager::ContentCaptureManager(LocalFrame& local_frame_root)
ContentCaptureManager::~ContentCaptureManager() = default; ContentCaptureManager::~ContentCaptureManager() = default;
void ContentCaptureManager::ScheduleTaskIfNeeded(const Node& node) { void ContentCaptureManager::ScheduleTaskIfNeeded(const Node& node) {
if (!task_session_)
return;
if (first_node_holder_created_) { if (first_node_holder_created_) {
ScheduleTask( ScheduleTask(
UserActivated(node) UserActivated(node)
...@@ -60,7 +58,6 @@ bool ContentCaptureManager::UserActivated(const Node& node) const { ...@@ -60,7 +58,6 @@ bool ContentCaptureManager::UserActivated(const Node& node) const {
void ContentCaptureManager::ScheduleTask( void ContentCaptureManager::ScheduleTask(
ContentCaptureTask::ScheduleReason reason) { ContentCaptureTask::ScheduleReason reason) {
DCHECK(task_session_);
if (!content_capture_idle_task_) { if (!content_capture_idle_task_) {
content_capture_idle_task_ = CreateContentCaptureTask(); content_capture_idle_task_ = CreateContentCaptureTask();
} }
...@@ -72,10 +69,12 @@ ContentCaptureTask* ContentCaptureManager::CreateContentCaptureTask() { ...@@ -72,10 +69,12 @@ ContentCaptureTask* ContentCaptureManager::CreateContentCaptureTask() {
*task_session_); *task_session_);
} }
void ContentCaptureManager::OnLayoutTextWillBeDestroyed(const Node& node) { void ContentCaptureManager::NotifyNodeDetached(const Node& node) {
if (!task_session_)
return;
task_session_->OnNodeDetached(node); task_session_->OnNodeDetached(node);
}
void ContentCaptureManager::OnLayoutTextWillBeDestroyed(const Node& node) {
NotifyNodeDetached(node);
ScheduleTask( ScheduleTask(
UserActivated(node) UserActivated(node)
? ContentCaptureTask::ScheduleReason::kUserActivatedContentChange ? ContentCaptureTask::ScheduleReason::kUserActivatedContentChange
...@@ -83,8 +82,6 @@ void ContentCaptureManager::OnLayoutTextWillBeDestroyed(const Node& node) { ...@@ -83,8 +82,6 @@ void ContentCaptureManager::OnLayoutTextWillBeDestroyed(const Node& node) {
} }
void ContentCaptureManager::OnScrollPositionChanged() { void ContentCaptureManager::OnScrollPositionChanged() {
if (!task_session_)
return;
ScheduleTask(ContentCaptureTask::ScheduleReason::kScrolling); ScheduleTask(ContentCaptureTask::ScheduleReason::kScrolling);
} }
...@@ -104,8 +101,6 @@ void ContentCaptureManager::NotifyInputEvent(WebInputEvent::Type type, ...@@ -104,8 +101,6 @@ void ContentCaptureManager::NotifyInputEvent(WebInputEvent::Type type,
} }
void ContentCaptureManager::OnNodeTextChanged(Node& node) { void ContentCaptureManager::OnNodeTextChanged(Node& node) {
if (!task_session_)
return;
task_session_->OnNodeChanged(node); task_session_->OnNodeChanged(node);
ScheduleTask( ScheduleTask(
UserActivated(node) UserActivated(node)
...@@ -120,23 +115,11 @@ void ContentCaptureManager::Trace(Visitor* visitor) const { ...@@ -120,23 +115,11 @@ void ContentCaptureManager::Trace(Visitor* visitor) const {
visitor->Trace(latest_user_activation_); 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() { void ContentCaptureManager::Shutdown() {
if (content_capture_idle_task_) { if (content_capture_idle_task_) {
content_capture_idle_task_->Shutdown(); content_capture_idle_task_->Shutdown();
content_capture_idle_task_ = nullptr; content_capture_idle_task_ = nullptr;
} }
task_session_ = nullptr;
} }
} // namespace blink } // namespace blink
third_party/blink/renderer/core/content_capture/content_capture_manager.h
View file @ dad827a2
...@@ -42,10 +42,6 @@ class CORE_EXPORT ContentCaptureManager ...@@ -42,10 +42,6 @@ class CORE_EXPORT ContentCaptureManager
// Invokes when text node content was changed. // Invokes when text node content was changed.
void OnNodeTextChanged(Node& node); void OnNodeTextChanged(Node& node);
// Invokes when the LocalFrameRoot was shown/hidden.
void OnFrameWasShown();
void OnFrameWasHidden();
// Invokes when the local_frame_root shutdown. // Invokes when the local_frame_root shutdown.
void Shutdown(); void Shutdown();
...@@ -70,6 +66,7 @@ class CORE_EXPORT ContentCaptureManager ...@@ -70,6 +66,7 @@ class CORE_EXPORT ContentCaptureManager
virtual void Trace(Visitor*) const; virtual void Trace(Visitor*) const;
}; };
void NotifyNodeDetached(const Node& node);
void ScheduleTask(ContentCaptureTask::ScheduleReason reason); void ScheduleTask(ContentCaptureTask::ScheduleReason reason);
// Returns true if the user had the input in last // Returns true if the user had the input in last
......
third_party/blink/renderer/core/content_capture/content_capture_task.cc
View file @ dad827a2
...@@ -72,7 +72,6 @@ ContentCaptureTask::~ContentCaptureTask() = default; ...@@ -72,7 +72,6 @@ ContentCaptureTask::~ContentCaptureTask() = default;
void ContentCaptureTask::Shutdown() { void ContentCaptureTask::Shutdown() {
DCHECK(local_frame_root_); DCHECK(local_frame_root_);
local_frame_root_ = nullptr; local_frame_root_ = nullptr;
CancelTask();
} }
bool ContentCaptureTask::CaptureContent(Vector<cc::NodeInfo>& data) { bool ContentCaptureTask::CaptureContent(Vector<cc::NodeInfo>& data) {
...@@ -300,10 +299,6 @@ bool ContentCaptureTask::ShouldPause() { ...@@ -300,10 +299,6 @@ bool ContentCaptureTask::ShouldPause() {
return ThreadScheduler::Current()->ShouldYieldForHighPriorityWork(); return ThreadScheduler::Current()->ShouldYieldForHighPriorityWork();
} }
void ContentCaptureTask::CancelTask() {
if (delay_task_ && delay_task_->IsActive())
delay_task_->Stop();
}
void ContentCaptureTask::ClearDocumentSessionsForTesting() { void ContentCaptureTask::ClearDocumentSessionsForTesting() {
task_session_->ClearDocumentSessionsForTesting(); task_session_->ClearDocumentSessionsForTesting();
} }
...@@ -315,7 +310,8 @@ base::TimeDelta ContentCaptureTask::GetTaskNextFireIntervalForTesting() const { ...@@ -315,7 +310,8 @@ base::TimeDelta ContentCaptureTask::GetTaskNextFireIntervalForTesting() const {
} }
void ContentCaptureTask::CancelTaskForTesting() { void ContentCaptureTask::CancelTaskForTesting() {
CancelTask(); if (delay_task_ && delay_task_->IsActive())
delay_task_->Stop();
} }
void ContentCaptureTask::Trace(Visitor* visitor) const { void ContentCaptureTask::Trace(Visitor* visitor) const {
......
third_party/blink/renderer/core/content_capture/content_capture_task.h
View file @ dad827a2
...@@ -135,8 +135,6 @@ class CORE_EXPORT ContentCaptureTask ...@@ -135,8 +135,6 @@ class CORE_EXPORT ContentCaptureTask
void ScheduleInternal(ScheduleReason reason); void ScheduleInternal(ScheduleReason reason);
bool CaptureContent(Vector<cc::NodeInfo>& data); bool CaptureContent(Vector<cc::NodeInfo>& data);
void CancelTask();
// Indicates if there is content change since last run. // Indicates if there is content change since last run.
bool has_content_change_ = false; bool has_content_change_ = false;
......
third_party/blink/renderer/core/frame/local_frame.cc
View file @ dad827a2
...@@ -1837,10 +1837,6 @@ void LocalFrame::WasHidden() { ...@@ -1837,10 +1837,6 @@ void LocalFrame::WasHidden() {
return; return;
hidden_ = true; hidden_ = true;
if (content_capture_manager_) {
content_capture_manager_->OnFrameWasHidden();
}
// An iframe may get a "was hidden" notification before it has been attached // An iframe may get a "was hidden" notification before it has been attached
// to the frame tree; in that case, skip further processing. // to the frame tree; in that case, skip further processing.
if (!Owner() || IsProvisional()) if (!Owner() || IsProvisional())
...@@ -1878,10 +1874,6 @@ void LocalFrame::WasShown() { ...@@ -1878,10 +1874,6 @@ void LocalFrame::WasShown() {
hidden_ = false; hidden_ = false;
if (LocalFrameView* frame_view = View()) if (LocalFrameView* frame_view = View())
frame_view->ScheduleAnimation(); frame_view->ScheduleAnimation();
if (content_capture_manager_) {
content_capture_manager_->OnFrameWasShown();
}
} }
bool LocalFrame::ClipsContent() const { bool LocalFrame::ClipsContent() const {
......
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