NQE: Always read prefs in cronet

NQE: Always read prefs in cronet

Always read the network quality estimator (NQE) prefs in Cronet, and
store the read prefs in the network quality store. This
ensures that prefs on the disk are not written over by empty prefs by
the store. 

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

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

[tbansal1, 2017-02-06 16:44:53]

Description was changed from

==========
NQE read prefs in cronet

BUG=
==========

to

==========
NQE read prefs in cronet

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

[tbansal1, 2017-02-06 16:45:41]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-06 16:46:02]

Dry run: 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-02-06 17:48:21]

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

[commit-bot: I haz the power, 2017-02-06 17:48:21]

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

[tbansal1, 2017-02-06 18:13:10]

Description was changed from

==========
NQE read prefs in cronet

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

to

==========
NQE: Always read prefs in cronet

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

[tbansal1, 2017-02-06 18:13:22]

Description was changed from

==========
NQE: Always read prefs in cronet

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

to

==========
NQE: Always read prefs in cronet

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

[tbansal1, 2017-02-06 18:13:47]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-06 18:14:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2017-02-06 19:19:44]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-06 19:20:16]

Dry run: 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-02-06 20:19:47]

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

[commit-bot: I haz the power, 2017-02-06 20:19:48]

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

[tbansal1, 2017-02-06 21:27:30]

Patchset #2 (id:20001) has been deleted

[tbansal1, 2017-02-06 21:27:39]

Patchset #2 (id:40001) has been deleted

[tbansal1, 2017-02-06 21:27:47]

Patchset #2 (id:60001) has been deleted

[tbansal1, 2017-02-06 21:28:00]

Patchset #1 (id:1) has been deleted

[tbansal1, 2017-02-07 05:32:40]

Patchset #1 (id:80001) has been deleted

[tbansal1, 2017-02-07 06:29:56]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 06:30:11]

Dry run: 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-02-07 06:32:13]

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

[commit-bot: I haz the power, 2017-02-07 06:32:13]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tbansal1, 2017-02-07 06:57:02]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 06:57:11]

Dry run: 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-02-07 07:04:27]

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

[commit-bot: I haz the power, 2017-02-07 07:04:28]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2017-02-07 07:22:42]

Description was changed from

==========
NQE: Always read prefs in cronet

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

to

==========
NQE: Always read prefs in cronet

Always read the network quality estimator (NQE) prefs in Cronet, and
store the read prefs in the network quality store. This
ensures that prefs on the disk are not written over by empty prefs by
the store. 

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

[tbansal1, 2017-02-07 18:11:50]

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

[tbansal1, 2017-02-07 18:11:50]

xunjieli: ptal. Thanks.

[xunjieli, 2017-02-07 21:05:59]

https://codereview.chromium.org/2671233003/diff/100001/components/cronet/andr...
File components/cronet/android/cronet_url_request_context_adapter.cc (right):

https://codereview.chromium.org/2671233003/diff/100001/components/cronet/andr...
components/cronet/android/cronet_url_request_context_adapter.cc:209: return
pref_service_->GetDictionary(kNetworkQualities)->CreateDeepCopy();
Does |pref_service_| return empty DictionaryValue if prefs are not yet loaded?

[tbansal1, 2017-02-09 17:36:26]

ptal. Thanks.

https://codereview.chromium.org/2671233003/diff/100001/components/cronet/andr...
File components/cronet/android/cronet_url_request_context_adapter.cc (right):

https://codereview.chromium.org/2671233003/diff/100001/components/cronet/andr...
components/cronet/android/cronet_url_request_context_adapter.cc:209: return
pref_service_->GetDictionary(kNetworkQualities)->CreateDeepCopy();
On 2017/02/07 21:05:59, xunjieli wrote:
> Does |pref_service_| return empty DictionaryValue if prefs are not yet loaded?

Yes, it will return an empty map. 
Prefs are read synchronously in Cronet and in the initial profile in Chrome, so
that should not be a problem.

[xunjieli, 2017-02-09 18:27:36]

lgtm

Talked to Tarun offline. NQE will revisit the decision to store observations in
user prefs, which might be loaded synchronously in Chrome and thus slow down
startup.

[tbansal1, 2017-02-09 18:31:37]

The CQ bit was checked by tbansal@chromium.org

[commit-bot: I haz the power, 2017-02-09 18:32:42]

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-02-09 19:27:30]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1486665097649630,
"parent_rev": "2de0058601feb0158f1b50a7348effb62de1bc86", "commit_rev":
"fad042aa5ace0033293dbbddcd7204fafccd4bdc"}

[commit-bot: I haz the power, 2017-02-09 19:28:09]

Description was changed from

==========
NQE: Always read prefs in cronet

Always read the network quality estimator (NQE) prefs in Cronet, and
store the read prefs in the network quality store. This
ensures that prefs on the disk are not written over by empty prefs by
the store. 

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

to

==========
NQE: Always read prefs in cronet

Always read the network quality estimator (NQE) prefs in Cronet, and
store the read prefs in the network quality store. This
ensures that prefs on the disk are not written over by empty prefs by
the store. 

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

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

[commit-bot: I haz the power, 2017-02-09 19:28:12]

Committed patchset #1 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/fad042aa5ace0033293dbbddcd72...