Reimplement Libcros fucntions using FlimflamProfileClient

Reimplement Libcros fucntions using FlimflamProfileClient

Change FlimflamProfileClient's interface to deal with multiple profile objects
Change FlimflamProfileClient's interface to send an entry's path as a string, not an ObjectPath
Add non-Libcros implementations of CrosDeleteServiceFromProfile, CrosRequestNetworkProfileProperties and CrosRequestNetworkProfileEntryProperties
Add EnableNonLibcrosNetworkFunctions
Add --enable-non-libcros-network-functions commandline option
Rename existing CrosNetworkFunctionsTest to CrosNetworkFunctionsLibcrosTest
Add new CrosNetworkFunctionsTest which tests the non Libcros implementation

BUG=chromium-os:16557
TEST=unit_tests --gtest_filter="CrosNetworkFunctions*"

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=132502

[hashimoto, 2012-04-12 09:16:14]

Changing the FlimflamProfileClient's interface to deal with multiple profiles.
Also changing the type of Get/DeleteEntry's argument from ObjectPath to string
because the implementation is using string type (http://goo.gl/LtI6H) even if
the flimflam doc says it's an object path (http://goo.gl/IE2V9).

[stevenjb, 2012-04-12 17:34:36]

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/cros_network_functions.cc (right):

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/cros_network_functions.cc:106: return;
I don't think this is a good place for early-exit style. I think all of this
code will be more readable as:
if (enable_libcros_functions) {
} else {
}

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/cros_network_functions.cc:201: // Use Libcros.
unnecessary comment here and elsewhere

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/cros_network_functions.h (right):

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/cros_network_functions.h:32: void
EnableNonLibcrosNetworkFunctions(bool enabled);
"EnableNon" is extremely confusing, should be SetLibcrosNetworkFunctionsEnabled.

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/cros_network_functions_unittest.cc (right):

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/cros_network_functions_unittest.cc:45: :
expected_result_(expected_result) {}
indent : 4 spaces

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/network_library_impl_cros.cc (right):

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/network_library_impl_cros.cc:49:
"enable-non-libcros-network-functions";
This should be defined in chrome_switches.h, and should be
"enable-dbus-network-fucntions" or "disable-libcros-network-fucntions" (or just
"disable-libcros").

http://codereview.chromium.org/9958045/diff/17002/chromeos/dbus/flimflam_prof...
File chromeos/dbus/flimflam_profile_client.cc (right):

http://codereview.chromium.org/9958045/diff/17002/chromeos/dbus/flimflam_prof...
chromeos/dbus/flimflam_profile_client.cc:42: typedef std::map<std::string,
FlimflamClientHelper*> HelpersMap;
s/HelpersMap/HelperMap/

[hashimoto, 2012-04-13 05:27:22]

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/cros_network_functions.cc (right):

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/cros_network_functions.cc:106: return;
On 2012/04/12 17:34:36, stevenjb (chromium) wrote:
> I don't think this is a good place for early-exit style. I think all of this
> code will be more readable as:
> if (enable_libcros_functions) {
> } else {
> }

I thought new (non libcros) implementations may be complex and it's good to
avoid indenting the new implementations.
But it's OK if you prefer the style where everything is placed in if-else
braces.

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/cros_network_functions.cc:201: // Use Libcros.
On 2012/04/12 17:34:36, stevenjb (chromium) wrote:
> unnecessary comment here and elsewhere

Done.

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/cros_network_functions.h (right):

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/cros_network_functions.h:32: void
EnableNonLibcrosNetworkFunctions(bool enabled);
On 2012/04/12 17:34:36, stevenjb (chromium) wrote:
> "EnableNon" is extremely confusing, should be
SetLibcrosNetworkFunctionsEnabled.

Done.

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/cros_network_functions_unittest.cc (right):

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/cros_network_functions_unittest.cc:45: :
expected_result_(expected_result) {}
On 2012/04/12 17:34:36, stevenjb (chromium) wrote:
> indent : 4 spaces

Done.

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
File chrome/browser/chromeos/cros/network_library_impl_cros.cc (right):

http://codereview.chromium.org/9958045/diff/17002/chrome/browser/chromeos/cro...
chrome/browser/chromeos/cros/network_library_impl_cros.cc:49:
"enable-non-libcros-network-functions";
On 2012/04/12 17:34:36, stevenjb (chromium) wrote:
> This should be defined in chrome_switches.h, and should be
> "enable-dbus-network-fucntions" or "disable-libcros-network-fucntions" (or
just
> "disable-libcros").

I thought we should be as accurate as possible, but "disable-libcros" looks much
simpler.

http://codereview.chromium.org/9958045/diff/17002/chromeos/dbus/flimflam_prof...
File chromeos/dbus/flimflam_profile_client.cc (right):

http://codereview.chromium.org/9958045/diff/17002/chromeos/dbus/flimflam_prof...
chromeos/dbus/flimflam_profile_client.cc:42: typedef std::map<std::string,
FlimflamClientHelper*> HelpersMap;
On 2012/04/12 17:34:36, stevenjb (chromium) wrote:
> s/HelpersMap/HelperMap/

Done.

[hashimoto, 2012-04-16 03:21:05]

ping?

[stevenjb, 2012-04-17 00:17:44]

LGTM

http://codereview.chromium.org/9958045/diff/24001/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/9958045/diff/24001/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:1212: const char kDisableLibcros[] =
"disable-libcros";
nit: align with others

[hashimoto, 2012-04-17 00:55:18]

http://codereview.chromium.org/9958045/diff/24001/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/9958045/diff/24001/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:1212: const char kDisableLibcros[] =
"disable-libcros";
On 2012/04/17 00:17:44, stevenjb (chromium) wrote:
> nit: align with others

Done.