GitLab

When the user closes a tab (or browser), we first determine whether we
can take a "fast shutdown" path and simply shut down the main frame
process.  That can be done if nothing in the current tab (or all tabs
if closing the browser) needs to run beforeunload/unload handlers.
Otherwise, we take the slow path, actually try to run those handlers
in all the frames, and wait for the results (including beforeunload
dialog dismissals) before proceeding with the close.

Historically, this was done by checking SuddenTerminationAllowed() on
the main frame's process, which would be set to true when *any* frame
in that process adds a beforeunload or unload handler.  When site
isolation shipped, we also added code to consult beforeunload handlers
in cross-site iframes (see issue https://crbug.com/853021) in M68.

But SuddenTerminationAllowed() is actually too coarse-grained for
this: it not only becomes false due to (before)unload handlers, but
other things as well (such as BlobBytesProvider).  So
https://chromium-review.googlesource.com/c/chromium/src/+/1504130
changed things to explicitly consult the main frame's
beforeunload/unload handler status, as tracked by
RFH::GetSuddenTerminationDisablerState(), instead of
SuddenTerminationAllowed().  Unfortunately, it seems that in that CL
we forgot about the process-wide nature of SuddenTerminationAllowed(),
specifically that it also covered handlers from any subframes in the
same process.  And/or, we missed that the call in
WebContentsImpl::NeedToFireBeforeUnload() to check subframes via
GetMainFrame()->ShouldDispatchBeforeUnload( true /*
check_subframes_only */); was in fact only checking cross-site
iframes, skipping not only the main frame but also its same-site
descendants.

That means we would incorrectly fast-close the tab (or browser) even
if a same-site iframe had a beforeunload handler.  This CL fixes this
by walking the full frame tree to check for these handlers and adds
two regression tests for the tab close and browser close cases.

Along the way, the CL also fixes a TODO to check not only beforeunload
but also unload handlers in subframes.  This is a bit more correct and
will also make it easier to fix issue 1014550, though note that
subframe unload handlers weren't broken before -- they actually still
ran due to the subframe unload process timeout that we still have in
RenderProcessHostImpl::DelayProcessShutdownForUnload().

Bug: 1010456, 1014550
Change-Id: I2b2c3e2ee546a766e5838e14f293266def4aeeed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1881948
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711413}