Add network throttling as an enterprise policy

chrome/browser/chromeos/net/network_throttling_observer_unittest.cc

Index: chrome/browser/chromeos/net/network_throttling_observer_unittest.cc
diff --git a/chrome/browser/chromeos/net/network_throttling_observer_unittest.cc b/chrome/browser/chromeos/net/network_throttling_observer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7cbcfc27a562b55cf0aaea6d1521050bac060769
--- /dev/null
+++ b/chrome/browser/chromeos/net/network_throttling_observer_unittest.cc
@@ -0,0 +1,80 @@
+// Copyright (c) 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 <memory>
+
+#include "base/message_loop/message_loop.h"
+#include "chrome/browser/chromeos/net/network_throttling_observer.h"
+#include "chrome/common/pref_names.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/mock_shill_manager_client.h"
+#include "chromeos/network/network_state_handler.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/testing_pref_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::_;
+using testing::Mock;
+using testing::NiceMock;
+
+namespace chromeos {
+
+namespace test {
+
+class NetworkThrottlingObserverTest : public ::testing::Test {
+ public:
+  NetworkThrottlingObserverTest() : message_loop_(new base::MessageLoop()) {
+    DBusThreadManager::Initialize();

[Lei Zhang, 2016/10/29 16:04:05]

This is missing a matching call to DBusThreadManager::Shutdown(); at the end.

Thus there is a red bot here complaining about the resulting memory leak(s):
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...

[kirtika1, 2016/10/29 21:26:38]

Naive question - is adding DBusThreadManager::Shutdown to the destructor enough?
This file is expected to contain only one test until NetworkThrottlingObserver's
functionality changes. 
My assumption is that as long as cleanup happens at the end of the test, it
should be fine - does that assumption and the fix patch look right?

+    std::unique_ptr<DBusThreadManagerSetter> dbus_setter =
+        DBusThreadManager::GetSetterForTesting();
+    mock_manager_client_ = new NiceMock<MockShillManagerClient>();
+    dbus_setter->SetShillManagerClient(
+        std::unique_ptr<ShillManagerClient>(mock_manager_client_));

[Lei Zhang, 2016/10/29 16:04:04]

I'm rather worried about this. Is this telling SetShillManagerClient() that it
can take ownership of|mock_manager_client_| ? Because |mock_manager_client_|
also gets deleted on line 43.

[kirtika1, 2016/10/29 21:26:38]

Done.  

+    NetworkStateHandler::InitializeForTest();
+    NetworkHandler::Initialize();
+    local_state_ = new TestingPrefServiceSimple();
+    local_state_->registry()->RegisterDictionaryPref(
+        prefs::kNetworkThrottlingEnabled);
+    observer_.reset(new NetworkThrottlingObserver(local_state_));

[Lei Zhang, 2016/10/29 16:04:04]

nit: foo.reset(new Foo(bar)) -> foo = base::MakeUnique<Foo>(bar)

[kirtika1, 2016/10/29 21:26:38]

Done.

+  }
+
+  ~NetworkThrottlingObserverTest() override { delete mock_manager_client_; }
+
+  std::unique_ptr<base::MessageLoop> message_loop_;
+  std::unique_ptr<NetworkThrottlingObserver> observer_;
+  TestingPrefServiceSimple* local_state_;
+  MockShillManagerClient* mock_manager_client_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NetworkThrottlingObserverTest);
+};
+
+TEST_F(NetworkThrottlingObserverTest, ThrottlingChangeCallsShill) {
+  // Test that a change in the throttling policy value leads to
+  // shill_manager_client being called.
+  base::DictionaryValue updated_throttling_policy;
+  bool enabled = true;
+  uint32_t upload_rate = 1200;
+  uint32_t download_rate = 2000;
+  updated_throttling_policy.SetBoolean("enabled", enabled);
+  updated_throttling_policy.SetInteger("upload_rate_kbits", upload_rate);
+  updated_throttling_policy.SetInteger("download_rate_kbits", download_rate);
+  EXPECT_CALL(
+      *mock_manager_client_,
+      SetNetworkThrottlingStatus(enabled, upload_rate, download_rate, _, _))
+      .Times(1);
+  local_state_->Set(prefs::kNetworkThrottlingEnabled,
+                    updated_throttling_policy);
+  Mock::VerifyAndClearExpectations(mock_manager_client_);
+
+  // Clearing the preference should disable throttling
+  EXPECT_CALL(*mock_manager_client_,
+              SetNetworkThrottlingStatus(false, 0, 0, _, _))
+      .Times(1);
+  local_state_->ClearPref(prefs::kNetworkThrottlingEnabled);
+}
+
+}  // namespace test
+}  // namespace chromeos