Skip to content

Commit

Permalink
Remove DidCommitAndDrawCompositorFrame IPC.
Browse files Browse the repository at this point in the history 
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}
  • Loading branch information
@danakj
danakj authored and Commit Bot committed Sep 6, 2019
1 parent 99d77bc commit 39dd14f
Show file tree
Hide file tree
Showing 15 changed files with 42 additions and 101 deletions.
34 changes: 20 additions & 14 deletions chrome/browser/autofill/captured_sites_test_utils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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),
Expand Down
9 changes: 2 additions & 7 deletions chrome/browser/autofill/captured_sites_test_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

Expand Down
31 changes: 3 additions & 28 deletions chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down
31 changes: 17 additions & 14 deletions components/autofill_assistant/browser/web/web_controller_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,25 +46,25 @@ 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");
}

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();
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions content/browser/renderer_host/render_view_host_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 0 additions & 4 deletions content/browser/renderer_host/render_view_host_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -787,10 +787,6 @@ void RenderViewHostImpl::RenderWidgetDidFirstVisuallyNonEmptyPaint() {
delegate_->DidFirstVisuallyNonEmptyPaint(this);
}

void RenderViewHostImpl::RenderWidgetDidCommitAndDrawCompositorFrame() {
delegate_->DidCommitAndDrawCompositorFrame(this);
}

bool RenderViewHostImpl::SuddenTerminationAllowed() const {
return sudden_termination_allowed_;
}
Expand Down
1 change: 0 additions & 1 deletion content/browser/renderer_host/render_view_host_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 0 additions & 7 deletions content/browser/renderer_host/render_widget_host_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions content/browser/renderer_host/render_widget_host_owner_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 0 additions & 6 deletions content/browser/web_contents/web_contents_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5117,12 +5117,6 @@ void WebContentsImpl::DidFirstVisuallyNonEmptyPaint(
}
}

void WebContentsImpl::DidCommitAndDrawCompositorFrame(
RenderViewHostImpl* source) {
for (auto& observer : observers_)
observer.DidCommitAndDrawCompositorFrame();
}

bool WebContentsImpl::IsPortal() const {
return portal();
}
Expand Down
1 change: 0 additions & 1 deletion content/browser/web_contents/web_contents_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ---------------------------------------------------------
Expand Down
3 changes: 0 additions & 3 deletions content/common/widget_messages.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */)
Expand Down
5 changes: 0 additions & 5 deletions content/public/browser/web_contents_observer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions content/renderer/render_widget.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 0 additions & 1 deletion content/test/stub_render_widget_host_owner_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 39dd14f

Please sign in to comment.