Skip to content

Commit

Permalink
[RenderDocument] Flush synthetic events before web test start
Browse files Browse the repository at this point in the history
Enabling RenderDocument causes same-origin navigations to swap to a new
RenderFrameHost. However, when a RenderFrameHost is changed, the browser
dispatches synthetic mouse events to ensure the renderer knows where the
mouse cursor is. In web tests using the event_sender API, events are
injected into Blink synchronously so the browser-generated synthetic
events may be processed after test events which can affect test output.

This CL changes the web test runner to ensure all synthetic events have
been flushed and processed before the test is started. It does this by
blocking parsing in a web test until input has been flushed, the order
of events from the perspective of WebTestControlHost:

  * Initial-state/Test Finished: Set a `next_nav_is_new_test` bit.
  * READY_TO_COMMIT: If `next_nav_is_new_test` - call
    BlockTestUntilStart in the renderer which blocks parsing in the
    current DocumentLoader, before it starts loading the new document.
  * <During CommitPending>: Post a task to generate the synthetic
    events.
  * DID_FINISH_NAVIGATION: If `next_nav_is_new_test` - clear the bit and
    post a task to:
      * Flush input - this waits until all input has been IPC'd to the
        renderer and ACK'd. Note: input may still be queued in
        renderer-side compositor or main thread queues.
      * Calls StartTest on the renderer.
  * StartTest (in renderer)
    * Flushes the compositor queue to ensure all its input has been
      processed (this may result in queueing input on the main thread).
    * Flushes the main thread event queue.
    * Unblocks the parser.

Bug: 1416496
Change-Id: I2ac0c3745e74a0a01055d5e7a474036096b8ec6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4727485
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1188325}
  • Loading branch information
bokand authored and Chromium LUCI CQ committed Aug 25, 2023
1 parent c915e22 commit 4d8331c
Show file tree
Hide file tree
Showing 18 changed files with 232 additions and 16 deletions.
4 changes: 4 additions & 0 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4626,6 +4626,10 @@ const RenderFrameImpl* RenderFrameImpl::GetLocalRoot() const {
: RenderFrameImpl::FromWebFrame(frame_->LocalRoot());
}

base::WeakPtr<RenderFrameImpl> RenderFrameImpl::GetWeakPtr() {
return weak_factory_.GetWeakPtr();
}

mojom::DidCommitProvisionalLoadParamsPtr
RenderFrameImpl::MakeDidCommitProvisionalLoadParams(
blink::WebHistoryCommitType commit_type,
Expand Down
2 changes: 2 additions & 0 deletions content/renderer/render_frame_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,8 @@ class CONTENT_EXPORT RenderFrameImpl
bool IsLocalRoot() const;
const RenderFrameImpl* GetLocalRoot() const;

base::WeakPtr<RenderFrameImpl> GetWeakPtr();

private:
friend class RenderFrameImplTest;
friend class RenderFrameObserver;
Expand Down
72 changes: 56 additions & 16 deletions content/web_test/browser/web_test_control_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,13 @@ void WebTestControlHost::ReadyToCommitNavigation(
request->GetRenderFrameHostRestoredFromBackForwardCache();
if (rfh)
GetWebTestRenderFrameRemote(rfh)->OnReactivated();

if (navigation_handle->IsInPrimaryMainFrame() &&
next_non_blank_nav_is_new_test_ &&
navigation_handle->GetURL() != GURL(kAboutBlankResetWebTest)) {
GetWebTestRenderFrameRemote(navigation_handle->GetRenderFrameHost())
->BlockTestUntilStart();
}
}

void WebTestControlHost::RenderProcessHostDestroyed(
Expand Down Expand Up @@ -1886,26 +1893,59 @@ void WebTestControlHost::PrepareRendererForNextWebTest() {
// |WebTestControlHost::DidFinishNavigation|.
}

void WebTestControlHost::DidFinishNavigation(NavigationHandle* navigation) {
if (navigation->GetURL() != GURL(kAboutBlankResetWebTest))
void WebTestControlHost::FlushInputAndStartTest(WeakDocumentPtr doc) {
RenderFrameHost* rfh = doc.AsRenderFrameHostIfValid();
if (!rfh) {
return;
}

// During fuzzing, the |main_window_| might close itself using window.close().
// This might happens after the end of the test, during the cleanup phase. In
// this case, the pending about:blank navigation might be canceled, within the
// |main_window_| destructor. It is no longer safe to access |main_window_|
// here. See https://crbug.com/1221183
if (!navigation->HasCommitted())
return;
// Ensures any synthetic input (e.g. mouse enter/leave/move events as a
// result of navigation) have been handled by the renderer.
rfh->GetRenderWidgetHost()->FlushForTesting();
GetWebTestRenderFrameRemote(rfh)->StartTest();
}

// Request additional web test specific cleanup in the renderer process:
content::WebContentsImpl* web_contents =
static_cast<WebContentsImpl*>(main_window_->web_contents());
RenderProcessHost* main_rfh_process =
web_contents->GetPrimaryMainFrame()->GetProcess();
GetWebTestRenderThreadRemote(main_rfh_process)->ResetRendererAfterWebTest();
void WebTestControlHost::DidFinishNavigation(NavigationHandle* navigation) {
if (navigation->GetURL() == GURL(kAboutBlankResetWebTest)) {
// During fuzzing, the |main_window_| might close itself using
// window.close(). This might happens after the end of the test, during the
// cleanup phase. In this case, the pending about:blank navigation might be
// canceled, within the |main_window_| destructor. It is no longer safe to
// access |main_window_| here. See https://crbug.com/1221183
if (!navigation->HasCommitted()) {
return;
}

next_non_blank_nav_is_new_test_ = true;

PrepareRendererForNextWebTestDone();
// Request additional web test specific cleanup in the renderer process:
content::WebContentsImpl* web_contents =
static_cast<WebContentsImpl*>(main_window_->web_contents());
RenderProcessHost* main_rfh_process =
web_contents->GetPrimaryMainFrame()->GetProcess();
GetWebTestRenderThreadRemote(main_rfh_process)->ResetRendererAfterWebTest();

PrepareRendererForNextWebTestDone();
} else if (navigation->IsInPrimaryMainFrame() &&
!navigation->GetURL().IsAboutBlank() &&
next_non_blank_nav_is_new_test_) {
next_non_blank_nav_is_new_test_ = false;

if (navigation->HasCommitted()) {
// If the browser is injecting synthetic mouse moves, it does so at
// CommitPending time by posting a task to perform the dispatch. Hence,
// that task must already be queued (or complete) by this time. Post the
// flush input task to ensure it runs after the synthetic mouse event
// dispatch task. See comments on next_non_blank_nav_is_new_test_ for
// more details.
base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(
&WebTestControlHost::FlushInputAndStartTest,
weak_factory_.GetWeakPtr(),
navigation->GetRenderFrameHost()->GetWeakDocumentPtr()));
}
}
}

void WebTestControlHost::PrepareRendererForNextWebTestDone() {
Expand Down
14 changes: 14 additions & 0 deletions content/web_test/browser/web_test_control_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ class WebTestControlHost : public WebContentsObserver,
override;

void DiscardMainWindow();
void FlushInputAndStartTest(WeakDocumentPtr rfh);
// Closes all windows opened by the test. This is every window but the main
// window, since it is created by the test harness and reused between tests.
void CloseTestOpenedWindows();
Expand Down Expand Up @@ -456,6 +457,19 @@ class WebTestControlHost : public WebContentsObserver,
NextPointerLockAction next_pointer_lock_action_ =
NextPointerLockAction::kWillSucceed;

// When navigating to a new web test, the control host blocks the renderer
// from starting the test while some initialization to a known state occurs
// (e.g. flushing synthetic input events associated with navigation).
//
// It does so by resetting this bit to `true` at the end of each web test.
// When this bit is set, the next ReadyToCommitNavigation seen will call
// BlockTestUntilStart in the renderer to block the renderer parser,
// preventing the test from running. When that navigation finishes in
// DidFinishNavigation, this bit is turned off and this class performs
// initialization. Once the initialization is complete, StartTest is run in
// the renderer, unblocking the parser.
bool next_non_blank_nav_is_new_test_ = true;

SEQUENCE_CHECKER(sequence_checker_);

base::WeakPtrFactory<WebTestControlHost> weak_factory_{this};
Expand Down
9 changes: 9 additions & 0 deletions content/web_test/common/web_test.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,15 @@ interface WebTestRenderFrame {
// Notifies a frame that has been restored from backward/forward cache.
// This is called on main frames to add them to TestRunner's management list.
OnReactivated();

// Prevents a new test from being run by blocking parsing. Parsing will
// resume when StartTest is called.
BlockTestUntilStart();

// Notifies the frame that the browser has fully completed any
// potentially-renderer-visible changes it needed to make. The frame may now
// perform any initialization and start running the test.
StartTest();
};

// Web test messages related runtime flags sent from the browser process to the
Expand Down
31 changes: 31 additions & 0 deletions content/web_test/renderer/web_frame_test_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,20 @@ void WebFrameTestProxy::DidClearWindowObject() {
RenderFrameImpl::DidClearWindowObject();
}

void WebFrameTestProxy::DidCommitNavigation(
blink::WebHistoryCommitType commit_type,
bool should_reset_browser_interface_broker,
const blink::ParsedPermissionsPolicy& permissions_policy_header,
const blink::DocumentPolicyFeatureState& document_policy_header) {
if (should_block_parsing_in_next_commit_) {
should_block_parsing_in_next_commit_ = false;
GetWebFrame()->BlockParserForTesting();
}
RenderFrameImpl::DidCommitNavigation(
commit_type, should_reset_browser_interface_broker,
permissions_policy_header, document_policy_header);
}

void WebFrameTestProxy::OnDeactivated() {
test_runner()->OnFrameDeactivated(this);
}
Expand All @@ -759,6 +773,23 @@ void WebFrameTestProxy::OnReactivated() {
test_runner()->OnFrameReactivated(this);
}

void WebFrameTestProxy::BlockTestUntilStart() {
should_block_parsing_in_next_commit_ = true;
}

void WebFrameTestProxy::StartTest() {
CHECK(!should_block_parsing_in_next_commit_);
GetWebFrame()->FlushInputForTesting(base::BindOnce(
[](base::WeakPtr<RenderFrameImpl> render_frame) {
if (!render_frame || !render_frame->GetWebFrame()) {
return;
}

render_frame->GetWebFrame()->ResumeParserForTesting();
},
GetWeakPtr()));
}

blink::FrameWidgetTestHelper*
WebFrameTestProxy::GetLocalRootFrameWidgetTestHelper() {
return GetLocalRootWebFrameWidget()->GetFrameWidgetTestHelperForTesting();
Expand Down
11 changes: 11 additions & 0 deletions content/web_test/renderer/web_frame_test_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ class WebFrameTestProxy : public RenderFrameImpl,
const blink::WebString& sink_id,
blink::WebSetSinkIdCompleteCallback completion_callback) override;
void DidClearWindowObject() override;
void DidCommitNavigation(
blink::WebHistoryCommitType commit_type,
bool should_reset_browser_interface_broker,
const blink::ParsedPermissionsPolicy& permissions_policy_header,
const blink::DocumentPolicyFeatureState& document_policy_header) override;

// mojom::WebTestRenderFrame implementation.
void SynchronouslyCompositeAfterTest(
Expand All @@ -93,6 +98,8 @@ class WebFrameTestProxy : public RenderFrameImpl,
bool starting_test) override;
void OnDeactivated() override;
void OnReactivated() override;
void BlockTestUntilStart() override;
void StartTest() override;

private:
void BindReceiver(
Expand All @@ -115,6 +122,10 @@ class WebFrameTestProxy : public RenderFrameImpl,

mojo::AssociatedReceiver<mojom::WebTestRenderFrame>
web_test_render_frame_receiver_{this};

// Prevents parsing on the next committed document. This is used to stop a
// test from running until StartTest() is called.
bool should_block_parsing_in_next_commit_ = false;
};

} // namespace content
Expand Down
10 changes: 10 additions & 0 deletions third_party/blink/public/web/web_local_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,16 @@ class BLINK_EXPORT WebLocalFrame : public WebFrame {
virtual void SetResourceCacheRemote(
CrossVariantMojoRemote<mojom::ResourceCacheInterfaceBase> remote) = 0;

// Used to block and resume parsing of the current document in the frame.
virtual void BlockParserForTesting() {}
virtual void ResumeParserForTesting() {}

// Processes all pending input in the widget associated with this frame.
// This is an asynchronous operation since it processes the compositor queue
// as well. The passed closure is invoked when queues of both threads have
// been processed.
virtual void FlushInputForTesting(base::OnceClosure) {}

protected:
explicit WebLocalFrame(mojom::TreeScopeType scope,
const LocalFrameToken& frame_token)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3792,6 +3792,11 @@ void WebFrameWidgetImpl::DidNavigate() {
widget_base_->widget_input_handler_manager()->DidNavigate();
}

void WebFrameWidgetImpl::FlushInputForTesting(base::OnceClosure done_callback) {
widget_base_->widget_input_handler_manager()->FlushEventQueuesForTesting(
std::move(done_callback));
}

void WebFrameWidgetImpl::SetMouseCapture(bool capture) {
if (mojom::blink::WidgetInputHandlerHost* host =
widget_base_->widget_input_handler_manager()
Expand Down
4 changes: 4 additions & 0 deletions third_party/blink/renderer/core/frame/web_frame_widget_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,10 @@ class CORE_EXPORT WebFrameWidgetImpl
// Called when the main frame navigates.
void DidNavigate();

// Ensures all queued input in the widget has been processed and the queues
// emptied.
void FlushInputForTesting(base::OnceClosure done_callback);

// Called when the widget should get targeting input.
void SetMouseCapture(bool capture);

Expand Down
21 changes: 21 additions & 0 deletions third_party/blink/renderer/core/frame/web_local_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3276,6 +3276,27 @@ void WebLocalFrameImpl::SetResourceCacheRemote(
GetFrame()->SetResourceCacheRemote(std::move(remote));
}

void WebLocalFrameImpl::BlockParserForTesting() {
// Avoid blocking for MHTML tests since MHTML archives are loaded
// synchronously during commit. WebFrameTestProxy only has a chance to act at
// DidCommit after that's happened.
if (GetFrame()->Loader().GetDocumentLoader()->Archive()) {
return;
}
GetFrame()->Loader().GetDocumentLoader()->BlockParser();
}

void WebLocalFrameImpl::ResumeParserForTesting() {
if (GetFrame()->Loader().GetDocumentLoader()->Archive()) {
return;
}
GetFrame()->Loader().GetDocumentLoader()->ResumeParser();
}

void WebLocalFrameImpl::FlushInputForTesting(base::OnceClosure done_callback) {
frame_widget_->FlushInputForTesting(std::move(done_callback));
}

void WebLocalFrameImpl::SetTargetToCurrentHistoryItem(const WebString& target) {
current_history_item_->SetTarget(target);
}
Expand Down
3 changes: 3 additions & 0 deletions third_party/blink/renderer/core/frame/web_local_frame_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,9 @@ class CORE_EXPORT WebLocalFrameImpl final
void SetResourceCacheRemote(
CrossVariantMojoRemote<mojom::blink::ResourceCacheInterfaceBase> remote)
override;
void BlockParserForTesting() override;
void ResumeParserForTesting() override;
void FlushInputForTesting(base::OnceClosure done_callback) override;

// WebNavigationControl overrides:
bool DispatchBeforeUnloadEvent(bool) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,14 @@ void InputHandlerProxy::UpdateBrowserControlsState(
input_handler_->UpdateBrowserControlsState(constraints, current, animate);
}

void InputHandlerProxy::FlushQueuedEventsForTesting() {
// The queue is blocked while there's a ScrollBegin hit test in progress.
CHECK(!scroll_begin_main_thread_hit_test_reasons_);

DispatchQueuedInputEvents(/*frame_aligned=*/true);
CHECK(compositor_event_queue_->empty());
}

void InputHandlerProxy::HandleOverscroll(
const gfx::PointF& causal_event_viewport_point,
const cc::InputHandlerScrollResult& scroll_result) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,9 @@ class PLATFORM_EXPORT InputHandlerProxy : public cc::InputHandlerClient,
return currently_active_gesture_device_.value();
}

// Immediately dispatches all queued events.
void FlushQueuedEventsForTesting();

private:
friend class test::TestInputHandlerProxy;
friend class test::InputHandlerProxyTest;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,11 @@ void MainThreadEventQueue::ClearRafFallbackTimerForTesting() {
raf_fallback_timer_.reset();
}

bool MainThreadEventQueue::IsEmptyForTesting() {
base::AutoLock lock(shared_state_lock_);
return shared_state_.events_.empty();
}

void MainThreadEventQueue::DispatchRafAlignedInput(base::TimeTicks frame_time) {
if (raf_fallback_timer_)
raf_fallback_timer_->Stop();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ class PLATFORM_EXPORT MainThreadEventQueue
mojom::blink::InputEventResultState::kSetNonBlockingDueToFling;
}

// Acquires a lock but use is restricted to tests.
bool IsEmptyForTesting();

protected:
friend class base::RefCountedThreadSafe<MainThreadEventQueue>;
virtual ~MainThreadEventQueue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1170,4 +1170,38 @@ void WidgetInputHandlerManager::UpdateBrowserControlsState(
animate);
}

void WidgetInputHandlerManager::FlushCompositorQueueForTesting() {
CHECK(InputThreadTaskRunner()->BelongsToCurrentThread());
if (!input_handler_proxy_) {
return;
}
input_handler_proxy_->FlushQueuedEventsForTesting();
}

void WidgetInputHandlerManager::FlushMainThreadQueueForTesting(
base::OnceClosure done) {
CHECK(main_thread_task_runner_->BelongsToCurrentThread());
input_event_queue()->DispatchRafAlignedInput(base::TimeTicks::Now());
CHECK(input_event_queue()->IsEmptyForTesting());
std::move(done).Run();
}

void WidgetInputHandlerManager::FlushEventQueuesForTesting(
base::OnceClosure done_callback) {
CHECK(main_thread_task_runner_->BelongsToCurrentThread());

auto flush_compositor_queue = base::BindOnce(
&WidgetInputHandlerManager::FlushCompositorQueueForTesting, this);

auto flush_main_queue =
base::BindOnce(&WidgetInputHandlerManager::FlushMainThreadQueueForTesting,
this, std::move(done_callback));

// Flush the compositor queue first since dispatching compositor events may
// bounce them back into the main thread event queue.
InputThreadTaskRunner()->PostTaskAndReply(FROM_HERE,
std::move(flush_compositor_queue),
std::move(flush_main_queue));
}

} // namespace blink

0 comments on commit 4d8331c

Please sign in to comment.