[Re-land] Add network throttling as 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..77e8786c63afc3fa5326e34455d507a6c7994c2e
--- /dev/null
+++ b/chrome/browser/chromeos/net/network_throttling_observer_unittest.cc
@@ -0,0 +1,84 @@
+// Copyright (c) 2016 The Chromium Authors. All rights reserved.

[Lei Zhang, 2016/11/03 19:38:39]

nit: no (c) 

[kirtika1, 2016/11/04 00:27:32]

Done.

+// 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();
+    std::unique_ptr<DBusThreadManagerSetter> dbus_setter =
+        DBusThreadManager::GetSetterForTesting();

[Lei Zhang, 2016/11/03 19:38:39]

Previously, you had:

DBusThreadManager::Initialize();
DBusThreadManager::GetSetterForTesting();

That's 2 DBusThreadManager initializations / 0 shutdowns.
Now you have 2 DBusThreadManager initializations / 1 shutdowns -> still leaking.

Drop the DBusThreadManager::Initialize(), you don't need it.

[kirtika1, 2016/11/04 00:27:32]

Done.

+    // Owned by DBusThreadManager, which takes care of cleaning it
+    mock_manager_client_ = new NiceMock<MockShillManagerClient>();
+    dbus_setter->SetShillManagerClient(
+        std::unique_ptr<ShillManagerClient>(mock_manager_client_));
+    NetworkStateHandler::InitializeForTest();

[Lei Zhang, 2016/11/03 19:38:39]

You need to take the returned pointer and free it later. Look at every existing
caller and see what they do. Note that you have to explicitly free it before
DBusThreadManager::Shutdown() happens.

[kirtika1, 2016/11/04 00:27:32]

Done.

+    NetworkHandler::Initialize();
+    local_state_ = new TestingPrefServiceSimple();

[Lei Zhang, 2016/11/03 19:38:39]

Who frees this? Did you try freeing it before? Did that fail and then you
decided it was "better" to not free it?

You should turn |local_state_| into an unique_ptr. However, the tricky bit here
is that |observer_| depends on |local_state_|, so you have to declare
|local_state_| before |observer_|. That way, when the class gets destroyed,
|observer_| gets destroyed first. The other way around does not work and results
in an use-after-free.

+    local_state_->registry()->RegisterDictionaryPref(
+        prefs::kNetworkThrottlingEnabled);
+    observer_ = base::MakeUnique<NetworkThrottlingObserver>(local_state_);
+  }
+
+  ~NetworkThrottlingObserverTest() override {
+    NetworkHandler::Shutdown();
+    DBusThreadManager::Shutdown();
+  }
+
+  std::unique_ptr<base::MessageLoop> message_loop_;

[Lei Zhang, 2016/11/03 19:38:39]

Just drop the std::unique_ptr.

[kirtika1, 2016/11/04 00:27:32]

Done.

+  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