Use new GValue chromeos_network calls instead of base::Value

Use new GValue chromeos_network calls instead of base::Value

BUG=chromium-os:19576
TEST=Requires updated libcros (166). Ensure tests pass. Thoroughly test Network UI.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98737

[stevenjb, 2011-08-27 00:35:39]

This CL is dependent on:
http://gerrit.chromium.org/gerrit/#change,6786
+ a DEPS roll.

[tony, 2011-08-29 17:01:22]

I see, you're just moving the conversion from within libcros into chrome.  Seems
fine to me.

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:161: void
AppendListElement(const GValue *gvalue, ::gpointer user_data) {
Nit: GValue* gvalue.  I would probably also drop the :: since we don't seem to
do that with other uses of gpointer.

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:169: ::gpointer user_data) {
ditto

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:191: const char* path =
static_cast<const char*>(::g_value_get_boxed(gvalue));
Nit: drop :: here and other users in this function.

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:274: // Other Value types -
DICTIONARY, LIST, NULL, REAL, BINARY -
Nit: I prefer to list all the types explicitly rather than using default.  That
way if a type is added or removed, the compiler will catch it and make the
author aware of this code.

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3148: void* object, const char*
manager_path, GHashTable* ghsash);
Nit: typo

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3786: int prefixlen =
static_cast<int>(ipconfig.GetPrefixLength());
int32 and int are the same (see the typedef in base/basictypes.h).  Is this
necessary?

[tony, 2011-08-29 17:01:39]

LGTM

[satorux1, 2011-08-29 19:52:46]

LGTM.

http://codereview.chromium.org/7765009/diff/2002/chrome/browser/chromeos/cros...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/7765009/diff/2002/chrome/browser/chromeos/cros...
chrome/browser/chromeos/cros/network_library.cc:239: // that's what flimflam
expects in its DBus API
Not fault of this change, but use of base::Value for D-Bus calls is worrisome.
D-Bus defines many integral types such as int16, uint32, etc, whereas Value does
not. This would become a problem if flimflam starts using something other than
int32.

[stevenjb, 2011-08-29 20:36:31]

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:161: void
AppendListElement(const GValue *gvalue, ::gpointer user_data) {
On 2011/08/29 17:01:22, tony wrote:
> Nit: GValue* gvalue.  I would probably also drop the :: since we don't seem to
> do that with other uses of gpointer.

Done.

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:169: ::gpointer user_data) {
On 2011/08/29 17:01:22, tony wrote:
> ditto

Done.

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:191: const char* path =
static_cast<const char*>(::g_value_get_boxed(gvalue));
On 2011/08/29 17:01:22, tony wrote:
> Nit: drop :: here and other users in this function.

Done.

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:274: // Other Value types -
DICTIONARY, LIST, NULL, REAL, BINARY -
On 2011/08/29 17:01:22, tony wrote:
> Nit: I prefer to list all the types explicitly rather than using default. 
That
> way if a type is added or removed, the compiler will catch it and make the
> author aware of this code.

Done.

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3148: void* object, const char*
manager_path, GHashTable* ghsash);
On 2011/08/29 17:01:22, tony wrote:
> Nit: typo

Done.

http://codereview.chromium.org/7765009/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3786: int prefixlen =
static_cast<int>(ipconfig.GetPrefixLength());
On 2011/08/29 17:01:22, tony wrote:
> int32 and int are the same (see the typedef in base/basictypes.h).  Is this
> necessary?
It isn't, since int is 32 bits on all modern compilers, just an old habit of
avoiding making assumptions :)