Set the first party for cookies url in RenderFrame

Set the first party for cookies url in RenderFrame

This CL sets the first party for cookies url in
RenderFrameImpl::willSendRequest. It is currently done in blink, but this will be removed by https://chromiumcodereview.appspot.com/314093002/.

BUG=376025
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275247

[clamy, 2014-06-05 09:47:14]

@ppi: PTAL

[ppi, 2014-06-05 15:27:16]

lgtm, one question below, thanks!

https://codereview.chromium.org/317853002/diff/40001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/317853002/diff/40001/content/renderer/render_...
content/renderer/render_frame_impl.cc:2362:
request.setFirstPartyForCookies(frame->top()->document().url());
Does it matter that we changed from ::firstPartyForCookies() to ::url() here?

[clamy, 2014-06-05 15:33:27]

@jam: PTAL

@ppi: thanks!

https://chromiumcodereview.appspot.com/317853002/diff/40001/content/renderer/...
File content/renderer/render_frame_impl.cc (right):

https://chromiumcodereview.appspot.com/317853002/diff/40001/content/renderer/...
content/renderer/render_frame_impl.cc:2362:
request.setFirstPartyForCookies(frame->top()->document().url());
On 2014/06/05 15:27:16, ppi wrote:
> Does it matter that we changed from ::firstPartyForCookies() to ::url() here?

firstPartyForCookies is not in the public blink API. What it does is get the url
of the topmost document. But AFAIK document() here already returns the topmost
document, so we should be fine.

[ppi, 2014-06-05 15:41:42]

https://codereview.chromium.org/317853002/diff/40001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/317853002/diff/40001/content/renderer/render_...
content/renderer/render_frame_impl.cc:2362:
request.setFirstPartyForCookies(frame->top()->document().url());
I agree it's probably fine, but the function seems to be in the API, so we might
as well use:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

On 2014/06/05 15:33:27, clamy wrote:
> On 2014/06/05 15:27:16, ppi wrote:
> > Does it matter that we changed from ::firstPartyForCookies() to ::url()
here?
> 
> firstPartyForCookies is not in the public blink API. What it does is get the
url
> of the topmost document. But AFAIK document() here already returns the topmost
> document, so we should be fine.

[clamy, 2014-06-05 16:04:59]

https://chromiumcodereview.appspot.com/317853002/diff/40001/content/renderer/...
File content/renderer/render_frame_impl.cc (right):

https://chromiumcodereview.appspot.com/317853002/diff/40001/content/renderer/...
content/renderer/render_frame_impl.cc:2362:
request.setFirstPartyForCookies(frame->top()->document().url());
On 2014/06/05 15:41:43, ppi wrote:
> I agree it's probably fine, but the function seems to be in the API, so we
might
> as well use:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> On 2014/06/05 15:33:27, clamy wrote:
> > On 2014/06/05 15:27:16, ppi wrote:
> > > Does it matter that we changed from ::firstPartyForCookies() to ::url()
> here?
> > 
> > firstPartyForCookies is not in the public blink API. What it does is get the
> url
> > of the topmost document. But AFAIK document() here already returns the
topmost
> > document, so we should be fine.
> 

My bad, missed it. In that case I agree that it is better to use it.

[jam, 2014-06-05 19:06:52]

lgtm

[clamy, 2014-06-05 19:43:20]

The CQ bit was checked by clamy@chromium.org

[commit-bot: I haz the power, 2014-06-05 19:45:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/clamy@chromium.org/317853002/60001

[commit-bot: I haz the power, 2014-06-05 20:53:40]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)

[commit-bot: I haz the power, 2014-06-05 21:23:22]

Change committed as 275247