[Cronet] Respect user agent string from config

[Cronet] Respect user agent string from config

BUG=456424
Committed: https://crrev.com/0ad1f3c9bec3b812809d515e377c84992f8a227c
Cr-Commit-Position: refs/heads/master@{#320828}

[xunjieli, 2015-03-10 17:53:37]

xunjieli@chromium.org changed reviewers:
+ mef@chromium.org

[xunjieli, 2015-03-13 18:25:23]

pinging Misha. thanks!

[mef, 2015-03-16 15:39:55]

Thanks for doing this!

https://codereview.chromium.org/997623002/diff/20001/components/cronet/androi...
File
components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestFactory.java
(right):

https://codereview.chromium.org/997623002/diff/20001/components/cronet/androi...
components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestFactory.java:27:
if (!userAgent.isEmpty()) {
Suggest changing to: 

if (userAgent.isEmpty())
  userAgent = UserAgent.from(context);

and removing else clause.

[xunjieli, 2015-03-16 15:45:56]

Thanks for the review!

https://codereview.chromium.org/997623002/diff/20001/components/cronet/androi...
File
components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestFactory.java
(right):

https://codereview.chromium.org/997623002/diff/20001/components/cronet/androi...
components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestFactory.java:27:
if (!userAgent.isEmpty()) {
On 2015/03/16 15:39:55, mef wrote:
> Suggest changing to: 
> 
> if (userAgent.isEmpty())
>   userAgent = UserAgent.from(context);
> 
> and removing else clause.

Done.

[mef, 2015-03-16 15:59:39]

lg, it also needs to be done for httpurlconnection-based implementation:
https://code.google.com/p/chromium/codesearch#chromium/src/components/cronet/...

[xunjieli, 2015-03-16 16:22:13]

On 2015/03/16 15:59:39, mef wrote:
> lg, it also needs to be done for httpurlconnection-based implementation:
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/cronet/...

Done. I passed the config to the request constructor. I wonder if it's better to
pass the user agent string. I don't have much a preference. Let me know what you
think.

[mef, 2015-03-16 16:53:47]

On 2015/03/16 16:22:13, xunjieli wrote:
> On 2015/03/16 15:59:39, mef wrote:
> > lg, it also needs to be done for httpurlconnection-based implementation:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/cronet/...
> 
> Done. I passed the config to the request constructor. I wonder if it's better
to
> pass the user agent string. I don't have much a preference. Let me know what
you
> think.

FI would extract userAgent from config into mDefaultUserAgent in context
constructor, and pass it as a string to request constructor.

[xunjieli, 2015-03-16 17:29:08]

On 2015/03/16 16:53:47, mef wrote:
> On 2015/03/16 16:22:13, xunjieli wrote:
> > On 2015/03/16 15:59:39, mef wrote:
> > > lg, it also needs to be done for httpurlconnection-based implementation:
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/cronet/...
> > 
> > Done. I passed the config to the request constructor. I wonder if it's
better
> to
> > pass the user agent string. I don't have much a preference. Let me know what
> you
> > think.
> 
> FI would extract userAgent from config into mDefaultUserAgent in context
> constructor, and pass it as a string to request constructor.

Done. thanks

[mef, 2015-03-16 21:56:38]

lgtm

[xunjieli, 2015-03-16 23:11:53]

The CQ bit was checked by xunjieli@chromium.org

[commit-bot: I haz the power, 2015-03-16 23:12:15]

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

[commit-bot: I haz the power, 2015-03-17 00:19:05]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-03-17 00:19:57]

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