NQE: Use cached estimates

chrome/browser/net/nqe/ui_network_quality_estimator_service.cc

Index: chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
diff --git a/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc b/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
index 0ebcc93c5686f271aac3e322aae657259ba9ecf4..d0aada131d3fd13aa17ae48a3d19c09dd4db8db3 100644
--- a/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
+++ b/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
@@ -39,10 +39,18 @@ std::string GetStringValueForVariationParamWithDefaultValue(
                                                       : it->second;
 }
 
-// Returns true if persistent caching has been enabled in the field trial.
-bool persistent_caching_enabled() {
+// Returns true if writing to the persistent cache has been enabled via field
+// trial.
+bool persistent_cache_writing_enabled() {

[RyanSturm, 2016/12/13 21:08:35]

Is there really a need for both of these field trial params? I feel like we
should be able to either read and write or do neither.

[tbansal1, 2016/12/14 01:37:37]

Yeah, I do not know of a better way. I want to enable writing for all users
(ramp up gradually), and then enable reading for a subset of users and compare
with Control.

If I enabled writing and reading for a small subset of users, then it is
possible that reading the prefs gives them very old prefs, which may be useless.

   return GetStringValueForVariationParamWithDefaultValue(
-             "persistent_caching_enabled", "false") == "true";
+             "persistent_cache_writing_enabled", "false") == "true";
+}
+
+// Returns true if reading from the persistent cache has been enabled via field
+// trial.
+bool persistent_cache_reading_enabled() {
+  return GetStringValueForVariationParamWithDefaultValue(
+             "persistent_cache_reading_enabled", "false") == "true";
 }
 
 // PrefDelegateImpl writes the provided dictionary value to the network quality
@@ -59,17 +67,19 @@ class PrefDelegateImpl
 
   void SetDictionaryValue(const base::DictionaryValue& value) override {
     DCHECK(thread_checker_.CalledOnValidThread());
-    if (!persistent_caching_enabled())
+    if (!persistent_cache_writing_enabled())
       return;
 
     pref_service_->Set(path_, value);
     UMA_HISTOGRAM_COUNTS_1000("NQE.Prefs.WriteCount", 1);
   }
 
-  const base::DictionaryValue& GetDictionaryValue() override {
+  std::unique_ptr<base::DictionaryValue> GetDictionaryValue() override {
     DCHECK(thread_checker_.CalledOnValidThread());
+    if (!persistent_cache_reading_enabled())
+      return base::WrapUnique(new base::DictionaryValue());
     UMA_HISTOGRAM_COUNTS_1000("NQE.Prefs.ReadCount", 1);
-    return *pref_service_->GetDictionary(path_);
+    return pref_service_->GetDictionary(path_)->CreateDeepCopy();
   }
 
  private: