Adding a NetworkProfileHandler used by ManagedNetworkConfigurationHandler.

chromeos/dbus/shill_profile_client_stub.h

 // 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.
 
 #ifndef CHROMEOS_DBUS_SHILL_PROFILE_CLIENT_STUB_H_
 #define CHROMEOS_DBUS_SHILL_PROFILE_CLIENT_STUB_H_
 
+#include <map>
 #include <string>
 
 #include "base/basictypes.h"
+#include "chromeos/dbus/shill_manager_client.h"
 #include "chromeos/dbus/shill_profile_client.h"
 
 namespace chromeos {
 
 // A stub implementation of ShillProfileClient.
 class ShillProfileClientStub : public ShillProfileClient,
                                public ShillProfileClient::TestInterface {
  public:
-  ShillProfileClientStub();
+  explicit ShillProfileClientStub(
+      ShillManagerClient::TestInterface* manager_test);

[stevenjb, 2013/04/26 21:00:49]

I don't know that I like this. Is there ever a situation where we would
initialize ShillProfileClientStub() without initializing
ShillManagerClientStub()? i.e. can't ShillProfileClientStub just use
DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()?

[pneubeck (no reviews), 2013/04/29 18:05:51]

My thoughts were:
1.
managed_network_configuration_handler_unittest.cc is doing exactly that.
There, notifications from Shill and return values from calls to Shill have to be
consistent. With mocks that's rather cumbersome and repetitive.
Since this simple bookkeeping exists already in the Stubs, why not using it?
Of the ShillProfileClientStub it's using the storage of profile entries.
Of the NetworkProfileHandlerStub it's using the mapping between profiles and
users.

2.
Overall I find it rather unhealthy that the Stubs are mutually dependent on each
other. Because of that, we cannot use the Stubs one by one and cannot combine
Mocks with Stubs etc., but have to use all ClientStubs or none.
I see that that might be necessary for the ShillManagerClient but for the other
Clients?

[stevenjb, 2013/04/30 17:42:47]

I believe strongly that we *shouldn't* combine mocks with stubs, not within a
subdirectory (i.e chromeos/dbus) anyway. We should either use all of the shill
stubs, or none.

The stubs are consciously designed to work together to emulate Shill behavior
and not work independently. While some may work independently now, maintaining
that is going to be difficult.

What problems arise if you use ShillManagerClientStub?

[pneubeck (no reviews), 2013/05/03 17:32:55]

I have to use the ClientMocks in the unit test to verify exactly which calls are
done by the ConfigHandler.

I don't use the ProfileStub anymore, which required to add a seperate
ProfilesStub in the test file.

   virtual ~ShillProfileClientStub();
 
   // ShillProfileClient overrides.
   virtual void AddPropertyChangedObserver(
       const dbus::ObjectPath& profile_path,
       ShillPropertyChangedObserver* observer) OVERRIDE;
   virtual void RemovePropertyChangedObserver(
       const dbus::ObjectPath& profile_path,
       ShillPropertyChangedObserver* observer) OVERRIDE;
   virtual void GetProperties(
       const dbus::ObjectPath& profile_path,
       const DictionaryValueCallbackWithoutStatus& callback,
       const ErrorCallback& error_callback) OVERRIDE;
   virtual void GetEntry(const dbus::ObjectPath& profile_path,
                         const std::string& entry_path,
                         const DictionaryValueCallbackWithoutStatus& callback,
                         const ErrorCallback& error_callback) OVERRIDE;
   virtual void DeleteEntry(const dbus::ObjectPath& profile_path,
                            const std::string& entry_path,
                            const base::Closure& callback,
                            const ErrorCallback& error_callback) OVERRIDE;
   virtual ShillProfileClient::TestInterface* GetTestInterface() OVERRIDE;
 
   // ShillProfileClient::TestInterface overrides.
-  virtual void AddProfile(const std::string& profile_path) OVERRIDE;
+  virtual void AddProfile(const std::string& profile_path,
+                          const std::string& userhash) OVERRIDE;
   virtual void AddEntry(const std::string& profile_path,
                         const std::string& entry_path,
                         const base::DictionaryValue& properties) OVERRIDE;
   virtual bool AddService(const std::string& service_path) OVERRIDE;
 
  private:
-  base::DictionaryValue* GetProfile(const dbus::ObjectPath& profile_path,
-                                    const ErrorCallback& error_callback);
+  struct ProfileProperties;
+  typedef std::map<std::string, ProfileProperties*> ProfileMap;
 
-  // This maps profile path -> entry path -> Shill properties.
-  base::DictionaryValue profile_entries_;
+  ProfileProperties* GetProfile(const dbus::ObjectPath& profile_path,
+                                const ErrorCallback& error_callback);
+
+  // The values are owned by this class and are explicitly destroyed where
+  // necessary.
+  ProfileMap profiles_;
+
+  ShillManagerClient::TestInterface* manager_test_;
 
   DISALLOW_COPY_AND_ASSIGN(ShillProfileClientStub);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_DBUS_SHILL_PROFILE_CLIENT_STUB_H_