Changes client_token to be client_auth_token.

Changes client_token to be client_auth_token.

Renames client_token to client_auth_token. Compromise between Chromium naming and blimp_service.proto naming.

Will add changes to effect logging and test_client_token path once this rename is approved.

R=kylechar@chromium.org,piman@chromium.org,sievers@chromium.org, maniscalco@chromium.org
CC=wez@chromium.org,lethalantidote@chromium.org
BUG=591074
Committed: https://crrev.com/6be513e0c4924679cdf1b6a32f1b1b60b167e6e0
Cr-Commit-Position: refs/heads/master@{#416146}

[piman, 2016-08-29 23:07:09]

https://codereview.chromium.org/2281783002/diff/1/ui/gl/BUILD.gn
File ui/gl/BUILD.gn (right):

https://codereview.chromium.org/2281783002/diff/1/ui/gl/BUILD.gn#newcode220
ui/gl/BUILD.gn:220: include_dirs = [ "//third_party/swiftshader/include" ]
This seems unrelated. Doing this caused some issues in the past (conflict with
the mesa headers).

[CJ, 2016-08-29 23:24:46]

On 2016/08/29 23:07:09, piman wrote:
> https://codereview.chromium.org/2281783002/diff/1/ui/gl/BUILD.gn
> File ui/gl/BUILD.gn (right):
> 
> https://codereview.chromium.org/2281783002/diff/1/ui/gl/BUILD.gn#newcode220
> ui/gl/BUILD.gn:220: include_dirs = [ "//third_party/swiftshader/include" ]
> This seems unrelated. Doing this caused some issues in the past (conflict with
> the mesa headers).

Not sure why, but I've been seeing that change occur despite me not touching the
file myself. hmmm. It's gone now. Thanks.

[piman, 2016-08-29 23:25:24]

On 2016/08/29 23:24:46, CJ wrote:
> On 2016/08/29 23:07:09, piman wrote:
> > https://codereview.chromium.org/2281783002/diff/1/ui/gl/BUILD.gn
> > File ui/gl/BUILD.gn (right):
> > 
> > https://codereview.chromium.org/2281783002/diff/1/ui/gl/BUILD.gn#newcode220
> > ui/gl/BUILD.gn:220: include_dirs = [ "//third_party/swiftshader/include" ]
> > This seems unrelated. Doing this caused some issues in the past (conflict
with
> > the mesa headers).
> 
> Not sure why, but I've been seeing that change occur despite me not touching
the
> file myself. hmmm. It's gone now. Thanks.

Ok, I don't think you need my review then, at this point.

[CJ, 2016-08-29 23:27:50]

lethalantidote@chromium.org changed reviewers:
+ wez@chromium.org

[CJ, 2016-08-29 23:27:51]

+ Wez, owners

[maniscalco, 2016-08-30 18:05:09]

lgtm

https://codereview.chromium.org/2281783002/diff/20001/blimp/client/core/sessi...
File blimp/client/core/session/assignment_source.cc (right):

https://codereview.chromium.org/2281783002/diff/20001/blimp/client/core/sessi...
blimp/client/core/session/assignment_source.cc:115:
CHECK(!assignment.client_auth_token.empty()) << "No client token provided.";
"No client token provided." -> "No client auth token provided." ?

https://codereview.chromium.org/2281783002/diff/20001/blimp/client/core/sessi...
File blimp/client/core/session/assignment_source_unittest.cc (right):

https://codereview.chromium.org/2281783002/diff/20001/blimp/client/core/sessi...
blimp/client/core/session/assignment_source_unittest.cc:172: // Path to the
client token;
client token -> client auth token ?

https://codereview.chromium.org/2281783002/diff/20001/blimp/common/proto/prot...
File blimp/common/proto/protocol_control.proto (right):

https://codereview.chromium.org/2281783002/diff/20001/blimp/common/proto/prot...
blimp/common/proto/protocol_control.proto:18: optional string client_auth_token
= 1;
I'm not so sure I'd change this proto field name since the value needs to match
the "client_token" returned by the assigner, but I don't feel that strongly at
this point.  Wez, WDYT?

[Wez, 2016-09-01 02:29:26]

https://codereview.chromium.org/2281783002/diff/20001/blimp/common/proto/prot...
File blimp/common/proto/protocol_control.proto (right):

https://codereview.chromium.org/2281783002/diff/20001/blimp/common/proto/prot...
blimp/common/proto/protocol_control.proto:18: optional string client_auth_token
= 1;
On 2016/08/30 18:05:09, maniscalco wrote:
> I'm not so sure I'd change this proto field name since the value needs to
match
> the "client_token" returned by the assigner, but I don't feel that strongly at
> this point.  Wez, WDYT?

My preference would be to rename it here, for consistency with the rest of the
Chromium-side code - then from the Chromium side only the Assigner API field has
the client_token name. Make sense?

[Wez, 2016-09-01 02:29:32]

lgtm

[maniscalco, 2016-09-01 14:44:30]

https://codereview.chromium.org/2281783002/diff/20001/blimp/common/proto/prot...
File blimp/common/proto/protocol_control.proto (right):

https://codereview.chromium.org/2281783002/diff/20001/blimp/common/proto/prot...
blimp/common/proto/protocol_control.proto:18: optional string client_auth_token
= 1;
On 2016/09/01 02:29:26, Wez wrote:
> On 2016/08/30 18:05:09, maniscalco wrote:
> > I'm not so sure I'd change this proto field name since the value needs to
> match
> > the "client_token" returned by the assigner, but I don't feel that strongly
at
> > this point.  Wez, WDYT?
> 
> My preference would be to rename it here, for consistency with the rest of the
> Chromium-side code - then from the Chromium side only the Assigner API field
has
> the client_token name. Make sense?

I'm fine with renaming it as well, but just so we're clear things still won't be
fully consistent throughout chromium as there are other chromium places that
refer to it as "client token" that aren't being changed (e.g. the command line
flags).  And, IMO, it's not worth changing those.

[Wez, 2016-09-01 21:20:09]

Yes, that sounds good to me!

On 1 September 2016 at 07:44, <maniscalco@chromium.org> wrote:

>
> https://codereview.chromium.org/2281783002/diff/20001/
> blimp/common/proto/protocol_control.proto
> File blimp/common/proto/protocol_control.proto (right):
>
> https://codereview.chromium.org/2281783002/diff/20001/
> blimp/common/proto/protocol_control.proto#newcode18
> blimp/common/proto/protocol_control.proto:18: optional string
> client_auth_token = 1;
> On 2016/09/01 02:29:26, Wez wrote:
> > On 2016/08/30 18:05:09, maniscalco wrote:
> > > I'm not so sure I'd change this proto field name since the value
> needs to
> > match
> > > the "client_token" returned by the assigner, but I don't feel that
> strongly at
> > > this point. Wez, WDYT?
> >
> > My preference would be to rename it here, for consistency with the
> rest of the
> > Chromium-side code - then from the Chromium side only the Assigner API
> field has
> > the client_token name. Make sense?
>
> I'm fine with renaming it as well, but just so we're clear things still
> won't be fully consistent throughout chromium as there are other
> chromium places that refer to it as "client token" that aren't being
> changed (e.g. the command line flags). And, IMO, it's not worth
> changing those.
>
> https://codereview.chromium.org/2281783002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[CJ, 2016-09-01 23:15:56]

On 2016/09/01 21:20:09, Wez wrote:
> Yes, that sounds good to me!
> 
> On 1 September 2016 at 07:44, <mailto:maniscalco@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/2281783002/diff/20001/
> > blimp/common/proto/protocol_control.proto
> > File blimp/common/proto/protocol_control.proto (right):
> >
> > https://codereview.chromium.org/2281783002/diff/20001/
> > blimp/common/proto/protocol_control.proto#newcode18
> > blimp/common/proto/protocol_control.proto:18: optional string
> > client_auth_token = 1;
> > On 2016/09/01 02:29:26, Wez wrote:
> > > On 2016/08/30 18:05:09, maniscalco wrote:
> > > > I'm not so sure I'd change this proto field name since the value
> > needs to
> > > match
> > > > the "client_token" returned by the assigner, but I don't feel that
> > strongly at
> > > > this point. Wez, WDYT?
> > >
> > > My preference would be to rename it here, for consistency with the
> > rest of the
> > > Chromium-side code - then from the Chromium side only the Assigner API
> > field has
> > > the client_token name. Make sense?
> >
> > I'm fine with renaming it as well, but just so we're clear things still
> > won't be fully consistent throughout chromium as there are other
> > chromium places that refer to it as "client token" that aren't being
> > changed (e.g. the command line flags). And, IMO, it's not worth
> > changing those.
> >
> > https://codereview.chromium.org/2281783002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

So should I bother with things like logging and the test_client_token path after
this?

[CJ, 2016-09-01 23:18:22]

https://codereview.chromium.org/2281783002/diff/20001/blimp/client/core/sessi...
File blimp/client/core/session/assignment_source.cc (right):

https://codereview.chromium.org/2281783002/diff/20001/blimp/client/core/sessi...
blimp/client/core/session/assignment_source.cc:115:
CHECK(!assignment.client_auth_token.empty()) << "No client token provided.";
On 2016/08/30 18:05:09, maniscalco wrote:
> "No client token provided." -> "No client auth token provided." ?

Done.

https://codereview.chromium.org/2281783002/diff/20001/blimp/client/core/sessi...
File blimp/client/core/session/assignment_source_unittest.cc (right):

https://codereview.chromium.org/2281783002/diff/20001/blimp/client/core/sessi...
blimp/client/core/session/assignment_source_unittest.cc:172: // Path to the
client token;
On 2016/08/30 18:05:09, maniscalco wrote:
> client token -> client auth token ?

Done.

https://codereview.chromium.org/2281783002/diff/20001/blimp/common/proto/prot...
File blimp/common/proto/protocol_control.proto (right):

https://codereview.chromium.org/2281783002/diff/20001/blimp/common/proto/prot...
blimp/common/proto/protocol_control.proto:18: optional string client_auth_token
= 1;
On 2016/09/01 14:44:30, maniscalco wrote:
> On 2016/09/01 02:29:26, Wez wrote:
> > On 2016/08/30 18:05:09, maniscalco wrote:
> > > I'm not so sure I'd change this proto field name since the value needs to
> > match
> > > the "client_token" returned by the assigner, but I don't feel that
strongly
> at
> > > this point.  Wez, WDYT?
> > 
> > My preference would be to rename it here, for consistency with the rest of
the
> > Chromium-side code - then from the Chromium side only the Assigner API field
> has
> > the client_token name. Make sense?
> 
> I'm fine with renaming it as well, but just so we're clear things still won't
be
> fully consistent throughout chromium as there are other chromium places that
> refer to it as "client token" that aren't being changed (e.g. the command line
> flags).  And, IMO, it's not worth changing those.

Acknowledged.

[CJ, 2016-09-01 23:25:51]

The CQ bit was checked by lethalantidote@chromium.org

[CJ, 2016-09-01 23:25:52]

The patchset sent to the CQ was uploaded after l-g-t-m from wez@chromium.org,
maniscalco@chromium.org
Link to the patchset: https://codereview.chromium.org/2281783002/#ps60001
(title: "Linting.")

[commit-bot: I haz the power, 2016-09-01 23:26:16]

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-02 00:38:27]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-02 00:40:53]

Description was changed from

==========
Changes client_token to be client_auth_token.

Renames client_token to client_auth_token. Compromise between Chromium naming
and blimp_service.proto naming.

Will add changes to effect logging and test_client_token path once this rename
is approved.

R=kylechar@chromium.org,piman@chromium.org,sievers@chromium.org,
maniscalco@chromium.org
CC=wez@chromium.org,lethalantidote@chromium.org
BUG=591074
==========

to

==========
Changes client_token to be client_auth_token.

Renames client_token to client_auth_token. Compromise between Chromium naming
and blimp_service.proto naming.

Will add changes to effect logging and test_client_token path once this rename
is approved.

R=kylechar@chromium.org,piman@chromium.org,sievers@chromium.org,
maniscalco@chromium.org
CC=wez@chromium.org,lethalantidote@chromium.org
BUG=591074

Committed: https://crrev.com/6be513e0c4924679cdf1b6a32f1b1b60b167e6e0
Cr-Commit-Position: refs/heads/master@{#416146}
==========

[commit-bot: I haz the power, 2016-09-02 00:40:54]

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