debugd: Add GetNetworkInterfaces functionality.

debugd: Add GetNetworkInterfaces functionality.

BUG=139442
TEST=CQ

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

[hshi1, 2012-07-30 21:52:18]

Please take a look. Thanks!

[hshi1, 2012-07-31 17:41:50]

Add ellyjones@ as she implemented the GetInterfaces in debugd.

[satorux1, 2012-07-31 20:36:52]

sorry for the belated response.

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
File chromeos/dbus/debug_daemon_client.h (right):

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
chromeos/dbus/debug_daemon_client.h:62: typedef base::Callback<void(bool
succeeded, const std::string& status)>
how does |status| look like? Could you add some comment?

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
chromeos/dbus/debug_daemon_client.h:65: virtual void GetInterfaces(const
GetInterfacesCallback& callback) = 0;
the function comment is missing here and elsewhere too! We should at least have
one here.


I guess  GetNetworkInterfaces would be a better name.

[hshi1, 2012-07-31 21:32:02]

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
File chromeos/dbus/debug_daemon_client.h (right):

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
chromeos/dbus/debug_daemon_client.h:62: typedef base::Callback<void(bool
succeeded, const std::string& status)>
The |status| is a json containing a fairly complicated dictionary of
dictionaries. The detailed structure is documented in chromeos
platform/debugd/src/heplers/netif.cc). Would this comment suffice, or shall I
cross-reference the cros source file?

On 2012/07/31 20:36:52, satorux1 wrote:
> how does |status| look like? Could you add some comment?

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
chromeos/dbus/debug_daemon_client.h:65: virtual void GetInterfaces(const
GetInterfacesCallback& callback) = 0;
Done.

On 2012/07/31 20:36:52, satorux1 wrote:
> the function comment is missing here and elsewhere too! We should at least
have
> one here.
> 
> 
> I guess  GetNetworkInterfaces would be a better name.

[satorux1, 2012-07-31 21:35:45]

LGTM with a request:

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
File chromeos/dbus/debug_daemon_client.h (right):

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
chromeos/dbus/debug_daemon_client.h:62: typedef base::Callback<void(bool
succeeded, const std::string& status)>
On 2012/07/31 21:32:02, hshi1 wrote:
> The |status| is a json containing a fairly complicated dictionary of
> dictionaries. The detailed structure is documented in chromeos
> platform/debugd/src/heplers/netif.cc). Would this comment suffice, or shall I
> cross-reference the cros source file?
> 
> On 2012/07/31 20:36:52, satorux1 wrote:
> > how does |status| look like? Could you add some comment?
> 

Would be nice to add a pointer to the documentation. http://goo.gl/ may be
useful to shorten the link

[hshi1, 2012-07-31 21:44:45]

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
File chromeos/dbus/debug_daemon_client.h (right):

http://codereview.chromium.org/10831059/diff/5001/chromeos/dbus/debug_daemon_...
chromeos/dbus/debug_daemon_client.h:62: typedef base::Callback<void(bool
succeeded, const std::string& status)>
On 2012/07/31 21:35:45, satorux1 wrote:
> On 2012/07/31 21:32:02, hshi1 wrote:
> > The |status| is a json containing a fairly complicated dictionary of
> > dictionaries. The detailed structure is documented in chromeos
> > platform/debugd/src/heplers/netif.cc). Would this comment suffice, or shall
I
> > cross-reference the cros source file?
> > 
> > On 2012/07/31 20:36:52, satorux1 wrote:
> > > how does |status| look like? Could you add some comment?
> > 
> 
> Would be nice to add a pointer to the documentation. http://goo.gl/ may be
> useful to shorten the link

Done.

[commit-bot: I haz the power, 2012-07-31 22:00:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hshi@chromium.org/10831059/1009