Implement policy for kNetworkPredictionOptions.

chrome/browser/policy/network_prediction_policy_handler.cc

Index: chrome/browser/policy/network_prediction_policy_handler.cc
diff --git a/chrome/browser/policy/network_prediction_policy_handler.cc b/chrome/browser/policy/network_prediction_policy_handler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9c51f5354a34303f0643216b65ead9214c00631f
--- /dev/null
+++ b/chrome/browser/policy/network_prediction_policy_handler.cc
@@ -0,0 +1,89 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/policy/network_prediction_policy_handler.h"
+
+#include "base/prefs/pref_value_map.h"
+#include "base/values.h"
+#include "chrome/browser/net/prediction_options.h"
+#include "chrome/common/content_settings.h"

[Joao da Silva, 2014/07/30 07:57:32]

Not used

[Bence, 2014/07/30 13:19:31]

Done.

+#include "chrome/common/pref_names.h"
+#include "components/policy/core/browser/policy_error_map.h"
+#include "components/policy/core/common/policy_map.h"
+#include "grit/components_strings.h"
+#include "policy/policy_constants.h"
+
+namespace policy {
+
+NetworkPredictionPolicyHandler::NetworkPredictionPolicyHandler() {
+}
+
+NetworkPredictionPolicyHandler::~NetworkPredictionPolicyHandler() {
+}
+
+bool NetworkPredictionPolicyHandler::CheckPolicySettings(
+    const PolicyMap& policies,
+    PolicyErrorMap* errors) {
+  // Deprecated boolean preference.
+  const base::Value* network_prediction_enabled =
+      policies.GetValue(key::kDnsPrefetchingEnabled);
+  // New enumerated preference.
+  const base::Value* network_prediction_options =
+      policies.GetValue(key::kNetworkPredictionOptions);
+
+  if (network_prediction_enabled &&
+      !network_prediction_enabled->IsType(base::Value::TYPE_BOOLEAN)) {
+    errors->AddError(key::kDnsPrefetchingEnabled,
+                     IDS_POLICY_TYPE_ERROR,
+                     ValueTypeToString(base::Value::TYPE_BOOLEAN));
+  }
+
+  if (network_prediction_options &&
+      !network_prediction_options->IsType(base::Value::TYPE_INTEGER)) {
+    errors->AddError(key::kNetworkPredictionOptions,
+                     IDS_POLICY_TYPE_ERROR,
+                     ValueTypeToString(base::Value::TYPE_INTEGER));
+  }
+
+  if (network_prediction_enabled && network_prediction_options) {
+    errors->AddError(key::kDnsPrefetchingEnabled,
+                     IDS_POLICY_OVERRIDDEN,
+                     key::kNetworkPredictionOptions);
+  }
+
+  return true;
+}
+
+void NetworkPredictionPolicyHandler::ApplyPolicySettings(
+    const PolicyMap& policies,
+    PrefValueMap* prefs) {
+  // If new preference is managed by policy, apply it to both preferences.
+  const base::Value* network_prediction_options =
+      policies.GetValue(key::kNetworkPredictionOptions);
+  if (network_prediction_options) {
+    int setting;
+    network_prediction_options->GetAsInteger(&setting);

[Joao da Silva, 2014/07/30 07:57:32]

This may fail, if the policy value isn't an integer. Suggestion:

  int setting;
  if (network_prediction_options &&
network_prediction_options->GetAsInteger(&settings) {
    ...
  }

[Bence, 2014/07/30 13:19:31]

Done.

+    prefs->SetInteger(prefs::kNetworkPredictionOptions, setting);
+    // Be conservative here and only set Enabled if policy says ALWAYS.
+    prefs->SetBoolean(prefs::kNetworkPredictionEnabled,
+                      setting == chrome_browser_net::NETWORK_PREDICTION_ALWAYS);
+    return;
+  }
+
+  // If deprecated preference is managed by policy, apply it to both
+  // preferences.
+  const base::Value* network_prediction_enabled =
+      policies.GetValue(key::kDnsPrefetchingEnabled);
+  if (network_prediction_enabled) {
+    bool setting;
+    network_prediction_enabled->GetAsBoolean(&setting);

[Joao da Silva, 2014/07/30 07:57:32]

Same here

[Bence, 2014/07/30 13:19:31]

Done.

+    prefs->SetBoolean(prefs::kNetworkPredictionEnabled, setting);
+    // Be conservative here and set Options to WIFI_ONLY.

[Joao da Silva, 2014/07/30 07:57:32]

The legacy semantic was equivalent to ALWAYS and I think we should keep that. Do
you have a reason to prefer WIFI_ONLY here?

[Bence, 2014/07/30 13:19:31]

Good question, sorry it is not clear from comments.  Yes, there is a reason:
some of the activities that are now controlled by the "Predictive network
action" preference used to be hardcoded never to run on cellular network.  For
example, prefetch used to do that, see left side of 
https://codereview.chromium.org/357413004/diff/20001/chrome/browser/prefetch/...
for original.  It was a mistake that I removed that check there, and it is being
reinstated:
https://codereview.chromium.org/419833003/diff/40001/chrome/browser/prefetch/....

Once we deprecate the old Boolean setting, we'll get rid of all this logic, and
allow prefetch on cellular network, but only if the user or the policy
explicitly asks for it using the new preference.  Therefore I suggest that the
old policy gets translated to WIFI_ONLY.

+    prefs->SetInteger(prefs::kNetworkPredictionOptions,
+                      setting ? chrome_browser_net::NETWORK_PREDICTION_WIFI_ONLY
+                              : chrome_browser_net::NETWORK_PREDICTION_NEVER);
+  }
+}
+
+}  // namespace policy