Thanks!  There's some relevant discussion on issue 558680, where we also saw cases of this RFH_INVALID_ORIGIN_ON_COMMIT.  In fact, I heavily suspect FrameLoader::updateForSameDocumentNavigation, given the same page observation (see comment 7 of issue 558680).  updateForSameDocumentNavigation will blindly update Document::url and not Document::origin, if RenderFrame tells us this is supposed to be a same-page navigation.

Maybe we're getting some PageState mismatch that makes us do a cross-site navigation for something that was labeled same-page?

Users experienced this crash on the following builds:

Mac Dev 53.0.2785.8 -  0.64 CPM, 10 reports, 7 clients (signature [Renderer kill] content::RenderFrameHostImpl::OnDidCommitProvisionalLoad)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

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

commit 0be4d34d5c65243b03d4fc6469a78b32eb1403c0
Author: nasko <nasko@chromium.org>
Date: Mon Jul 18 23:43:43 2016

Add a check for mismatching URL and origin in the renderer process.

The renderer process kills with reason RFH_INVALID_ORIGIN_ON_COMMIT are
checking parameters passed from the renderer process. However, we are
lacking a bunch of information since it is in the browser process and
it is hard to reason about what the state was in the renderer.

We can also try to implement similar checking in the renderer process,
so we can catch these mismatches closer to where we have more data.
This CL implements some of the checks performed browser side, but
it is a set that should catch the majority of the cases we have seen
in crash reports.

BUG= 628677 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2151323003
Cr-Commit-Position: refs/heads/master@{#406145}

[modify] https://crrev.com/0be4d34d5c65243b03d4fc6469a78b32eb1403c0/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/0be4d34d5c65243b03d4fc6469a78b32eb1403c0/content/renderer/render_frame_impl.cc

Users experienced this crash on the following builds:

Win Dev 53.0.2785.21 -  0.23 CPM, 31 reports, 30 clients (signature [Renderer kill 114] content::RenderFrameHostImpl::OnDidCommitProvisionalLoad)
Win Canary 54.0.2803.0 -  0.27 CPM, 4 reports, 4 clients (signature content::RenderFrameImpl::SendDidCommitProvisionalLoad)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

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

commit e3e35c2d36325c10840b60d2a7cb2a7fe1c6cd14
Author: nasko <nasko@chromium.org>
Date: Sat Jul 23 00:25:34 2016

Add check for mismatching item and document sequence numbers.

When a PageState update is received from the renderer process, it must
be for the current session history entry. This means that the sequence
numbers associated with the FrameNavigationEntry should match the ones
coming from the renderer process.
This CL is adding explicit check for this and a DumpWithoutCrashing to
help diagnose what cases can cause a mismatch.

BUG= 628677 

Review-Url: https://codereview.chromium.org/2172123004
Cr-Commit-Position: refs/heads/master@{#407316}

[modify] https://crrev.com/e3e35c2d36325c10840b60d2a7cb2a7fe1c6cd14/content/browser/web_contents/web_contents_impl.cc

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

commit 10a855e46c9e920bcddd65b79f5047e35f1107ae
Author: nasko <nasko@chromium.org>
Date: Tue Jul 26 20:06:47 2016

Revert of Add check for mismatching item and document sequence numbers. (patchset #2 id:20001 of https://codereview.chromium.org/2172123004/ )

Reason for revert:
Reverting this change as it has resulted in much higher number of reports than anticipated. We have enough data to investigate and can reland a modified version if needed.

Original issue's description:
> Add check for mismatching item and document sequence numbers.
>
> When a PageState update is received from the renderer process, it must
> be for the current session history entry. This means that the sequence
> numbers associated with the FrameNavigationEntry should match the ones
> coming from the renderer process.
> This CL is adding explicit check for this and a DumpWithoutCrashing to
> help diagnose what cases can cause a mismatch.
>
> BUG= 628677 
>
> Committed: https://crrev.com/e3e35c2d36325c10840b60d2a7cb2a7fe1c6cd14
> Cr-Commit-Position: refs/heads/master@{#407316}

TBR=creis@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 628677 

Review-Url: https://codereview.chromium.org/2187633002
Cr-Commit-Position: refs/heads/master@{#407893}

[modify] https://crrev.com/10a855e46c9e920bcddd65b79f5047e35f1107ae/content/browser/web_contents/web_contents_impl.cc

Using the data from the various reports, we were able to find repro steps for a generic session history problem - putting PageState object on the wrong FrameNavigationEntry. I am not certain yet how to use this issue to repro the mismatched origin kill, but here is a scenario where the instrumentation from r407316 would hit:

1. Navigate to any simple page
2. Create iframe with name “foo” and add to main frame
3. Remove the “foo” iframe from the main frame
4. Create unnamed iframe and add to main frame
5. Create iframe named “foo” as a child of the previously created unnamed frame
6. Remove the innermost iframe

This results in finding the wrong FrameNavigationEntry and setting PageState on it. Attached is a simple repro file.

Deleted: repro.html 517 bytes

repro.html 517 bytes View Download

I ran into crash f927a53e00000000 a few minutes ago; I was on b/, command clicked the back button, it opened the new tab in the background then the tab died.  Three other tabs died simultaneously; some b/, some others.

Holler if you need more details.

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

commit 3463467767b4dbd5b0e52a1a0001cd6fb8288be4
Author: nasko <nasko@chromium.org>
Date: Fri Jul 29 18:46:06 2016

Add check for mismatching item and document sequence numbers.

When a PageState update is received from the renderer process, it must
be for the current session history entry. This means that the sequence
numbers associated with the FrameNavigationEntry should match the ones
coming from the renderer process.
This CL adds a check for this case and drops the update if mismatch is
found.

BUG= 628677 

Review-Url: https://codereview.chromium.org/2196623003
Cr-Commit-Position: refs/heads/master@{#408698}

[modify] https://crrev.com/3463467767b4dbd5b0e52a1a0001cd6fb8288be4/content/browser/web_contents/web_contents_impl.cc

Just to comment on the status, r408698 should prevent the corrupted PageState issue described in comment 8 by ignoring the state update (though we'll forget legitimate state for the new frame for the time being).  We hope this will prevent the RFH_INVALID_ORIGIN_ON_COMMIT kills.

Nasko's working on a more complete fix in https://codereview.chromium.org/2191543003/.  The core problem is that we leave FrameNavigationEntries in the NavigationEntry tree after the frame has been deleted (so that we restore the right URL if we leave and come back, with the frame resurrected).  If a new frame with the same unique name is added somewhere else in the tree, we can end up putting the PageState on the wrong FrameNavigationEntry, leading to the state corruption and thus renderer kill.  The fix will remove the stale FrameNavigationEntry when a new frame with the same unique name is added to the tree, to avoid ever having two entries for the same unique name.

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

commit be89fac7f6e0117dd8f12ae844a34565a9d25927
Author: nasko <nasko@chromium.org>
Date: Mon Aug 01 18:59:58 2016

Add parent pointer to NavigationEntryImpl::TreeNode.

Currently the tree of FrameNavigationEntries in NavigationEntryImpl does
not have any way for traversing from an arbitrary node in the tree up
to its parent. This is useful when comparing the expected chain of
ancestors, which is needed to fix  issue 628677 .

BUG= 628677 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2198903002
Cr-Commit-Position: refs/heads/master@{#409013}

[modify] https://crrev.com/be89fac7f6e0117dd8f12ae844a34565a9d25927/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/be89fac7f6e0117dd8f12ae844a34565a9d25927/content/browser/frame_host/navigation_entry_impl.h

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

commit 03ecfad9cb9219e03c87312150aaa5dd89eebc32
Author: nasko <nasko@chromium.org>
Date: Tue Aug 02 00:54:06 2016

Remove existing FrameNavigationEntry when new named frame is added.

FrameTreeNode is identified by a unique frame name, which allows us to
look up FrameNavigationEntry objects in NavigationEntry. However, when
a frame is removed from the page, the corresponding FrameNavigationEntry
is not removed. This is done intentionally to support back-forward
navigations in subframes and more intuitive UX on tab restore.

This behavior allows for more than one FrameNavigationEntry to exist in
the NavigationEntry with the same unique name, which can lead to corruption
of session history data. This CL changes the behavior to ensure that
when a new frame is added to the page, any existing entries with the
same unique name are deleted to avoid conflicts.

BUG= 628677 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2191543003
Cr-Commit-Position: refs/heads/master@{#409107}

[modify] https://crrev.com/03ecfad9cb9219e03c87312150aaa5dd89eebc32/content/browser/frame_host/frame_tree.cc
[modify] https://crrev.com/03ecfad9cb9219e03c87312150aaa5dd89eebc32/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/03ecfad9cb9219e03c87312150aaa5dd89eebc32/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/03ecfad9cb9219e03c87312150aaa5dd89eebc32/content/browser/frame_host/navigation_entry_impl.h

We have a lead for this (with repro steps) in issue 630103.  (These kills have been replaced by SendDidCommitProvisionalLoad crashes as of 54.0.2800.0, when r406145 landed.)

This crash has high impact on Chrome's stability.
Signature: content::RenderFrameImpl::SendDidCommitProvisionalLoad.
Channel: dev. Platform: android.
Labeling  issue 628677  with ReleaseBlock-Dev.


If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

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

commit af18219268a332a92b3c8bbaa306aade92142c55
Author: nasko <nasko@chromium.org>
Date: Thu Aug 11 02:12:01 2016

Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103,  628677 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2224213005
Cr-Commit-Position: refs/heads/master@{#411224}

[modify] https://crrev.com/af18219268a332a92b3c8bbaa306aade92142c55/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/af18219268a332a92b3c8bbaa306aade92142c55/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/af18219268a332a92b3c8bbaa306aade92142c55/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/af18219268a332a92b3c8bbaa306aade92142c55/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/af18219268a332a92b3c8bbaa306aade92142c55/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

r408698 has had more than enough time to bake on Canary and I believe it is safe to merge back to M53. 
r411224 is also useful to merge back, but hasn't had enough time on Canary yet.

Release PMs, we have lots of renderer kills with NC_AUTO_SUBFRAME in M52 and expected in M53, once it reaches stable. These two CLs will likely help get the rate down quite a bit.

Can I get approval for merging r408698?

Your change meets the bar and is auto-approved for M53 (branch: 2785)

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

commit f38836976725d366dbd0fb08054bea04f632ac59
Author: Nasko Oskov <nasko@chromium.org>
Date: Fri Aug 12 23:22:22 2016

Add check for mismatching item and document sequence numbers.

When a PageState update is received from the renderer process, it must
be for the current session history entry. This means that the sequence
numbers associated with the FrameNavigationEntry should match the ones
coming from the renderer process.
This CL adds a check for this case and drops the update if mismatch is
found.

BUG= 628677 

Review-Url: https://codereview.chromium.org/2196623003
Cr-Commit-Position: refs/heads/master@{#408698}
(cherry picked from commit 3463467767b4dbd5b0e52a1a0001cd6fb8288be4)

Review URL: https://codereview.chromium.org/2243443005 .

Cr-Commit-Position: refs/branch-heads/2785@{#578}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/f38836976725d366dbd0fb08054bea04f632ac59/content/browser/web_contents/web_contents_impl.cc

Tested the issue on Windows 7, Mac 10.11.6, Ubuntu 14.04 using 53.0.2785.70 with below steps:

1.Opened repro.html(in comment #8) in chrome.
2.Clicked on 'repro' button.
3.Not observed any crash.

Please find attached screenshot.

nasko@:Could you please confirm if this is the expected behavior.

Deleted: 628677.png 145 KB

	628677.png 145 KB View Download

Yes, you've tested one of the repro cases and it confirms it is fixed. However, there are more occurrences of this issue, so the bug cannot be closed quite yet.

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

commit fbed18057c8891791b821fe387d985a35c933e93
Author: nasko <nasko@chromium.org>
Date: Wed Aug 17 21:29:55 2016

Add crash keys to capture useful data for origin mismatch crashes.

The CHECK in RenderFrameImpl::SendDidCommitProvisionalLoad for mismatch
between origin and URL is still getting hit by users and higher frequency
on Android. This CL adds some of the data I found useful in debugging these
as crash keys, so the values from non-Windows platforms can be reviewed
without needing to process minidumps by hand.

BUG= 628677 , 630103
TBR=grt@chromium.org

Review-Url: https://codereview.chromium.org/2252153002
Cr-Commit-Position: refs/heads/master@{#412646}

[modify] https://crrev.com/fbed18057c8891791b821fe387d985a35c933e93/blimp/engine/app/blimp_engine_crash_keys.cc
[modify] https://crrev.com/fbed18057c8891791b821fe387d985a35c933e93/chrome/app/chrome_crash_reporter_client_win.cc
[modify] https://crrev.com/fbed18057c8891791b821fe387d985a35c933e93/chrome/common/crash_keys.cc
[modify] https://crrev.com/fbed18057c8891791b821fe387d985a35c933e93/content/renderer/render_frame_impl.cc

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

commit cf477b509f7a5a3409ac1f2aecf9513d7c82c5b3
Author: nasko <nasko@chromium.org>
Date: Sat Aug 20 23:55:15 2016

Don't mark navigations as in-page when DSNs are mismatched.

This CL implements a TODO by creis@ to ensure that the renderer doesn't
classify history navigations as in-page if the document sequence number
of the current document and the one being navigated to are mismatched.

BUG= 628677 , 630103

Review-Url: https://codereview.chromium.org/2253233002
Cr-Commit-Position: refs/heads/master@{#413353}

[modify] https://crrev.com/cf477b509f7a5a3409ac1f2aecf9513d7c82c5b3/content/renderer/render_frame_impl.cc

There are no crashes for now in the current canary release (54.0.2836.0) and as such, I think the overall issue is well understood - PageState corruption for in-page navigations. I am removing the blocking label for Isolate Extensions, but will keep the bug open since I want to understand what other repro cases cause such corruption.

Requesting merge for r411224. It has had enough time to bake on canary and should be helpful overall.

[Automated comment] Less than 2 weeks to go before stable on M53, manual review required.

Approving merge for r411224 to M53 branch 2785 based on comment #25. And per chat with nasko@, this change should be safe and is also likely to help with the number of renderer kills. 

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

commit 4ab88c96e291c275b3c0801aaffe13f124dd427d
Author: Nasko Oskov <nasko@chromium.org>
Date: Fri Aug 26 01:16:05 2016

Ensure FrameNavigationEntry is fully updated.

Currently RendererDidNavigateToExistingPage and RendererDidNavigateToSamePage
only partially update the FrameNavigationEntry for the committing
navigation. This leads to inconsistency between sequence numbers and
PageState, which can cause origin/URL mismatches.

This CL updates both these methods to call AddOrUpdateFrameEntry to
ensure that all members are properly updated.

BUG=630103,  628677 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2224213005
Cr-Commit-Position: refs/heads/master@{#411224}
(cherry picked from commit af18219268a332a92b3c8bbaa306aade92142c55)
TBR=creis@chromium.org

Review URL: https://codereview.chromium.org/2284533002 .

Cr-Commit-Position: refs/branch-heads/2785@{#755}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/4ab88c96e291c275b3c0801aaffe13f124dd427d/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/4ab88c96e291c275b3c0801aaffe13f124dd427d/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/4ab88c96e291c275b3c0801aaffe13f124dd427d/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/4ab88c96e291c275b3c0801aaffe13f124dd427d/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

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

commit 18ae7a912d650b63b480186c574f672e3ed5d47f
Author: nasko <nasko@chromium.org>
Date: Sun Aug 28 03:20:04 2016

Revert of Don't mark navigations as in-page when DSNs are mismatched. (patchset #1 id:1 of https://codereview.chromium.org/2253233002/ )

Reason for revert:
Reverting post branch point for another attempt at diagnosing the remaining cases where origin and URL are mismatched.

Original issue's description:
> Don't mark navigations as in-page when DSNs are mismatched.
>
> This CL implements a TODO by creis@ to ensure that the renderer doesn't
> classify history navigations as in-page if the document sequence number
> of the current document and the one being navigated to are mismatched.
>
> BUG= 628677 , 630103
>
> Committed: https://crrev.com/cf477b509f7a5a3409ac1f2aecf9513d7c82c5b3
> Cr-Commit-Position: refs/heads/master@{#413353}

TBR=avi@chromium.org,creis@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 628677 , 630103

Review-Url: https://codereview.chromium.org/2287933002
Cr-Commit-Position: refs/heads/master@{#414955}

[modify] https://crrev.com/18ae7a912d650b63b480186c574f672e3ed5d47f/content/renderer/render_frame_impl.cc

Users experienced this crash on the following builds:

Win Canary 55.0.2843.0 -  0.65 CPM, 3 reports, 3 clients (signature content::RenderFrameImpl::SendDidCommitProvisionalLoad)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

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

commit 04a322f25adf8b9cab272e985f44db9a01814450
Author: nasko <nasko@chromium.org>
Date: Thu Sep 01 00:00:34 2016

Update FrameNavigationEntry members when setting PageState.

PageState contains data that is duplicated in the actual member variables
of FrameNavigationEntry. When the PageState is set for a FrameNavigationEntry
the duplicate entries should not mismatch, so this CL is adding code to
explicitly set them.

BUG= 628677 , 630103
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
NOTRY=true

Review-Url: https://codereview.chromium.org/2294343002
Cr-Commit-Position: refs/heads/master@{#415812}

[modify] https://crrev.com/04a322f25adf8b9cab272e985f44db9a01814450/content/browser/frame_host/frame_navigation_entry.cc
[modify] https://crrev.com/04a322f25adf8b9cab272e985f44db9a01814450/content/browser/frame_host/frame_navigation_entry.h
[modify] https://crrev.com/04a322f25adf8b9cab272e985f44db9a01814450/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/04a322f25adf8b9cab272e985f44db9a01814450/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/04a322f25adf8b9cab272e985f44db9a01814450/content/browser/web_contents/web_contents_impl.cc

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

commit e6c6898cbfcf2be644cf5c8f283f74f75c60161f
Author: nasko <nasko@chromium.org>
Date: Wed Sep 07 18:53:27 2016

Don't mark navigations as in-page when DSNs are mismatched.

When renderer is asked to do a session history navigation that is in-page,
it should verify whether it has already committed a different document
by verifying the document sequence number (DSN) matches between the one
it has and the one sent by the browser process. When there is a mismatch,
the navigation should not be treated as in-page, since that can lead to
mismatch between the origin and URL of a document.

This is a slightly modified version of
https://codereview.chromium.org/2253233002/, which was reverted and
includes a test case to reproduce the race condition causing the mismatch.

BUG= 628677 , 630103
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2316133003
Cr-Commit-Position: refs/heads/master@{#417012}

[modify] https://crrev.com/e6c6898cbfcf2be644cf5c8f283f74f75c60161f/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/e6c6898cbfcf2be644cf5c8f283f74f75c60161f/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/e6c6898cbfcf2be644cf5c8f283f74f75c60161f/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

With the latest fix, I consider this one complete. There are no more kills of this type.

I disabled the EnsureFrameNavigationEntriesClearedOnMismatchNoSrc test as it has been flaky on windows  https://crbug.com/765107