Make ParseAndSetExperimentalOptions() a member of URLRequestContextConfig

Make ParseAndSetExperimentalOptions() a member of URLRequestContextConfig

It didn't previously have a way of handling experimental options which
don't translate to settings on the URLRequestContextBuilder. This change
allows it to directly change settings on the URLRequestContextConfig so
those settings can be used outside of the builder.

This struct shouldn't really be a struct anymore, but it already
shouldn't really have been a struct. This CL is the minimum change
needed to unblock an upcoming feature plus some closely related cleanup.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2952653002
Cr-Commit-Position: refs/heads/master@{#480996}
Committed: https://chromium.googlesource.com/chromium/src/+/cc6ae89f12dca8195dc58251a404de1411d89d46

[mgersh, 2017-06-20 16:30:29]

Description was changed from

==========
Make ParseAndSetExperimentalOptions() a member of URLRequestContextConfig

It didn't previously have a way of handling experimental options which
don't translate to settings on the URLRequestContextBuilder. This change
allows it to directly change settings on the URLRequestContextConfig so
those settings can be used outside of the builder.

This struct shouldn't really be a struct anymore, but it already
shouldn't really have been a struct. This CL is the minimum change
needed to unblock an upcoming feature plus some closely related cleanup.
==========

to

==========
Make ParseAndSetExperimentalOptions() a member of URLRequestContextConfig

It didn't previously have a way of handling experimental options which
don't translate to settings on the URLRequestContextBuilder. This change
allows it to directly change settings on the URLRequestContextConfig so
those settings can be used outside of the builder.

This struct shouldn't really be a struct anymore, but it already
shouldn't really have been a struct. This CL is the minimum change
needed to unblock an upcoming feature plus some closely related cleanup.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[mgersh, 2017-06-20 16:32:16]

mgersh@chromium.org changed reviewers:
+ kapishnikov@chromium.org

[mgersh, 2017-06-20 16:32:17]

PTAL. (The way the diff displays is a little weird--it thinks I moved a bunch of
shorter things, but what I really did is move one very long thing.)

[kapishnikov, 2017-06-20 19:34:08]

Should we break the ParseAndSetExperimentalOptions() method into two methods.

The first method should be responsible for parsing the experimental options and
storing them in private config member/members. This method should not get
URLRequestContextBuilder as the input parameter.

The second method should be responsible for getting already parsed values and
populating URLRequestContextBuilder with them. The second method can be a
submethod of ConfigureURLRequestContextBuilder(). WDYT?

https://codereview.chromium.org/2952653002/diff/1/components/cronet/url_reque...
File components/cronet/url_request_context_config.h (right):

https://codereview.chromium.org/2952653002/diff/1/components/cronet/url_reque...
components/cronet/url_request_context_config.h:120: void
ParseAndSetExperimentalOptions(
This method looks internal to URLRequestContextConfig and should not exposed as
public. What other code is going to use it rather than
ConfigureURLRequestContextBuilder()?

[mgersh, 2017-06-20 21:33:57]

Per in-person conversation, leaving the overall structure as-is for now--it's
hard to separate parsing and configuration without creating other problems like
excessive duplication of the list of experiments.

Filed crbug.com/735207.

https://codereview.chromium.org/2952653002/diff/1/components/cronet/url_reque...
File components/cronet/url_request_context_config.h (right):

https://codereview.chromium.org/2952653002/diff/1/components/cronet/url_reque...
components/cronet/url_request_context_config.h:120: void
ParseAndSetExperimentalOptions(
On 2017/06/20 19:34:08, kapishnikov wrote:
> This method looks internal to URLRequestContextConfig and should not exposed
as
> public. What other code is going to use it rather than
> ConfigureURLRequestContextBuilder()?

Made it private.

[kapishnikov, 2017-06-20 21:35:55]

lgtm

[mgersh, 2017-06-20 21:36:53]

The CQ bit was checked by mgersh@chromium.org

[commit-bot: I haz the power, 2017-06-20 21:37:33]

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, 2017-06-20 21:50:10]

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

[commit-bot: I haz the power, 2017-06-20 21:50:11]

Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)

[mgersh, 2017-06-20 22:02:21]

The CQ bit was checked by mgersh@chromium.org

[mgersh, 2017-06-20 22:02:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
kapishnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2952653002/#ps40001
(title: "oops")

[commit-bot: I haz the power, 2017-06-20 22:02:40]

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, 2017-06-20 22:33:55]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1497996141222630,
"parent_rev": "ae12b73ccecb1b3aa184572b82b0a028866e2ebb", "commit_rev":
"cc6ae89f12dca8195dc58251a404de1411d89d46"}

[commit-bot: I haz the power, 2017-06-20 22:34:09]

Description was changed from

==========
Make ParseAndSetExperimentalOptions() a member of URLRequestContextConfig

It didn't previously have a way of handling experimental options which
don't translate to settings on the URLRequestContextBuilder. This change
allows it to directly change settings on the URLRequestContextConfig so
those settings can be used outside of the builder.

This struct shouldn't really be a struct anymore, but it already
shouldn't really have been a struct. This CL is the minimum change
needed to unblock an upcoming feature plus some closely related cleanup.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Make ParseAndSetExperimentalOptions() a member of URLRequestContextConfig

It didn't previously have a way of handling experimental options which
don't translate to settings on the URLRequestContextBuilder. This change
allows it to directly change settings on the URLRequestContextConfig so
those settings can be used outside of the builder.

This struct shouldn't really be a struct anymore, but it already
shouldn't really have been a struct. This CL is the minimum change
needed to unblock an upcoming feature plus some closely related cleanup.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2952653002
Cr-Commit-Position: refs/heads/master@{#480996}
Committed:
https://chromium.googlesource.com/chromium/src/+/cc6ae89f12dca8195dc58251a404...
==========

[commit-bot: I haz the power, 2017-06-20 22:34:10]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/cc6ae89f12dca8195dc58251a404...