From 39dd14f61b0787cd82553d834b51ee3db0cf54f4 Mon Sep 17 00:00:00 2001
From: danakj <danakj@chromium.org>
Date: Fri, 6 Sep 2019 17:31:31 +0000
Subject: [PATCH] Remove DidCommitAndDrawCompositorFrame IPC.
This IPC was used for tests to synchronize with the renderer from the
browser process. The RenderFrameSubmissionObserver does this job for
us instead now and does not rely on legacy mojo or test-only IPCs. This
CL converts the 3 test cases relying on the old IPC over to use the
RenderFrameSubmissionObserver utility instead.
R=avi@chromium.org, rouslan@chromium.org
Bug: 419087
Change-Id: I61d3cf37f55bfbbf97e73f8523605613d4e9be39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1787406
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694304}
---
.../autofill/captured_sites_test_utils.cc | 34 +++++++++++--------
.../autofill/captured_sites_test_utils.h | 9 ++---
...s_slide_controller_chromeos_browsertest.cc | 31 ++---------------
.../browser/web/web_controller_browsertest.cc | 31 +++++++++--------
.../renderer_host/render_view_host_delegate.h | 4 ---
.../renderer_host/render_view_host_impl.cc | 4 ---
.../renderer_host/render_view_host_impl.h | 1 -
.../renderer_host/render_widget_host_impl.cc | 7 ----
.../render_widget_host_owner_delegate.h | 4 ---
.../browser/web_contents/web_contents_impl.cc | 6 ----
.../browser/web_contents/web_contents_impl.h | 1 -
content/common/widget_messages.h | 3 --
.../public/browser/web_contents_observer.h | 5 ---
content/renderer/render_widget.cc | 2 --
.../stub_render_widget_host_owner_delegate.h | 1 -
15 files changed, 42 insertions(+), 101 deletions(-)
diff --git a/chrome/browser/autofill/captured_sites_test_utils.cc b/chrome/browser/autofill/captured_sites_test_utils.cc
index a0b3012f9d4b1..e6d242a222a6a 100644
--- a/chrome/browser/autofill/captured_sites_test_utils.cc
+++ b/chrome/browser/autofill/captured_sites_test_utils.cc
@@ -195,14 +195,18 @@ PageActivityObserver::PageActivityObserver(content::RenderFrameHost* frame)
void PageActivityObserver::WaitTillPageIsIdle(
base::TimeDelta continuous_paint_timeout) {
base::TimeTicks finished_load_time = base::TimeTicks::Now();
- bool page_is_loading = false;
- do {
- paint_occurred_during_last_loop_ = false;
- base::RunLoop heart_beat;
- base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
- FROM_HERE, heart_beat.QuitClosure(), kPaintEventCheckInterval);
- heart_beat.Run();
- page_is_loading =
+ while (true) {
+ content::RenderFrameSubmissionObserver frame_submission_observer(
+ web_contents());
+ // Runs a loop for kPaintEventCheckInterval to see if the renderer is
+ // idle.
+ {
+ base::RunLoop heart_beat;
+ base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE, heart_beat.QuitClosure(), kPaintEventCheckInterval);
+ heart_beat.Run();
+ }
+ bool page_is_loading =
web_contents()->IsWaitingForResponse() || web_contents()->IsLoading();
if (page_is_loading) {
finished_load_time = base::TimeTicks::Now();
@@ -214,13 +218,19 @@ void PageActivityObserver::WaitTillPageIsIdle(
// blinking caret or a persistent animation is making Chrome paint at
// regular intervals. Exit.
break;
+ } else if (frame_submission_observer.render_frame_count() == 0) {
+ // If the renderer has stopped submitting frames for the waiting interval
+ // then we're done.
+ break;
}
- } while (page_is_loading || paint_occurred_during_last_loop_);
+ }
}
bool PageActivityObserver::WaitForVisualUpdate(base::TimeDelta timeout) {
base::TimeTicks start_time = base::TimeTicks::Now();
- while (!paint_occurred_during_last_loop_) {
+ content::RenderFrameSubmissionObserver frame_submission_observer(
+ web_contents());
+ while (frame_submission_observer.render_frame_count() == 0) {
base::RunLoop heart_beat;
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, heart_beat.QuitClosure(), kPaintEventCheckInterval);
@@ -232,10 +242,6 @@ bool PageActivityObserver::WaitForVisualUpdate(base::TimeDelta timeout) {
return true;
}
-void PageActivityObserver::DidCommitAndDrawCompositorFrame() {
- paint_occurred_during_last_loop_ = true;
-}
-
// FrameObserver --------------------------------------------------------------
IFrameWaiter::IFrameWaiter(content::WebContents* web_contents)
: content::WebContentsObserver(web_contents),
diff --git a/chrome/browser/autofill/captured_sites_test_utils.h b/chrome/browser/autofill/captured_sites_test_utils.h
index 0d83d2ad1a078..ff3c36f58c048 100644
--- a/chrome/browser/autofill/captured_sites_test_utils.h
+++ b/chrome/browser/autofill/captured_sites_test_utils.h
@@ -117,16 +117,11 @@ class PageActivityObserver : public content::WebContentsObserver {
private:
// PageActivityObserver determines if Chrome stopped painting by checking if
- // Chrome hasn't painted for a specific amount of time.
- // kPaintEventCheckInterval defines this amount of time.
+ // the renderer hasn't submitted a compositor frame for a specific amount of
+ // time. kPaintEventCheckInterval defines this amount of time.
static constexpr base::TimeDelta kPaintEventCheckInterval =
base::TimeDelta::FromMilliseconds(500);
- // content::WebContentsObserver:
- void DidCommitAndDrawCompositorFrame() override;
-
- bool paint_occurred_during_last_loop_ = false;
-
DISALLOW_COPY_AND_ASSIGN(PageActivityObserver);
};
diff --git a/chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc b/chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc
index 0d7fcc8f2739c..ac749579aca71 100644
--- a/chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc
+++ b/chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc
@@ -1164,32 +1164,6 @@ IN_PROC_BROWSER_TEST_F(TopControlsSlideControllerTest, TestPermissionBubble) {
TopChromeShownState::kFullyHidden);
}
-// Waits for a compositor frame to be drawn and committed on the given
-// web_contents.
-class CompositorFrameWaiter : content::WebContentsObserver {
- public:
- explicit CompositorFrameWaiter(content::WebContents* contents)
- : WebContentsObserver(contents) {}
- ~CompositorFrameWaiter() override = default;
-
- void Wait() {
- run_loop_ = std::make_unique<base::RunLoop>();
- run_loop_->Run();
- run_loop_.reset();
- }
-
- // content::WebContentsObserver:
- void DidCommitAndDrawCompositorFrame() override {
- if (run_loop_)
- run_loop_->Quit();
- }
-
- private:
- std::unique_ptr<base::RunLoop> run_loop_;
-
- DISALLOW_COPY_AND_ASSIGN(CompositorFrameWaiter);
-};
-
IN_PROC_BROWSER_TEST_F(TopControlsSlideControllerTest, TestToggleChromeVox) {
ToggleTabletMode();
ASSERT_TRUE(GetTabletModeEnabled());
@@ -1220,9 +1194,10 @@ IN_PROC_BROWSER_TEST_F(TopControlsSlideControllerTest, TestToggleChromeVox) {
// Now disable Chromevox, and expect it's now possible to hide top-chrome with
// gesture scrolling.
- CompositorFrameWaiter compositor_frame_waiter(active_contents);
+ content::RenderFrameSubmissionObserver compositor_frame_waiter(
+ active_contents);
chromeos::AccessibilityManager::Get()->EnableSpokenFeedback(false);
- compositor_frame_waiter.Wait();
+ compositor_frame_waiter.WaitForAnyFrameSubmission();
content::WaitForResizeComplete(active_contents);
EXPECT_FALSE(
chromeos::AccessibilityManager::Get()->IsSpokenFeedbackEnabled());
diff --git a/components/autofill_assistant/browser/web/web_controller_browsertest.cc b/components/autofill_assistant/browser/web/web_controller_browsertest.cc
index 9481f480c9c7a..62ddd64d7159b 100644
--- a/components/autofill_assistant/browser/web/web_controller_browsertest.cc
+++ b/components/autofill_assistant/browser/web/web_controller_browsertest.cc
@@ -46,10 +46,6 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
Observe(shell()->web_contents());
}
- void DidCommitAndDrawCompositorFrame() override {
- paint_occurred_during_last_loop_ = true;
- }
-
void SetUpCommandLine(base::CommandLine* command_line) override {
ContentBrowserTest::SetUpCommandLine(command_line);
command_line->AppendSwitch("allow-pre-commit-input");
@@ -57,14 +53,18 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
void WaitTillPageIsIdle(base::TimeDelta continuous_paint_timeout) {
base::TimeTicks finished_load_time = base::TimeTicks::Now();
- bool page_is_loading = false;
- do {
- paint_occurred_during_last_loop_ = false;
- base::RunLoop heart_beat;
- base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
- FROM_HERE, heart_beat.QuitClosure(), base::TimeDelta::FromSeconds(3));
- heart_beat.Run();
- page_is_loading =
+ while (true) {
+ content::RenderFrameSubmissionObserver frame_submission_observer(
+ web_contents());
+ // Runs a loop for 3 seconds to see if the renderer is idle.
+ {
+ base::RunLoop heart_beat;
+ base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE, heart_beat.QuitClosure(),
+ base::TimeDelta::FromSeconds(3));
+ heart_beat.Run();
+ }
+ bool page_is_loading =
web_contents()->IsWaitingForResponse() || web_contents()->IsLoading();
if (page_is_loading) {
finished_load_time = base::TimeTicks::Now();
@@ -76,8 +76,12 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
// blinking caret or a persistent animation is making Chrome paint at
// regular intervals. Exit.
break;
+ } else if (frame_submission_observer.render_frame_count() == 0) {
+ // If the renderer has stopped submitting frames for 3 seconds then
+ // we're done.
+ break;
}
- } while (page_is_loading || paint_occurred_during_last_loop_);
+ }
}
void RunStrictElementCheck(const Selector& selector, bool result) {
@@ -461,7 +465,6 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
private:
std::unique_ptr<net::EmbeddedTestServer> http_server_;
- bool paint_occurred_during_last_loop_ = false;
ClientSettings settings_;
DISALLOW_COPY_AND_ASSIGN(WebControllerBrowserTest);
diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h
index 7ecbec7dab3a3..9a5e428690f7b 100644
--- a/content/browser/renderer_host/render_view_host_delegate.h
+++ b/content/browser/renderer_host/render_view_host_delegate.h
@@ -194,10 +194,6 @@ class CONTENT_EXPORT RenderViewHostDelegate {
// The RenderView finished the first visually non-empty paint.
virtual void DidFirstVisuallyNonEmptyPaint(RenderViewHostImpl* source) {}
- // The RenderView has issued a draw command, signaling the it
- // has been visually updated.
- virtual void DidCommitAndDrawCompositorFrame(RenderViewHostImpl* source) {}
-
// Returns true if the render view is rendering a portal.
virtual bool IsPortal() const;
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 344eba7291bdf..01ea60106eb06 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -787,10 +787,6 @@ void RenderViewHostImpl::RenderWidgetDidFirstVisuallyNonEmptyPaint() {
delegate_->DidFirstVisuallyNonEmptyPaint(this);
}
-void RenderViewHostImpl::RenderWidgetDidCommitAndDrawCompositorFrame() {
- delegate_->DidCommitAndDrawCompositorFrame(this);
-}
-
bool RenderViewHostImpl::SuddenTerminationAllowed() const {
return sudden_termination_allowed_;
}
diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
index 1a98f40d1c76d..0b2ad958b98d0 100644
--- a/content/browser/renderer_host/render_view_host_impl.h
+++ b/content/browser/renderer_host/render_view_host_impl.h
@@ -219,7 +219,6 @@ class CONTENT_EXPORT RenderViewHostImpl
void RenderWidgetDidInit() override;
void RenderWidgetDidClose() override;
void RenderWidgetDidFirstVisuallyNonEmptyPaint() override;
- void RenderWidgetDidCommitAndDrawCompositorFrame() override;
void RenderWidgetGotFocus() override;
void RenderWidgetLostFocus() override;
void RenderWidgetDidForwardMouseEvent(
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index cfa68bf03b993..39f1820b3e90a 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -645,8 +645,6 @@ bool RenderWidgetHostImpl::OnMessageReceived(const IPC::Message &msg) {
OnForceRedrawComplete)
IPC_MESSAGE_HANDLER(WidgetHostMsg_DidFirstVisuallyNonEmptyPaint,
OnFirstVisuallyNonEmptyPaint)
- IPC_MESSAGE_HANDLER(WidgetHostMsg_DidCommitAndDrawCompositorFrame,
- OnCommitAndDrawCompositorFrame)
IPC_MESSAGE_HANDLER(WidgetHostMsg_HasTouchEventHandlers,
OnHasTouchEventHandlers)
IPC_MESSAGE_HANDLER(WidgetHostMsg_IntrinsicSizingInfoChanged,
@@ -1838,11 +1836,6 @@ void RenderWidgetHostImpl::OnFirstVisuallyNonEmptyPaint() {
owner_delegate_->RenderWidgetDidFirstVisuallyNonEmptyPaint();
}
-void RenderWidgetHostImpl::OnCommitAndDrawCompositorFrame() {
- if (owner_delegate_)
- owner_delegate_->RenderWidgetDidCommitAndDrawCompositorFrame();
-}
-
void RenderWidgetHostImpl::RendererExited() {
if (!renderer_initialized_)
return;
diff --git a/content/browser/renderer_host/render_widget_host_owner_delegate.h b/content/browser/renderer_host/render_widget_host_owner_delegate.h
index 8f44ffb163030..fb8a31134835c 100644
--- a/content/browser/renderer_host/render_widget_host_owner_delegate.h
+++ b/content/browser/renderer_host/render_widget_host_owner_delegate.h
@@ -41,10 +41,6 @@ class CONTENT_EXPORT RenderWidgetHostOwnerDelegate {
// The RenderWidget finished the first visually non-empty paint.
virtual void RenderWidgetDidFirstVisuallyNonEmptyPaint() = 0;
- // The RenderWidget has issued a draw command, signaling the widget
- // has been visually updated.
- virtual void RenderWidgetDidCommitAndDrawCompositorFrame() = 0;
-
// The RenderWidgetHost got the focus.
virtual void RenderWidgetGotFocus() = 0;
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index a4e4a612d82c8..ddd8255bcf586 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -5117,12 +5117,6 @@ void WebContentsImpl::DidFirstVisuallyNonEmptyPaint(
}
}
-void WebContentsImpl::DidCommitAndDrawCompositorFrame(
- RenderViewHostImpl* source) {
- for (auto& observer : observers_)
- observer.DidCommitAndDrawCompositorFrame();
-}
-
bool WebContentsImpl::IsPortal() const {
return portal();
}
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index dfb543d031dc6..b256fe63e3a98 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -698,7 +698,6 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
bool IsSpatialNavigationDisabled() const override;
RenderFrameHost* GetPendingMainFrame() override;
void DidFirstVisuallyNonEmptyPaint(RenderViewHostImpl* source) override;
- void DidCommitAndDrawCompositorFrame(RenderViewHostImpl* source) override;
bool IsPortal() const override;
// NavigatorDelegate ---------------------------------------------------------
diff --git a/content/common/widget_messages.h b/content/common/widget_messages.h
index 1f3e6fcf467c8..c4061482b0be1 100644
--- a/content/common/widget_messages.h
+++ b/content/common/widget_messages.h
@@ -333,9 +333,6 @@ IPC_MESSAGE_ROUTED0(WidgetHostMsg_WaitForNextFrameForTests_ACK)
// after the frame widget has painted something.
IPC_MESSAGE_ROUTED0(WidgetHostMsg_DidFirstVisuallyNonEmptyPaint)
-// Sent once the RenderWidgetCompositor issues a draw command.
-IPC_MESSAGE_ROUTED0(WidgetHostMsg_DidCommitAndDrawCompositorFrame)
-
// Notifies whether there are JavaScript touch event handlers or not.
IPC_MESSAGE_ROUTED1(WidgetHostMsg_HasTouchEventHandlers,
bool /* has_handlers */)
diff --git a/content/public/browser/web_contents_observer.h b/content/public/browser/web_contents_observer.h
index e97e4616cb51f..0e0de8f6a0f75 100644
--- a/content/public/browser/web_contents_observer.h
+++ b/content/public/browser/web_contents_observer.h
@@ -568,11 +568,6 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener {
const std::string& interface_name,
mojo::ScopedMessagePipeHandle* interface_pipe) {}
- // Notifies that the RenderWidgetCompositor has issued a draw command. An
- // observer can use this method to detect when Chrome visually updated a
- // tab.
- virtual void DidCommitAndDrawCompositorFrame() {}
-
// Called when "audible" playback starts or stops on a WebAudio AudioContext.
using AudioContextId = std::pair<RenderFrameHost*, int>;
virtual void AudioContextPlaybackStarted(
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 93f95b11ebeea..0a5eecd586835 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -1217,8 +1217,6 @@ void RenderWidget::DidCommitAndDrawCompositorFrame() {
// Notify subclasses that we initiated the paint operation.
DidInitiatePaint();
-
- Send(new WidgetHostMsg_DidCommitAndDrawCompositorFrame(routing_id_));
}
void RenderWidget::WillCommitCompositorFrame() {
diff --git a/content/test/stub_render_widget_host_owner_delegate.h b/content/test/stub_render_widget_host_owner_delegate.h
index fdb151a1a9c7e..3185b79b98f38 100644
--- a/content/test/stub_render_widget_host_owner_delegate.h
+++ b/content/test/stub_render_widget_host_owner_delegate.h
@@ -14,7 +14,6 @@ class StubRenderWidgetHostOwnerDelegate : public RenderWidgetHostOwnerDelegate {
void RenderWidgetDidInit() override {}
void RenderWidgetDidClose() override {}
void RenderWidgetDidFirstVisuallyNonEmptyPaint() override {}
- void RenderWidgetDidCommitAndDrawCompositorFrame() override {}
void RenderWidgetGotFocus() override {}
void RenderWidgetLostFocus() override {}
void RenderWidgetDidForwardMouseEvent(