Rename first-party-only cookies to same-site cookies.

Rename first-party-only cookies to same-site cookies.

As per https://tools.ietf.org/html/draft-west-first-party-cookies-05 and
https://lists.w3.org/Archives/Public/ietf-http-wg/2016JanMar/0134.html.

BUG=459154
Committed: https://crrev.com/4654941d380d76638610198ac497015b8d979bb9
Cr-Commit-Position: refs/heads/master@{#372630}

[Mike West, 2016-01-22 10:30:50]

mkwst@chromium.org changed reviewers:
+ jochen@chromium.org, mmenke@chromium.org

[Mike West, 2016-01-22 10:30:51]

mmenke@: Would you mind taking a look at the (mostly mechanical) changes to
//net?

jochen@: Could you stamp the non-//net changes?

[jochen (gone - plz use gerrit), 2016-01-22 14:19:29]

ugh

anyways, if net is happy the rest lgtm

[mmenke, 2016-01-22 17:05:28]

May not have time to do a full review until Monday, due to an offsite.

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/canonical_c...
File net/cookies/canonical_cookie.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/canonical_c...
net/cookies/canonical_cookie.h:41: bool samesite,
Should this be same_site?  The property itself uses SameSite, and the property
is queried by IsSameSite, which would imply that it should be.

[Mike West, 2016-01-25 09:46:54]

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/canonical_c...
File net/cookies/canonical_cookie.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/canonical_c...
net/cookies/canonical_cookie.h:41: bool samesite,
On 2016/01/22 at 17:05:28, mmenke wrote:
> Should this be same_site?  The property itself uses SameSite, and the property
is queried by IsSameSite, which would imply that it should be.

I'm basically following along with `httponly` here, which is a bit fluid in the
existing code. I'm happy to use either `httponly`/`samesite` or
`http_only`/`same_site` at each callsite if you'd prefer.

[mmenke, 2016-01-25 18:47:10]

Tagged samesites (As I think we should go with same_site and http_only, per
comment).  And one tangential question.

https://codereview.chromium.org/1615773005/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1615773005/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:10268: +      <message
name="IDS_COOKIES_COOKIE_SENDFOR_SAMESITE" desc="Send Cookie for any same-site
connection">
SAME_SITE?

https://codereview.chromium.org/1615773005/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:10271: +      <message
name="IDS_COOKIES_COOKIE_SENDFOR_SECURE_SAMESITE" desc="Send Cookie for secure,
same-site connections">
SAME_SITE?

https://codereview.chromium.org/1615773005/diff/40001/chrome/browser/android/...
File chrome/browser/android/cookies/cookies_fetcher.cc (right):

https://codereview.chromium.org/1615773005/diff/40001/chrome/browser/android/...
chrome/browser/android/cookies/cookies_fetcher.cc:113: bool samesite,
same_site (x2)

https://codereview.chromium.org/1615773005/diff/40001/ios/net/cookies/cookie_...
File ios/net/cookies/cookie_store_ios_unittest.mm (right):

https://codereview.chromium.org/1615773005/diff/40001/ios/net/cookies/cookie_...
ios/net/cookies/cookie_store_ios_unittest.mm:226: false,         // samesite
same_site

https://codereview.chromium.org/1615773005/diff/40001/ios/net/cookies/system_...
File ios/net/cookies/system_cookie_util_unittest.mm (right):

https://codereview.chromium.org/1615773005/diff/40001/ios/net/cookies/system_...
ios/net/cookies/system_cookie_util_unittest.mm:35: false,  // samesite
same_site (x2)

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/canonical_c...
File net/cookies/canonical_cookie.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/canonical_c...
net/cookies/canonical_cookie.h:41: bool samesite,
On 2016/01/25 09:46:54, Mike West wrote:
> On 2016/01/22 at 17:05:28, mmenke wrote:
> > Should this be same_site?  The property itself uses SameSite, and the
property
> is queried by IsSameSite, which would imply that it should be.
> 
> I'm basically following along with `httponly` here, which is a bit fluid in
the
> existing code. I'm happy to use either `httponly`/`samesite` or
> `http_only`/`same_site` at each callsite if you'd prefer.

I'd say go with `http_only`/`same_site`

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/cookie_mons...
File net/cookies/cookie_monster.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/cookie_mons...
net/cookies/cookie_monster.h:420: COOKIE_TYPE_SAMESITE = 0,
SAME_SITE

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/cookie_opti...
File net/cookies/cookie_options.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/cookie_opti...
net/cookies/cookie_options.h:36: void set_include_samesite() { include_samesite_
= true; }
same_site

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/parsed_cook...
File net/cookies/parsed_cookie.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/parsed_cook...
net/cookies/parsed_cookie.h:143: size_t samesite_index_;
same_site

https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
File net/url_request/url_request_http_job.cc (right):

https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:674:
cookie_store->GetAllCookiesForURLAsync(
Erm...wait...We get cookies without options, and pass them on to the
embedder...And then we get the cookies with the options, and those are the ones
that we actually use?  Is there a reason for this?

Know this is tangential to this CL, but I'd like to understand what's going on
here.

https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:696:
url::Origin(request_->first_party_for_cookies())) &&
Should first_party_for_cookies be renamed to same_site_for_cookies?

[Mike West, 2016-01-26 11:05:45]

Thanks!

https://codereview.chromium.org/1615773005/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1615773005/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:10268: +      <message
name="IDS_COOKIES_COOKIE_SENDFOR_SAMESITE" desc="Send Cookie for any same-site
connection">
On 2016/01/25 at 18:47:10, mmenke wrote:
> SAME_SITE?

Yup.

https://codereview.chromium.org/1615773005/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:10271: +      <message
name="IDS_COOKIES_COOKIE_SENDFOR_SECURE_SAMESITE" desc="Send Cookie for secure,
same-site connections">
On 2016/01/25 at 18:47:10, mmenke wrote:
> SAME_SITE?

Yup. :)

https://codereview.chromium.org/1615773005/diff/40001/chrome/browser/android/...
File chrome/browser/android/cookies/cookies_fetcher.cc (right):

https://codereview.chromium.org/1615773005/diff/40001/chrome/browser/android/...
chrome/browser/android/cookies/cookies_fetcher.cc:113: bool samesite,
On 2016/01/25 at 18:47:10, mmenke wrote:
> same_site (x2)

Done (x2)

https://codereview.chromium.org/1615773005/diff/40001/ios/net/cookies/cookie_...
File ios/net/cookies/cookie_store_ios_unittest.mm (right):

https://codereview.chromium.org/1615773005/diff/40001/ios/net/cookies/cookie_...
ios/net/cookies/cookie_store_ios_unittest.mm:226: false,         // samesite
On 2016/01/25 at 18:47:10, mmenke wrote:
> same_site

Done.

https://codereview.chromium.org/1615773005/diff/40001/ios/net/cookies/system_...
File ios/net/cookies/system_cookie_util_unittest.mm (right):

https://codereview.chromium.org/1615773005/diff/40001/ios/net/cookies/system_...
ios/net/cookies/system_cookie_util_unittest.mm:35: false,  // samesite
On 2016/01/25 at 18:47:10, mmenke wrote:
> same_site (x2)

Done (x2)

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/canonical_c...
File net/cookies/canonical_cookie.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/canonical_c...
net/cookies/canonical_cookie.h:41: bool samesite,
On 2016/01/25 at 18:47:10, mmenke wrote:
> On 2016/01/25 09:46:54, Mike West wrote:
> > On 2016/01/22 at 17:05:28, mmenke wrote:
> > > Should this be same_site?  The property itself uses SameSite, and the
property
> > is queried by IsSameSite, which would imply that it should be.
> > 
> > I'm basically following along with `httponly` here, which is a bit fluid in
the
> > existing code. I'm happy to use either `httponly`/`samesite` or
> > `http_only`/`same_site` at each callsite if you'd prefer.
> 
> I'd say go with `http_only`/`same_site`

`same_site` is done in this patch. I'll do `http_only` in a subsequent patch to
keep the diff clean.

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/cookie_mons...
File net/cookies/cookie_monster.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/cookie_mons...
net/cookies/cookie_monster.h:420: COOKIE_TYPE_SAMESITE = 0,
On 2016/01/25 at 18:47:10, mmenke wrote:
> SAME_SITE

Done.

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/cookie_opti...
File net/cookies/cookie_options.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/cookie_opti...
net/cookies/cookie_options.h:36: void set_include_samesite() { include_samesite_
= true; }
On 2016/01/25 at 18:47:10, mmenke wrote:
> same_site

Done.

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/parsed_cook...
File net/cookies/parsed_cookie.h (right):

https://codereview.chromium.org/1615773005/diff/40001/net/cookies/parsed_cook...
net/cookies/parsed_cookie.h:143: size_t samesite_index_;
On 2016/01/25 at 18:47:10, mmenke wrote:
> same_site

Done.

https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
File net/url_request/url_request_http_job.cc (right):

https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:696:
url::Origin(request_->first_party_for_cookies())) &&
On 2016/01/25 at 18:47:10, mmenke wrote:
> Should first_party_for_cookies be renamed to same_site_for_cookies?

Maybe. Our UI still talks about blocking "third-party" cookies, but there's
probably value in being consistent with the naming. I'll talk to UI folks. Would
you mind deferring that to a separate discussion/CL?

[Mike West, 2016-01-26 11:06:45]

mkwst@chromium.org changed reviewers:
+ marq@chromium.org

[Mike West, 2016-01-26 11:06:46]

+marq for ios/net/cookies. Mind taking a quick look?

[marq (ping after 24h), 2016-01-26 11:58:17]

/ios/net/ LGTM

[mmenke, 2016-01-26 15:08:05]

https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
File net/url_request/url_request_http_job.cc (right):

https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:674:
cookie_store->GetAllCookiesForURLAsync(
On 2016/01/25 18:47:10, mmenke wrote:
> Erm...wait...We get cookies without options, and pass them on to the
> embedder...And then we get the cookies with the options, and those are the
ones
> that we actually use?  Is there a reason for this?
> 
> Know this is tangential to this CL, but I'd like to understand what's going on
> here.

Do you have any idea about this?  I was unaware of this behavior when I signed
off on your original modifications to DoLoadCookies, so I'm wondering if having
the options stuff only in there is a bug.

[Mike West, 2016-01-26 16:19:13]

https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
File net/url_request/url_request_http_job.cc (right):

https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
net/url_request/url_request_http_job.cc:674:
cookie_store->GetAllCookiesForURLAsync(
On 2016/01/26 at 15:08:05, mmenke wrote:
> On 2016/01/25 18:47:10, mmenke wrote:
> > Erm...wait...We get cookies without options, and pass them on to the
> > embedder...And then we get the cookies with the options, and those are the
ones
> > that we actually use?  Is there a reason for this?
> > 
> > Know this is tangential to this CL, but I'd like to understand what's going
on
> > here.
> 
> Do you have any idea about this?  I was unaware of this behavior when I signed
off on your original modifications to DoLoadCookies, so I'm wondering if having
the options stuff only in there is a bug.

Sorry, I missed this comment in the first pass.

I also have no idea why we grab one set of cookies without options, and then
grab another set of cookies with options. I suspect this is a bug. I'll add a
TODO and dig into `git log` to try to figure out when they diverged and whether
there was a reason.

[mmenke, 2016-01-26 16:40:38]

On 2016/01/26 16:19:13, Mike West wrote:
>
https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
> File net/url_request/url_request_http_job.cc (right):
> 
>
https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
> net/url_request/url_request_http_job.cc:674:
> cookie_store->GetAllCookiesForURLAsync(
> On 2016/01/26 at 15:08:05, mmenke wrote:
> > On 2016/01/25 18:47:10, mmenke wrote:
> > > Erm...wait...We get cookies without options, and pass them on to the
> > > embedder...And then we get the cookies with the options, and those are the
> ones
> > > that we actually use?  Is there a reason for this?
> > > 
> > > Know this is tangential to this CL, but I'd like to understand what's
going
> on
> > > here.
> > 
> > Do you have any idea about this?  I was unaware of this behavior when I
signed
> off on your original modifications to DoLoadCookies, so I'm wondering if
having
> the options stuff only in there is a bug.
> 
> Sorry, I missed this comment in the first pass.
> 
> I also have no idea why we grab one set of cookies without options, and then
> grab another set of cookies with options. I suspect this is a bug. I'll add a
> TODO and dig into `git log` to try to figure out when they diverged and
whether
> there was a reason.

LGTM, once you have this compiling.

[mmenke, 2016-01-26 16:41:05]

On 2016/01/26 16:40:38, mmenke wrote:
> On 2016/01/26 16:19:13, Mike West wrote:
> >
>
https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
> > File net/url_request/url_request_http_job.cc (right):
> > 
> >
>
https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
> > net/url_request/url_request_http_job.cc:674:
> > cookie_store->GetAllCookiesForURLAsync(
> > On 2016/01/26 at 15:08:05, mmenke wrote:
> > > On 2016/01/25 18:47:10, mmenke wrote:
> > > > Erm...wait...We get cookies without options, and pass them on to the
> > > > embedder...And then we get the cookies with the options, and those are
the
> > ones
> > > > that we actually use?  Is there a reason for this?
> > > > 
> > > > Know this is tangential to this CL, but I'd like to understand what's
> going
> > on
> > > > here.
> > > 
> > > Do you have any idea about this?  I was unaware of this behavior when I
> signed
> > off on your original modifications to DoLoadCookies, so I'm wondering if
> having
> > the options stuff only in there is a bug.
> > 
> > Sorry, I missed this comment in the first pass.
> > 
> > I also have no idea why we grab one set of cookies without options, and then
> > grab another set of cookies with options. I suspect this is a bug. I'll add
a
> > TODO and dig into `git log` to try to figure out when they diverged and
> whether
> > there was a reason.
> 
> LGTM, once you have this compiling.

Err...Compiling -> tests passing

[mmenke, 2016-01-26 16:42:43]

On 2016/01/26 16:41:05, mmenke wrote:
> On 2016/01/26 16:40:38, mmenke wrote:
> > On 2016/01/26 16:19:13, Mike West wrote:
> > >
> >
>
https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
> > > File net/url_request/url_request_http_job.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1615773005/diff/40001/net/url_request/url_req...
> > > net/url_request/url_request_http_job.cc:674:
> > > cookie_store->GetAllCookiesForURLAsync(
> > > On 2016/01/26 at 15:08:05, mmenke wrote:
> > > > On 2016/01/25 18:47:10, mmenke wrote:
> > > > > Erm...wait...We get cookies without options, and pass them on to the
> > > > > embedder...And then we get the cookies with the options, and those are
> the
> > > ones
> > > > > that we actually use?  Is there a reason for this?
> > > > > 
> > > > > Know this is tangential to this CL, but I'd like to understand what's
> > going
> > > on
> > > > > here.
> > > > 
> > > > Do you have any idea about this?  I was unaware of this behavior when I
> > signed
> > > off on your original modifications to DoLoadCookies, so I'm wondering if
> > having
> > > the options stuff only in there is a bug.
> > > 
> > > Sorry, I missed this comment in the first pass.
> > > 
> > > I also have no idea why we grab one set of cookies without options, and
then
> > > grab another set of cookies with options. I suspect this is a bug. I'll
add
> a
> > > TODO and dig into `git log` to try to figure out when they diverged and
> > whether
> > > there was a reason.
> > 
> > LGTM, once you have this compiling.
> 
> Err...Compiling -> tests passing

Actually, that seems like a pretty significant bug.  Ping me once they're
passing, and I'll do another once over.

[Mike West, 2016-01-29 06:39:37]

> Actually, that seems like a pretty significant bug.  Ping me once they're
passing, and I'll do another once over.

Mind taking another look? Basically, this was bad regexing on my part.

https://codereview.chromium.org/1615773005/diff/60001/net/cookies/parsed_cook...
File net/cookies/parsed_cookie.cc (right):

https://codereview.chromium.org/1615773005/diff/60001/net/cookies/parsed_cook...
net/cookies/parsed_cookie.cc:58: const char kSameSiteTokenName[] = "same_site";
This should be `samesite`, not `same_site`. Hooray for over-enthusiastic regex.

https://codereview.chromium.org/1615773005/diff/60001/net/cookies/parsed_cook...
File net/cookies/parsed_cookie_unittest.cc (right):

https://codereview.chromium.org/1615773005/diff/60001/net/cookies/parsed_cook...
net/cookies/parsed_cookie_unittest.cc:95: ParsedCookie pc("BLAHHH; Path=/;
sECuRe; httpONLY; same_site; pRIoRitY=hIgH");
And all of these should be `samesite` (or variants thereof, as here). *sigh*

https://codereview.chromium.org/1615773005/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/components/cookie-parser-expected.txt
(right):

https://codereview.chromium.org/1615773005/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/components/cookie-parser-expected.txt:137:
sameSite : undefined
The internal bits of the cookie are all lower-case, as devtools uses this as a
shortcut for cookie attribute comparison. Updating the test results accordingly.

[mmenke, 2016-01-29 16:44:05]

LGTM!

[Mike West, 2016-01-29 19:35:57]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-01-29 19:35:57]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
marq@chromium.org
Link to the patchset: https://codereview.chromium.org/1615773005/#ps80001
(title: "Tests.")

[commit-bot: I haz the power, 2016-01-29 19:36:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1615773005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1615773005/80001

[commit-bot: I haz the power, 2016-01-29 20:19:45]

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

[commit-bot: I haz the power, 2016-01-29 20:19:46]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mike West, 2016-02-01 06:32:38]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-02-01 06:33:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1615773005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1615773005/80001

[commit-bot: I haz the power, 2016-02-01 10:05:47]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-02-01 10:07:57]

Description was changed from

==========
Rename first-party-only cookies to same-site cookies.

As per https://tools.ietf.org/html/draft-west-first-party-cookies-05 and
https://lists.w3.org/Archives/Public/ietf-http-wg/2016JanMar/0134.html.

BUG=459154
==========

to

==========
Rename first-party-only cookies to same-site cookies.

As per https://tools.ietf.org/html/draft-west-first-party-cookies-05 and
https://lists.w3.org/Archives/Public/ietf-http-wg/2016JanMar/0134.html.

BUG=459154

Committed: https://crrev.com/4654941d380d76638610198ac497015b8d979bb9
Cr-Commit-Position: refs/heads/master@{#372630}
==========

[commit-bot: I haz the power, 2016-02-01 10:07:58]

Patchset 5 (id:??) landed as
https://crrev.com/4654941d380d76638610198ac497015b8d979bb9
Cr-Commit-Position: refs/heads/master@{#372630}