This bug needs to be fixed and merged back within the next week or it will start blocking our R68 stable release.

I don't see a connection with the merge of the beforeunload fix in r575133, but maybe Alex can spot one?

Doesn't seem like it could have been clamy@'s r574553, since that landed in 69.0.3491.0 and wasn't merged to M68.

There might be a connection.  Looking at some crash dumps, I see that the navigating frame's RFHI becomes nullptr somewhere along the road from entering RFHI::DidCommitProvisionalLoad() to the NotifySwappedFromRenderManager() call in RFHM::CommitPending().  This is also happening in the main frame.  The most likely explanation is that in CommitPending(), slightly before NotifySwappedFromRenderManager() we call frame_tree_node_->ResetForNewProcess().  As part of that, a subframe's destruction might trigger a beforeunload ACK to an ancestor frame, which we know from  issue 866382  can cause a reentrant call to TabStripModel::CloseWebContentses() (if there's a pending tab close in parallel with the navigation), which might destroy the WebContentsImpl, RFHM, and main frame RFHI.  After we unwind the stack, we'll come back to CommitPending() with the freed WebContentsImpl/RFHM/RFHI (which is what I see in the crash dumps with |this| being null), and crash when we try to dereference something -- in this case, this is probably new_host->GetRenderViewHost() in NotifySwappedFromRenderManager(), where |new_host| is null.

I think the fix for  issue 866382  of scheduling the beforeunload ACK when destroying a subframe's RFHI should fix this issue as well.

Proactively marking Restrict-View-Security given the discussion of returning to freed objects, in case there's a UaF here.  Alex thinks the repro steps at least require user interaction, which would be a mitigating factor if so.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe000eecfd314aff088f7d981f96b19a80a20515

commit fe000eecfd314aff088f7d981f96b19a80a20515
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Wed Jul 25 00:01:13 2018

Schedule OnBeforeUnloadACK to prevent reentrancy when destroying a RFHI.

Previously, it was possible that in the middle of closing a tab and
shutting down the main frame's process, we could call
ResetForNewProcess() to clear the main frame's children, but
destroying a child's RenderFrameHostImpl could reenter
TabStripModel::CloseWebContentses if the main frame was also waiting
for that child's beforeunload ACK.  This could result in freeing state
such as the WebContents or main frame's RFHM and RFHI, and then later
unwinding the stack to proceed with the first CloseWebContentses and
running into crashes when trying to dereference pointers which were
already freed or nulled out.

To prevent this reentrancy, schedule the beforeunload ACK in the cases
where it's used for tab close.  This is similar to a fix for issue
851400, but addresses a new way to get the reentrant calls that became
possible after r575133.

Bug:  866382 ,  866365 
Change-Id: Id20068cb24f457f1fae1005050d46441dac78d9a
Reviewed-on: https://chromium-review.googlesource.com/1148775
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577736}
[modify] https://crrev.com/fe000eecfd314aff088f7d981f96b19a80a20515/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/fe000eecfd314aff088f7d981f96b19a80a20515/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/fe000eecfd314aff088f7d981f96b19a80a20515/content/browser/frame_host/render_frame_host_impl.h

This appears to be fixed by r577736.  We're monitoring the crashes and will request merge in  issue 866382 .

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/838874422ea8a995a120ac284e386b209ce4c533

commit 838874422ea8a995a120ac284e386b209ce4c533
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Fri Jul 27 17:22:48 2018

Schedule OnBeforeUnloadACK to prevent reentrancy when destroying a RFHI (Merge to M69)

Previously, it was possible that in the middle of closing a tab and
shutting down the main frame's process, we could call
ResetForNewProcess() to clear the main frame's children, but
destroying a child's RenderFrameHostImpl could reenter
TabStripModel::CloseWebContentses if the main frame was also waiting
for that child's beforeunload ACK.  This could result in freeing state
such as the WebContents or main frame's RFHM and RFHI, and then later
unwinding the stack to proceed with the first CloseWebContentses and
running into crashes when trying to dereference pointers which were
already freed or nulled out.

To prevent this reentrancy, schedule the beforeunload ACK in the cases
where it's used for tab close.  This is similar to a fix for issue
851400, but addresses a new way to get the reentrant calls that became
possible after r575133.

TBR=alexmos@chromium.org

(cherry picked from commit fe000eecfd314aff088f7d981f96b19a80a20515)

Bug:  866382 ,  866365 
Change-Id: Id20068cb24f457f1fae1005050d46441dac78d9a
Reviewed-on: https://chromium-review.googlesource.com/1148775
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577736}
Reviewed-on: https://chromium-review.googlesource.com/1153366
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#162}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/838874422ea8a995a120ac284e386b209ce4c533/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/838874422ea8a995a120ac284e386b209ce4c533/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/838874422ea8a995a120ac284e386b209ce4c533/content/browser/frame_host/render_frame_host_impl.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3cccf1a40279cf1f5d789358f80c15deb6c884c2

commit 3cccf1a40279cf1f5d789358f80c15deb6c884c2
Author: Charlie Reis <creis@chromium.org>
Date: Fri Jul 27 18:31:35 2018

Merge to M68: Schedule OnBeforeUnloadACK to prevent reentrancy when destroying a RFHI.

Previously, it was possible that in the middle of closing a tab and
shutting down the main frame's process, we could call
ResetForNewProcess() to clear the main frame's children, but
destroying a child's RenderFrameHostImpl could reenter
TabStripModel::CloseWebContentses if the main frame was also waiting
for that child's beforeunload ACK.  This could result in freeing state
such as the WebContents or main frame's RFHM and RFHI, and then later
unwinding the stack to proceed with the first CloseWebContentses and
running into crashes when trying to dereference pointers which were
already freed or nulled out.

To prevent this reentrancy, schedule the beforeunload ACK in the cases
where it's used for tab close.  This is similar to a fix for issue
851400, but addresses a new way to get the reentrant calls that became
possible after r575133.

Bug:  866382 ,  866365 
Change-Id: Id20068cb24f457f1fae1005050d46441dac78d9a
Reviewed-on: https://chromium-review.googlesource.com/1148775
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577736}
Reviewed-on: https://chromium-review.googlesource.com/1153223
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#761}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/3cccf1a40279cf1f5d789358f80c15deb6c884c2/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/3cccf1a40279cf1f5d789358f80c15deb6c884c2/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/3cccf1a40279cf1f5d789358f80c15deb6c884c2/content/browser/frame_host/render_frame_host_impl.h

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot