Commit 0908efa5 authored by erikchen's avatar erikchen Committed by Commit Bot

Flush WidgetInputHandler messages before running layout tests.

Layout test initialization is non-deterministic. WidgetInputHandler::OnFocus()
races against the contents of the layout test, which may also attempt to
set/unset focus -- the ordering between the two is non-deterministic. This CL
adds a FlushForTesting() message to WidgetInputHandler to make the ordering
explicit.

Bug: 889036, 889952
Change-Id: I26adca82915e75a9941c93b60a555f0c16084014
Reviewed-on: https://chromium-review.googlesource.com/c/1255782Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596321}
parent d6466202
...@@ -269,6 +269,10 @@ std::vector<DropData::Metadata> DropDataToMetaData(const DropData& drop_data) { ...@@ -269,6 +269,10 @@ std::vector<DropData::Metadata> DropDataToMetaData(const DropData& drop_data) {
class UnboundWidgetInputHandler : public mojom::WidgetInputHandler { class UnboundWidgetInputHandler : public mojom::WidgetInputHandler {
public: public:
void FlushForTesting(FlushForTestingCallback callback) override {
CHECK(false) << "Flush for testing called on an unbound interface.";
}
void SetFocus(bool focused) override { void SetFocus(bool focused) override {
DLOG(WARNING) << "Input request on unbound interface"; DLOG(WARNING) << "Input request on unbound interface";
} }
...@@ -981,6 +985,12 @@ void RenderWidgetHostImpl::Blur() { ...@@ -981,6 +985,12 @@ void RenderWidgetHostImpl::Blur() {
focused_widget->SetPageFocus(false); focused_widget->SetPageFocus(false);
} }
void RenderWidgetHostImpl::FlushForTesting(FlushForTestingCallback callback) {
// Flush IPC messages on WidgetInputHandler to ensure that SetFocus() messages
// have been processed.
GetWidgetInputHandler()->FlushForTesting(std::move(callback));
}
void RenderWidgetHostImpl::SetPageFocus(bool focused) { void RenderWidgetHostImpl::SetPageFocus(bool focused) {
is_focused_ = focused; is_focused_ = focused;
......
...@@ -187,6 +187,7 @@ class CONTENT_EXPORT RenderWidgetHostImpl ...@@ -187,6 +187,7 @@ class CONTENT_EXPORT RenderWidgetHostImpl
void NotifyTextDirection() override; void NotifyTextDirection() override;
void Focus() override; void Focus() override;
void Blur() override; void Blur() override;
void FlushForTesting(FlushForTestingCallback callback) override;
void SetActive(bool active) override; void SetActive(bool active) override;
void ForwardMouseEvent(const blink::WebMouseEvent& mouse_event) override; void ForwardMouseEvent(const blink::WebMouseEvent& mouse_event) override;
void ForwardWheelEvent(const blink::WebMouseWheelEvent& wheel_event) override; void ForwardWheelEvent(const blink::WebMouseWheelEvent& wheel_event) override;
......
...@@ -198,6 +198,10 @@ interface WidgetInputHandlerHost { ...@@ -198,6 +198,10 @@ interface WidgetInputHandlerHost {
// an input interface for an associated Widget object. See FrameInputHandler // an input interface for an associated Widget object. See FrameInputHandler
// for an interface at the frame level. // for an interface at the frame level.
interface WidgetInputHandler { interface WidgetInputHandler {
// A simple mechanism by which the caller can ensure that all previous
// messages have been flushed.
FlushForTesting() => ();
// Tells widget focus has been changed. // Tells widget focus has been changed.
SetFocus(bool focused); SetFocus(bool focused);
......
...@@ -165,6 +165,10 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender { ...@@ -165,6 +165,10 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender {
virtual void Focus() = 0; virtual void Focus() = 0;
virtual void Blur() = 0; virtual void Blur() = 0;
// Tests may need to flush IPCs to ensure deterministic behavior.
using FlushForTestingCallback = base::OnceClosure;
virtual void FlushForTesting(FlushForTestingCallback callback) = 0;
// Sets whether the renderer should show controls in an active state. On all // Sets whether the renderer should show controls in an active state. On all
// platforms except mac, that's the same as focused. On mac, the frontmost // platforms except mac, that's the same as focused. On mac, the frontmost
// window will show active controls even if the focus is not in the web // window will show active controls even if the focus is not in the web
......
...@@ -67,6 +67,18 @@ void WidgetInputHandlerImpl::SetBinding( ...@@ -67,6 +67,18 @@ void WidgetInputHandlerImpl::SetBinding(
base::BindOnce(&WidgetInputHandlerImpl::Release, base::Unretained(this))); base::BindOnce(&WidgetInputHandlerImpl::Release, base::Unretained(this)));
} }
void WidgetInputHandlerImpl::FlushForTesting(FlushForTestingCallback callback) {
auto run_on_this_thread = base::BindOnce(
[](scoped_refptr<base::SingleThreadTaskRunner> task_runner,
FlushForTestingCallback callback) {
task_runner->PostTask(FROM_HERE, std::move(callback));
},
base::ThreadTaskRunnerHandle::Get(), std::move(callback));
// Hop to main thread to ensure everything there is flushed.
RunOnMainThread(std::move(run_on_this_thread));
}
void WidgetInputHandlerImpl::SetFocus(bool focused) { void WidgetInputHandlerImpl::SetFocus(bool focused) {
RunOnMainThread( RunOnMainThread(
base::BindOnce(&RenderWidget::OnSetFocus, render_widget_, focused)); base::BindOnce(&RenderWidget::OnSetFocus, render_widget_, focused));
......
...@@ -32,6 +32,7 @@ class WidgetInputHandlerImpl : public mojom::WidgetInputHandler { ...@@ -32,6 +32,7 @@ class WidgetInputHandlerImpl : public mojom::WidgetInputHandler {
mojom::WidgetInputHandlerAssociatedRequest interface_request); mojom::WidgetInputHandlerAssociatedRequest interface_request);
void SetBinding(mojom::WidgetInputHandlerRequest interface_request); void SetBinding(mojom::WidgetInputHandlerRequest interface_request);
void FlushForTesting(FlushForTestingCallback callback) override;
void SetFocus(bool focused) override; void SetFocus(bool focused) override;
void MouseCaptureLost() override; void MouseCaptureLost() override;
void SetEditCommandsForNextKeyEvent( void SetEditCommandsForNextKeyEvent(
......
...@@ -408,6 +408,14 @@ bool BlinkTestController::PrepareForLayoutTest( ...@@ -408,6 +408,14 @@ bool BlinkTestController::PrepareForLayoutTest(
if (is_devtools_js_test) { if (is_devtools_js_test) {
LoadDevToolsJSTest(); LoadDevToolsJSTest();
} else { } else {
// Flush IPC messages on the widget.
base::RunLoop run_loop;
main_window_->web_contents()
->GetRenderViewHost()
->GetWidget()
->FlushForTesting(run_loop.QuitClosure());
run_loop.Run();
// Loading the URL will immediately start the layout test. Manually call // Loading the URL will immediately start the layout test. Manually call
// LoadURLWithParams on the WebContents to avoid extraneous calls from // LoadURLWithParams on the WebContents to avoid extraneous calls from
// content::Shell such as SetFocus(), which could race with the layout // content::Shell such as SetFocus(), which could race with the layout
......
...@@ -26,6 +26,10 @@ MockWidgetInputHandler::MockWidgetInputHandler( ...@@ -26,6 +26,10 @@ MockWidgetInputHandler::MockWidgetInputHandler(
MockWidgetInputHandler::~MockWidgetInputHandler() {} MockWidgetInputHandler::~MockWidgetInputHandler() {}
void MockWidgetInputHandler::FlushForTesting(FlushForTestingCallback callback) {
std::move(callback).Run();
}
void MockWidgetInputHandler::SetFocus(bool focused) { void MockWidgetInputHandler::SetFocus(bool focused) {
dispatched_messages_.emplace_back( dispatched_messages_.emplace_back(
std::make_unique<DispatchedFocusMessage>(focused)); std::make_unique<DispatchedFocusMessage>(focused));
......
...@@ -192,6 +192,7 @@ class MockWidgetInputHandler : public mojom::WidgetInputHandler { ...@@ -192,6 +192,7 @@ class MockWidgetInputHandler : public mojom::WidgetInputHandler {
}; };
// mojom::WidgetInputHandler override. // mojom::WidgetInputHandler override.
void FlushForTesting(FlushForTestingCallback callback) override;
void SetFocus(bool focused) override; void SetFocus(bool focused) override;
void MouseCaptureLost() override; void MouseCaptureLost() override;
void SetEditCommandsForNextKeyEvent( void SetEditCommandsForNextKeyEvent(
......
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