Add NetworkHandler to own network handlers in src/chromeos/network

chromeos/network/network_handler.cc

Index: chromeos/network/network_handler.cc
diff --git a/chromeos/network/network_handler.cc b/chromeos/network/network_handler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3f88031688f13dc670e4ea61c2743749d4e2400e
--- /dev/null
+++ b/chromeos/network/network_handler.cc
@@ -0,0 +1,185 @@
+// Copyright (c) 2013 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 "chromeos/network/network_handler.h"
+
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/login/login_state.h"
+#include "chromeos/network/cert_loader.h"
+#include "chromeos/network/geolocation_handler.h"
+#include "chromeos/network/managed_network_configuration_handler.h"
+#include "chromeos/network/network_configuration_handler.h"
+#include "chromeos/network/network_event_log.h"
+#include "chromeos/network/network_profile_handler.h"
+#include "chromeos/network/network_profile_observer.h"
+#include "chromeos/network/network_state_handler.h"
+#include "chromeos/network/network_state_handler_observer.h"
+
+namespace chromeos {
+
+static NetworkHandler* g_network_handler = NULL;
+
+class NetworkHandler::LoginStateInitializer {
+ public:
+  LoginStateInitializer() {
+    LoginState::Initialize();
+  }
+  ~LoginStateInitializer() {
+    LoginState::Shutdown();
+  }
+};
+
+NetworkHandler::NetworkHandler()
+    : test_cert_loader_(NULL),
+      test_network_state_handler_(NULL),
+      test_network_profile_handler_(NULL),
+      test_network_configuration_handler_(NULL),
+      test_managed_network_configuration_handler_(NULL),
+      test_geolocation_handler_(NULL) {
+  CHECK(DBusThreadManager::IsInitialized());
+
+  if (!LoginState::IsInitialized())
+    login_state_initializer_.reset(new LoginStateInitializer());
+
+  network_event_log::Initialize();
+
+  cert_loader_.reset(new CertLoader);

[pneubeck (no reviews), 2013/05/13 09:17:06]

I'd suggest to use explicit passing of the mutual dependencies, like

... = new ProfileHandler(state_handler_.get())
... = new ManagedNetworkConfigurationHandler(profile_handler_.get())
...

That would improve the unit tests and avoid dependency/initialization errors.

[stevenjb, 2013/05/13 23:49:35]

See meta comment

+  network_state_handler_.reset(new NetworkStateHandler());
+  network_profile_handler_.reset(new NetworkProfileHandler());
+  network_configuration_handler_.reset(new NetworkConfigurationHandler());
+  managed_network_configuration_handler_.reset(
+      new ManagedNetworkConfigurationHandler());
+  geolocation_handler_.reset(new GeolocationHandler());
+}
+
+NetworkHandler::~NetworkHandler() {
+  network_event_log::Shutdown();
+}
+
+void NetworkHandler::Init() {
+  network_state_handler_->InitShillPropertyHandler();
+  managed_network_configuration_handler_->Init();
+  geolocation_handler_->Init();
+}
+
+// static
+void NetworkHandler::Initialize() {
+  CHECK(!g_network_handler);
+  g_network_handler = new NetworkHandler();
+  g_network_handler->Init();
+}
+
+// static
+void NetworkHandler::Shutdown() {
+  CHECK(g_network_handler);
+  delete g_network_handler;
+  g_network_handler = NULL;
+}
+
+// static
+NetworkHandler* NetworkHandler::Get() {
+  CHECK(g_network_handler)
+      << "NetworkHandler::Get() called before Initialize()";
+  return g_network_handler;
+}
+
+// static
+bool NetworkHandler::IsInitialized() {
+  return g_network_handler;
+}
+
+CertLoader* NetworkHandler::cert_loader() {
+  if (test_cert_loader_)
+    return test_cert_loader_;
+  return cert_loader_.get();
+}
+
+NetworkStateHandler* NetworkHandler::network_state_handler() {
+  if (test_network_state_handler_)
+    return test_network_state_handler_;
+  return network_state_handler_.get();
+}
+
+NetworkProfileHandler* NetworkHandler::network_profile_handler() {
+  if (test_network_profile_handler_)
+    return test_network_profile_handler_;
+  return network_profile_handler_.get();
+}
+
+NetworkConfigurationHandler* NetworkHandler::network_configuration_handler() {
+  if (test_network_configuration_handler_)
+    return test_network_configuration_handler_;
+  return network_configuration_handler_.get();
+}
+
+ManagedNetworkConfigurationHandler*
+NetworkHandler::managed_network_configuration_handler() {
+  if (test_managed_network_configuration_handler_)
+    return test_managed_network_configuration_handler_;
+  return managed_network_configuration_handler_.get();
+}
+
+GeolocationHandler* NetworkHandler::geolocation_handler() {
+  if (test_geolocation_handler_)
+    return test_geolocation_handler_;
+  return geolocation_handler_.get();
+}
+
+void NetworkHandler::SetCertLoaderForTest(CertLoader* cert_loader) {
+  test_cert_loader_ = cert_loader;
+  // Copy any existing observers to the test handler.
+  if (cert_loader_.get()) {
+    ObserverList<CertLoader::Observer>::Iterator iter(cert_loader_->observers_);
+    CertLoader::Observer* observer;
+    while ((observer = iter.GetNext()) != NULL) {
+      test_cert_loader_->AddObserver(observer);

[pneubeck (no reviews), 2013/05/13 09:17:06]

I think, in this state, the Set*ForTest methods will behave in a rather
unexpected way.
There will be two handlers after a Set*ForTest call and both may have observers
and fire notifications.

The default handler should be destructed. But even then, there is no guarantee
that these calls will work reliably: The handlers might have done some work at
start up already. When these functions are called, the state of the handlers and
all related classes might be in an arbitrary state.

If possible, I strongly prefer using handler replacements from the very
beginning of a test instead of handing them in later.

[stevenjb, 2013/05/13 23:49:35]

If we exclusively use global accessors, then only one handler will ever be
active. This was just so that handlers that observe each other (and set that up
on init) work correctly with test implementations.

Alternately we can require that any test handlers be set up before
NetworkHandler::Init() gets called (we would need to call that explicitly
instead).

Since these are getting removed this is moot for now, we can decide this if/when
any of these get added.

+    }
+  }
+}
+
+void NetworkHandler::SetNetworkStateHandlerForTest(
+    NetworkStateHandler* network_state_handler) {
+  test_network_state_handler_ = network_state_handler;
+  // Copy any existing observers to the test handler.
+  if (network_state_handler_.get()) {
+    ObserverList<NetworkStateHandlerObserver>::Iterator iter(
+        network_state_handler_->observers_);
+    NetworkStateHandlerObserver* observer;
+    while ((observer = iter.GetNext()) != NULL) {
+      test_network_state_handler_->AddObserver(observer);
+    }
+  }
+}
+
+void NetworkHandler::SetNetworkProfileHandlerForTest(
+    NetworkProfileHandler* network_profile_handler) {
+  test_network_profile_handler_ = network_profile_handler;
+  // Copy any existing observers to the test handler.
+  if (network_profile_handler_.get()) {
+    ObserverList<NetworkProfileObserver>::Iterator iter(
+        network_profile_handler_->observers_);
+    NetworkProfileObserver* observer;
+    while ((observer = iter.GetNext()) != NULL) {
+      test_network_profile_handler_->AddObserver(observer);
+    }
+  }
+}
+
+void NetworkHandler::SetNetworkConfigurationHandlerForTest(
+    NetworkConfigurationHandler* network_configuration_handler) {
+  test_network_configuration_handler_ = network_configuration_handler;
+}
+
+void NetworkHandler::SetManagedNetworkConfigurationHandlerForTest(
+    ManagedNetworkConfigurationHandler* managed_network_configuration_handler) {
+  test_managed_network_configuration_handler_ =
+      managed_network_configuration_handler;
+}
+
+void NetworkHandler::SetGeolocationHandlerForTest(
+    GeolocationHandler* geolocation_handler) {
+  test_geolocation_handler_ = geolocation_handler;
+}
+
+}  // namespace chromeos