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 "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+#include "chromeos/login/login_state.h"
+#include "chromeos/network/device_state.h"
+#include "chromeos/network/network_state_handler.h"
+#include "components/prefs/pref_registry_simple.h"
+
+namespace chromeos {
+
+namespace {
+
+// Weak pointer.  This class is owned by ChromeBrowserMainPartsChromeos.
+NetworkThrottlingObserver* g_network_throttling_observer = NULL;
+
+}  // namespace
+
+namespace network_throttling_observer {
+
+void RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterDictionaryPref(prefs::kNetworkThrottlingEnabled);
+}

[stevenjb, 2016/10/21 19:41:20]

nit: blank line

[kirtika1, 2016/10/23 00:04:49]

Done.

+}
+
+NetworkThrottlingObserver::NetworkThrottlingObserver()
+    : weak_ptr_factory_(this) {
+  // TODO(kirtika): Running on ChromeOS and user not logged in requirement
+  // borrowed from wake_on_wifi_manager - check if it is really needed here.

[Andrew T Wilson (Slow), 2016/10/21 13:27:30]

What is this? What is the purpose here? Let's not check this in without
explaining what this code does.

[stevenjb, 2016/10/21 19:41:20]

This should only be necessary if network throttling needs to be applied to the
user profile in Shill (instead of the device profile). That is the case for wake
on wifi which is why it does this check. Either remove this or update the
comment.

[stevenjb, 2016/10/21 20:24:00]

I see now that the policy code is specifically in
device_policy_decoder_chromeos.cc, so that seems fine. We shouldn't need the
logged in state check here in that case.

[kirtika1, 2016/10/23 00:04:49]

Done.

+  CHECK(!base::SysInfo::IsRunningOnChromeOS() ||
+        !LoginState::Get()->IsUserLoggedIn());
+  DCHECK(!g_network_throttling_observer);
+
+  g_network_throttling_observer = this;
+}
+
+NetworkThrottlingObserver::~NetworkThrottlingObserver() {
+  DCHECK(g_network_throttling_observer);
+  g_network_throttling_observer = NULL;

[stevenjb, 2016/10/21 19:41:20]

s/NULL/nullptr (but see other comments)

[kirtika1, 2016/10/23 00:04:49]

Done.

+}
+
+// static
+NetworkThrottlingObserver* NetworkThrottlingObserver::Get() {
+  DCHECK(g_network_throttling_observer);

[stevenjb, 2016/10/21 19:41:20]

Use CHECK instead of DCHECK, we want to make sure global singletons are never
accessed after destruction.

[kirtika1, 2016/10/23 00:04:49]

Done.

+  return g_network_throttling_observer;
+}
+
+void NetworkThrottlingObserver::OnPreferenceChanged(
+    bool throttling_enabled,
+    uint32_t upload_rate_kbits,
+    uint32_t download_rate_kbits) {
+  NetworkHandler::Get()->network_state_handler()->SetNetworkThrottlingStatus(
+      throttling_enabled, upload_rate_kbits, download_rate_kbits);

[stevenjb, 2016/10/21 19:41:20]

This class isn't really an observer, i.e. it's not implementing some other
Observer base class, so it is not clear to me that we even need it. Why not just
call NetworkStateHandler::SetNetworkThrottlingStatus directly instead of
NetworkThrottlingObserver::OnPreferenceChanged()?

+}
+
+}  // namespace chromeos