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

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

Committed: https://crrev.com/e6c6898cbfcf2be644cf5c8f283f74f75c60161f
Cr-Commit-Position: refs/heads/master@{#417012}

[nasko, 2016-09-07 17:19:28]

Description was changed from

==========
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
==========

to

==========
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
==========

[nasko, 2016-09-07 17:21:16]

The CQ bit was checked by nasko@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-07 17:21:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nasko, 2016-09-07 17:23:17]

nasko@chromium.org changed reviewers:
+ creis@chromium.org

[nasko, 2016-09-07 17:23:18]

Hey Charlie,
I was able to write a test for the race condition we discussed :). I did choose
to go with a BrowserMessageFilter, as it is very clear what we want to achieve
in the test. I am not certain a NavigationThrottle would be able to guarantee
the order of events we need.
Can you review it for me?
Thanks in advance!
Nasko

[Charlie Reis, 2016-09-07 17:42:09]

Glad you got a test working!  LGTM.

https://codereview.chromium.org/2316133003/diff/20001/content/renderer/render...
File content/renderer/render_frame_impl.cc (left):

https://codereview.chromium.org/2316133003/diff/20001/content/renderer/render...
content/renderer/render_frame_impl.cc:5546: // NOTREACHED when we're confident
this won't happen.
I don't want to lose track of adding a NOTREACHED if the browser sends a
SameDocumentLoad type when there's no current_history_item_, but no need for a
new patch for that.  I'll just put together a CL for it after this lands.

[commit-bot: I haz the power, 2016-09-07 18:47:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-07 18:47:04]

Dry run: This issue passed the CQ dry run.

[nasko, 2016-09-07 18:47:12]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2016-09-07 18:47:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 18:53:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-07 18:55:24]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/e6c6898cbfcf2be644cf5c8f283f74f75c60161f
Cr-Commit-Position: refs/heads/master@{#417012}
==========

[commit-bot: I haz the power, 2016-09-07 18:55:25]

Patchset 2 (id:??) landed as
https://crrev.com/e6c6898cbfcf2be644cf5c8f283f74f75c60161f
Cr-Commit-Position: refs/heads/master@{#417012}

[gogerald1, 2016-09-08 15:32:54]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2323803002/ by gogerald@chromium.org.

The reason for reverting is: Introduced flaky test,
https://bugs.chromium.org/p/chromium/issues/detail?id=645022.