SameSite: Correctly set requests' initiator for new tabs.

SameSite: Correctly set requests' initiator for new tabs.

A user-initiatated navigation in a newly opened tab (via the omnibox, for
example) should be considered SameSite (this is step 1 of Section 2.1
of the spec[1]). This patch adjusts 'FrameLoadRequest' and 'RenderFrameImpl' accordingly.

Now when the browser initiates a navigation on a user's behalf, the frame
type will be set correctly (in 'RenderFrameImpl::NavigateInternal()'), and
the initiator will be updated only if it isn't already set (in
'RenderFrameImpl::willSendRequest()' and 'FrameLoadRequest()').

[1]: https://tools.ietf.org/html/draft-west-first-party-cookies-07#section-2.1

BUG=619603
Committed: https://crrev.com/4f2cb7dddfefb352de6c980d9597539674ba0272
Cr-Commit-Position: refs/heads/master@{#401552}

[Mike West, 2016-06-20 10:22:14]

mkwst@chromium.org changed reviewers:
+ foolip@chromium.org, jochen@chromium.org, yoav@yoav.ws

[Mike West, 2016-06-20 10:22:15]

Jochen, mind looking at the //content bits?

Yoav, Philip, mind poking at Blink?

Thanks!

https://codereview.chromium.org/2080653002/diff/1/content/renderer/render_fra...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2080653002/diff/1/content/renderer/render_fra...
content/renderer/render_frame_impl.cc:3906:
request.setFirstPartyForCookies(frame->document().firstPartyForCookies());
Drive-by: We don't need to poke only at the top-level document now;
'Document::firstPartyForCookies()' should correctly handle remote frames (this
change is verified by the last part of
'RequestDataResourceDispatcherHostBrowserTest::CrossOriginNested').

[jochen (gone - plz use gerrit), 2016-06-21 07:45:21]

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

https://codereview.chromium.org/2080653002/diff/20001/content/renderer/render...
content/renderer/render_frame_impl.cc:3907: // This is broken and should be
fixed to propagate the first party.
Why is that no longer true?

[Mike West, 2016-06-21 08:58:20]

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

https://codereview.chromium.org/2080653002/diff/20001/content/renderer/render...
content/renderer/render_frame_impl.cc:3907: // This is broken and should be
fixed to propagate the first party.
On 2016/06/21 at 07:45:21, jochen wrote:
> Why is that no longer true?

Because we now replicate the origin across remote security contexts, which means
that alexmos@ was able to rework the method in
https://codereview.chromium.org/2046593003.

[Mike West, 2016-06-21 09:01:01]

https://codereview.chromium.org/2080653002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/2080653002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:4164: return
importsController()->master()->firstPartyForCookies();
Note also: I could split this and the 'render_frame_impl.cc'
'firstPartyForCookies()' piece out into a separate patch if you like. The one
requires the other, as the current implementation is busted for imports (they
have no `frame()`, so asking them for a first-party causes a null-deref.

[Mike West, 2016-06-22 08:32:24]

On 2016/06/21 at 09:01:01, Mike West wrote:
>
https://codereview.chromium.org/2080653002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/dom/Document.cpp (right):
> 
>
https://codereview.chromium.org/2080653002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/dom/Document.cpp:4164: return
importsController()->master()->firstPartyForCookies();
> Note also: I could split this and the 'render_frame_impl.cc'
'firstPartyForCookies()' piece out into a separate patch if you like. The one
requires the other, as the current implementation is busted for imports (they
have no `frame()`, so asking them for a first-party causes a null-deref.

I split this out into https://codereview.chromium.org/2084333002.

[Mike West, 2016-06-22 09:20:32]

Description was changed from

==========
SameSite: Correctly set requests' initiator for new tabs.

A user-initiatated navigation in a newly opened tab (via the omnibox, for
example) should be considered SameSite (this is step 1 of Section 2.1
of the spec[1]). This patch adjusts 'FrameFetchContext', 'FrameLoadRequest'
and 'RenderFrameImpl' accordingly.

Now when the browser initiates a navigation on a user's behalf, the frame
type will be set correctly (in 'RenderFrameImpl::NavigateInternal()'), and
the initiator will be updated only if it isn't already set (in
'RenderFrameImpl::willSendRequest()' and 'FrameLoadRequest()').

[1]: https://tools.ietf.org/html/draft-west-first-party-cookies-07#section-2.1

BUG=619603
==========

to

==========
SameSite: Correctly set requests' initiator for new tabs.

A user-initiatated navigation in a newly opened tab (via the omnibox, for
example) should be considered SameSite (this is step 1 of Section 2.1
of the spec[1]). This patch adjusts 'FrameLoadRequest' and 'RenderFrameImpl'
accordingly.

Now when the browser initiates a navigation on a user's behalf, the frame
type will be set correctly (in 'RenderFrameImpl::NavigateInternal()'), and
the initiator will be updated only if it isn't already set (in
'RenderFrameImpl::willSendRequest()' and 'FrameLoadRequest()').

[1]: https://tools.ietf.org/html/draft-west-first-party-cookies-07#section-2.1

BUG=619603
==========

[Mike West, 2016-06-22 09:20:41]

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

[commit-bot: I haz the power, 2016-06-22 09:20:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2080653002/40001

[Mike West, 2016-06-22 09:22:38]

I've stripped out the FrameFetchContext changes; the bug this patch is
addressing can be fixed without them.

That said, I think it's worthwhile to go back and add them in again in a
subsequent patch, alongside changes to ResourceRequest to start with a
`requestorOrigin` of `nullptr` rather than a unique origin, but those wider
refactorings are unnecessary, and just complicate this patch.

[commit-bot: I haz the power, 2016-06-22 10:54:10]

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

[commit-bot: I haz the power, 2016-06-22 10:54:11]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2016-06-22 13:17:49]

lgtm

[Mike West, 2016-06-22 13:44:25]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-06-22 13:44:30]

The CQ bit was unchecked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-06-22 13:44:34]

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

[commit-bot: I haz the power, 2016-06-22 13:44:35]

This CL has an open dependency (Issue 2084333002 Patch 40001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Mike West, 2016-06-23 06:27:16]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-06-23 06:27:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2080653002/40001

[commit-bot: I haz the power, 2016-06-23 06:32:38]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-23 06:36:12]

Description was changed from

==========
SameSite: Correctly set requests' initiator for new tabs.

A user-initiatated navigation in a newly opened tab (via the omnibox, for
example) should be considered SameSite (this is step 1 of Section 2.1
of the spec[1]). This patch adjusts 'FrameLoadRequest' and 'RenderFrameImpl'
accordingly.

Now when the browser initiates a navigation on a user's behalf, the frame
type will be set correctly (in 'RenderFrameImpl::NavigateInternal()'), and
the initiator will be updated only if it isn't already set (in
'RenderFrameImpl::willSendRequest()' and 'FrameLoadRequest()').

[1]: https://tools.ietf.org/html/draft-west-first-party-cookies-07#section-2.1

BUG=619603
==========

to

==========
SameSite: Correctly set requests' initiator for new tabs.

A user-initiatated navigation in a newly opened tab (via the omnibox, for
example) should be considered SameSite (this is step 1 of Section 2.1
of the spec[1]). This patch adjusts 'FrameLoadRequest' and 'RenderFrameImpl'
accordingly.

Now when the browser initiates a navigation on a user's behalf, the frame
type will be set correctly (in 'RenderFrameImpl::NavigateInternal()'), and
the initiator will be updated only if it isn't already set (in
'RenderFrameImpl::willSendRequest()' and 'FrameLoadRequest()').

[1]: https://tools.ietf.org/html/draft-west-first-party-cookies-07#section-2.1

BUG=619603

Committed: https://crrev.com/4f2cb7dddfefb352de6c980d9597539674ba0272
Cr-Commit-Position: refs/heads/master@{#401552}
==========

[commit-bot: I haz the power, 2016-06-23 06:36:14]

Patchset 3 (id:??) landed as
https://crrev.com/4f2cb7dddfefb352de6c980d9597539674ba0272
Cr-Commit-Position: refs/heads/master@{#401552}