[cronet] Add member for QUIC user agent ID, with getter and setter.

[cronet] Add member for QUIC user agent ID, with getter and setter.

This adds a QUIC user agent ID member, passes is to the URLRequestContextBuilder
in InitializeOnNetworkThread(), and provides appropriate modification function.
Additionally, getDefaultQuicUserAgentId() provides a non-empty default value,
which is used if no other value is given before calling Initialize().

BUG=670686
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Committed: https://crrev.com/52f8be34c505eb08f6df0f0763fc25b41eb6f85d
Cr-Commit-Position: refs/heads/master@{#438912}

[lilyhoughton1, 2016-12-12 17:25:41]

Description was changed from

==========
[cronet] Add member for QUIC user agent ID, with getter and setter.

This adds a QUIC user agent ID member, passes is to the URLRequestContextBuilder
in InitializeOnNetworkThread(), and provides appropriate modification function.
Additionally, getDefaultQuicUserAgentId() provides a non-empty default value,
which is used if no other value is given before calling Initialize().

BUG=670686
==========

to

==========
[cronet] Add member for QUIC user agent ID, with getter and setter.

This adds a QUIC user agent ID member, passes is to the URLRequestContextBuilder
in InitializeOnNetworkThread(), and provides appropriate modification function.
Additionally, getDefaultQuicUserAgentId() provides a non-empty default value,
which is used if no other value is given before calling Initialize().

BUG=670686
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[lilyhoughton, 2016-12-12 19:41:11]

lilyhoughton@chromium.org changed reviewers:
+ lilyhoughton@chromium.org, mef@chromium.org

[mef, 2016-12-12 21:52:20]

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
File components/cronet/ios/cronet_environment.h (right):

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.h:76: quic_user_agent_id_ =
getDefaultQuicUserAgentId();
Maybe call getDefaultQuicUserAgentId() when it is needed if
|quic_user_agent_id_| is empty? This will avoid call order requirement and make
NSBundle call unnecessary if app sets it explicitly.

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.h:82: void
set_quic_user_agent_id(std::string& quic_user_agent_id) {
const std::string&

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.h:133: std::string
getDefaultQuicUserAgentId() {
non-trivial set/get methods should have non-inline implementation (in .mm as
opposed to .h file).

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
File components/cronet/ios/cronet_environment.mm (right):

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.mm:216: quic_user_agent_id_(""),
std::string is initialized to empty string by default.

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.mm:275: quic_user_agent_id_,           
// QUIC User Agent ID.
could call getDefault... here if |quic_user_agent_id_| is empty.

[lilyhoughton, 2016-12-13 15:10:23]

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
File components/cronet/ios/cronet_environment.h (right):

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.h:76: quic_user_agent_id_ =
getDefaultQuicUserAgentId();
On 2016/12/12 21:52:20, mef wrote:
> Maybe call getDefaultQuicUserAgentId() when it is needed if
> |quic_user_agent_id_| is empty? This will avoid call order requirement and
make
> NSBundle call unnecessary if app sets it explicitly.

call to getDefaultQuicUserAgentId() removed per comment in
cronet_environment.mm.  |quic_user_agent_id_| is set to the default at
initialization time.

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.h:82: void
set_quic_user_agent_id(std::string& quic_user_agent_id) {
On 2016/12/12 21:52:20, mef wrote:
> const std::string&

Done.

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.h:133: std::string
getDefaultQuicUserAgentId() {
On 2016/12/12 21:52:20, mef wrote:
> non-trivial set/get methods should have non-inline implementation (in .mm as
> opposed to .h file). 

Done.

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
File components/cronet/ios/cronet_environment.mm (right):

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.mm:216: quic_user_agent_id_(""),
On 2016/12/12 21:52:20, mef wrote:
> std::string is initialized to empty string by default.

Done.

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.mm:275: quic_user_agent_id_,           
// QUIC User Agent ID.
On 2016/12/12 21:52:20, mef wrote:
> could call getDefault... here if |quic_user_agent_id_| is empty.

It should also check if |quic_enabled_| is true, to avoid an unnecessary
NSBundle call, right?

[mef, 2016-12-13 20:46:43]

Looks good with minor nits. Any ideas on whether we can somehow test this?

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
File components/cronet/ios/cronet_environment.mm (right):

https://codereview.chromium.org/2568883002/diff/40001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.mm:275: quic_user_agent_id_,           
// QUIC User Agent ID.
On 2016/12/13 15:10:22, lilyhoughton wrote:
> On 2016/12/12 21:52:20, mef wrote:
> > could call getDefault... here if |quic_user_agent_id_| is empty.
> 
> It should also check if |quic_enabled_| is true, to avoid an unnecessary
> NSBundle call, right?

Yes, good point.

https://codereview.chromium.org/2568883002/diff/60001/components/cronet/ios/c...
File components/cronet/ios/cronet_environment.h (right):

https://codereview.chromium.org/2568883002/diff/60001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.h:127: std::string
getDefaultQuicUserAgentId();
nit: const;

https://codereview.chromium.org/2568883002/diff/60001/components/cronet/ios/c...
File components/cronet/ios/cronet_environment.mm (right):

https://codereview.chromium.org/2568883002/diff/60001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.mm:274: quic_enabled_ &&
!quic_user_agent_id_.size()
nit: !quic_user_agent_id_.size() => quic_user_agent_id_.empty()

https://codereview.chromium.org/2568883002/diff/60001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.mm:276: : quic_user_agent_id_,      //
QUIC User Agent ID.
Maybe move it into explicit if (...) construction above, as I'm not sure about
proper formatting for this.

I can't find explicit guideline not to use this construct, do you see it used
elsewhere?

[lilyhoughton, 2016-12-14 18:32:15]

https://codereview.chromium.org/2568883002/diff/60001/components/cronet/ios/c...
File components/cronet/ios/cronet_environment.mm (right):

https://codereview.chromium.org/2568883002/diff/60001/components/cronet/ios/c...
components/cronet/ios/cronet_environment.mm:276: : quic_user_agent_id_,      //
QUIC User Agent ID.
On 2016/12/13 20:46:43, mef wrote:
> Maybe move it into explicit if (...) construction above, as I'm not sure about
> proper formatting for this.
> 
> I can't find explicit guideline not to use this construct, do you see it used
> elsewhere?

It's used for basically the same thing in the android CL  (in the
getDefaultQuicUserId() function)

This is the formatting that cl format generated.

[mef, 2016-12-15 19:10:55]

lgtm

[lilyhoughton, 2016-12-15 19:31:22]

The CQ bit was checked by lilyhoughton@chromium.org

[commit-bot: I haz the power, 2016-12-15 19:32:06]

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-12-15 20:33:52]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481830282312900,
"parent_rev": "3704613d7ee6737610bc9fc3167aec2936bdd9c2", "commit_rev":
"0386427af14f6aed0d460717a5a4250f784d29ce"}

[commit-bot: I haz the power, 2016-12-15 20:34:18]

Description was changed from

==========
[cronet] Add member for QUIC user agent ID, with getter and setter.

This adds a QUIC user agent ID member, passes is to the URLRequestContextBuilder
in InitializeOnNetworkThread(), and provides appropriate modification function.
Additionally, getDefaultQuicUserAgentId() provides a non-empty default value,
which is used if no other value is given before calling Initialize().

BUG=670686
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
[cronet] Add member for QUIC user agent ID, with getter and setter.

This adds a QUIC user agent ID member, passes is to the URLRequestContextBuilder
in InitializeOnNetworkThread(), and provides appropriate modification function.
Additionally, getDefaultQuicUserAgentId() provides a non-empty default value,
which is used if no other value is given before calling Initialize().

BUG=670686
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2568883002
==========

[commit-bot: I haz the power, 2016-12-15 20:34:20]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-12-15 20:36:58]

Description was changed from

==========
[cronet] Add member for QUIC user agent ID, with getter and setter.

This adds a QUIC user agent ID member, passes is to the URLRequestContextBuilder
in InitializeOnNetworkThread(), and provides appropriate modification function.
Additionally, getDefaultQuicUserAgentId() provides a non-empty default value,
which is used if no other value is given before calling Initialize().

BUG=670686
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2568883002
==========

to

==========
[cronet] Add member for QUIC user agent ID, with getter and setter.

This adds a QUIC user agent ID member, passes is to the URLRequestContextBuilder
in InitializeOnNetworkThread(), and provides appropriate modification function.
Additionally, getDefaultQuicUserAgentId() provides a non-empty default value,
which is used if no other value is given before calling Initialize().

BUG=670686
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Committed: https://crrev.com/52f8be34c505eb08f6df0f0763fc25b41eb6f85d
Cr-Commit-Position: refs/heads/master@{#438912}
==========

[commit-bot: I haz the power, 2016-12-15 20:36:59]

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