Add network throttling as an enterprise policy

chrome/browser/chromeos/net/network_throttling_observer.cc

+// Copyright 2016 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/chromeos/net/network_throttling_observer.h"
+
+#include <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/sys_info.h"
+#include "base/values.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile.h"

[stevenjb, 2016/10/24 17:14:16]

Needed?

[kirtika1, 2016/10/27 18:15:42]

Removed.

+#include "chrome/common/pref_names.h"
+#include "chromeos/network/network_state_handler.h"
+#include "components/prefs/pref_member.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+
+namespace chromeos {
+
+namespace {
+
+// Weak pointer. This class is owned by ChromeBrowserMainPartsChromeos.
+NetworkThrottlingObserver* g_network_throttling_observer = nullptr;

[stevenjb, 2016/10/24 17:14:16]

You shouldn't need this any more.

[kirtika1, 2016/10/27 18:15:42]

Done.

+
+}  // namespace
+
+NetworkThrottlingObserver::NetworkThrottlingObserver()
+    : weak_ptr_factory_(this) {
+  DCHECK(!g_network_throttling_observer);
+  g_network_throttling_observer = this;
+
+  pref_change_registrar_.Init(g_browser_process->local_state());

[stevenjb, 2016/10/24 17:14:16]

Instead of accessing g_browser_process, pass g_browser_process->local_state() to
the constructor as a PrefService* and store it as a member, e.g. local_state_
for use in OnPreferenceChanged. That way this class does not have any
chrome/browser dependencies and we can more easily move it later if we decide we
need to.

[kirtika1, 2016/10/27 18:15:42]

Done.

+
+  base::Callback<void(const std::string&)> throttle_callback = base::Bind(
+      &NetworkThrottlingObserver::OnPreferenceChanged, base::Unretained(this));

[stevenjb, 2016/10/24 17:14:16]

base::Unretained is dangerous here. If this class is destroyed before
PrefService the callback could occur after the class is destroyed. You need to
use base::WeakPtr here (See WakeOnWifiManager for an example).

[kirtika1, 2016/10/27 18:15:42]

Done. 
I could follow your explanation for this specific case, but am still confused
about general usage. 
This link does not list WeakPtr as a possible option, though lots of Chromium
(and Shill, and others) code uses that binding?
https://cs.chromium.org/chromium/src/base/bind_helpers.h?q=base::Unretained&s...

+
+  pref_change_registrar_.Add(prefs::kNetworkThrottlingEnabled,
+                             throttle_callback);
+}
+
+NetworkThrottlingObserver::~NetworkThrottlingObserver() {
+  DCHECK(g_network_throttling_observer);
+  g_network_throttling_observer = nullptr;
+}
+
+void NetworkThrottlingObserver::RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterDictionaryPref(prefs::kNetworkThrottlingEnabled);
+}
+
+void NetworkThrottlingObserver::OnPreferenceChanged(
+    const std::string& pref_name) {
+  DCHECK(pref_name == prefs::kNetworkThrottlingEnabled);
+
+  const base::DictionaryValue* throttling_policy =
+      g_browser_process->local_state()->GetDictionary(
+          prefs::kNetworkThrottlingEnabled);
+
+  if (throttling_policy) {

[stevenjb, 2016/10/24 17:14:16]

nit: early exit:
if (!throttling_policy)
  return;

[kirtika1, 2016/10/27 18:15:42]

Done.

+    bool enabled;
+    uint32_t upload_rate, download_rate;
+    throttling_policy->GetBoolean("enabled", &enabled);
+    throttling_policy->GetInteger("upload_rate_kbits",
+                                  reinterpret_cast<int*>(&upload_rate));
+    throttling_policy->GetInteger("download_rate_kbits",
+                                  reinterpret_cast<int*>(&download_rate));
+    NetworkHandler::Get()->network_state_handler()->SetNetworkThrottlingStatus(
+        enabled, upload_rate, download_rate);
+  }
+}
+
+}  // namespace chromeos