Commit 2b3f2a52 authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

Allow sudden termination of renderers that are stuck in beforeunload.

The beforeunload timer in RenderFrameHostImpl may take a long time to
actually run if there are other tasks queued before it in the ui thread
task runner. This change allows sudden termination in the case where
the user tries to close a tab, the timeout fails to fire quickly, and
the user tries to close the tab again.

This also skips the close event if beforeunload has already timed out
(not allowing the renderer another chance to time out).

This allows the user to close, in relatively short order, tabs that
run the following:

  while (true) console.log("bar");

Whether that's in a general context, in the beforeunload handler,
or both.

Bug: 1056257
Change-Id: Idf1242dab5f06e347cf5fc2ca1019255f0da774e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2092123Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748353}
parent af0b4e55
......@@ -3040,6 +3040,13 @@ bool RenderFrameHostImpl::IsWaitingForUnloadACK() const {
is_waiting_for_unload_ack_;
}
bool RenderFrameHostImpl::BeforeUnloadTimedOut() const {
return beforeunload_timeout_ &&
(send_before_unload_start_time_ != base::TimeTicks()) &&
(base::TimeTicks::Now() - send_before_unload_start_time_) >
beforeunload_timeout_delay_;
}
void RenderFrameHostImpl::OnUnloadACK() {
if (frame_tree_node_->render_manager()->is_attaching_inner_delegate()) {
// This RFH was unloaded while attaching an inner delegate. The RFH
......
......@@ -642,6 +642,11 @@ class CONTENT_EXPORT RenderFrameHostImpl
return is_waiting_for_beforeunload_completion_;
}
// True if more than |beforeunload_timeout_delay_| has elapsed since starting
// beforeunload. This may be true before |beforeunload_timeout_| actually
// fires, as the task can be delayed by task scheduling. See crbug.com/1056257
bool BeforeUnloadTimedOut() const;
// Whether the RFH is waiting for an unload ACK from the renderer.
bool IsWaitingForUnloadACK() const;
......
......@@ -701,12 +701,7 @@ void RenderViewHostImpl::DispatchRenderViewCreated() {
void RenderViewHostImpl::ClosePage() {
is_waiting_for_page_close_completion_ = true;
bool is_javascript_dialog_showing = delegate_->IsJavaScriptDialogShowing();
// If there is a JavaScript dialog up, don't bother sending the renderer the
// close event because it is known unresponsive, waiting for the reply from
// the dialog.
if (IsRenderViewLive() && !is_javascript_dialog_showing) {
if (IsRenderViewLive() && !SuddenTerminationAllowed()) {
close_timeout_->Start(TimeDelta::FromMilliseconds(kUnloadTimeoutMS));
// TODO(creis): Should this be moved to Shutdown? It may not be called for
......@@ -815,8 +810,14 @@ void RenderViewHostImpl::RenderWidgetDidFirstVisuallyNonEmptyPaint() {
delegate_->DidFirstVisuallyNonEmptyPaint(this);
}
bool RenderViewHostImpl::SuddenTerminationAllowed() const {
return sudden_termination_allowed_;
bool RenderViewHostImpl::SuddenTerminationAllowed() {
// If there is a JavaScript dialog up, don't bother sending the renderer the
// close event because it is known unresponsive, waiting for the reply from
// the dialog.
return sudden_termination_allowed_ ||
delegate_->IsJavaScriptDialogShowing() ||
static_cast<RenderFrameHostImpl*>(GetMainFrame())
->BeforeUnloadTimedOut();
}
///////////////////////////////////////////////////////////////////////////////
......
......@@ -186,7 +186,7 @@ class CONTENT_EXPORT RenderViewHostImpl
// Tells the renderer view to focus the first (last if reverse is true) node.
void SetInitialFocus(bool reverse);
bool SuddenTerminationAllowed() const;
bool SuddenTerminationAllowed();
void set_sudden_termination_allowed(bool enabled) {
sudden_termination_allowed_ = enabled;
}
......
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