Add additional cellular-related properties to Network|DeviceState

Add additional cellular-related properties to Network|DeviceState

These properties (specifically related to cellular activation)
are used by MobileActivator and needed to transition it to use
the Network*Handlers.

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

[stevenjb, 2013-06-21 01:15:11]

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/device_st...
File chromeos/network/device_state.h (right):

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/device_st...
chromeos/network/device_state.h:24: const std::string& mac_address() const {
return mac_address_; }
nit: We should add a 'Cellular specific accessors' comment here to match the
comment below.

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/network_s...
File chromeos/network/network_state.h (right):

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/network_s...
chromeos/network/network_state.h:73: const std::string& post_data() const {
return post_data_; }
Ugh, do we really need instant access to all of these, or can we use
NetworkConfigurationHandler::GetProperties() instead? I know that I have been
guilty of letting this class bloat, and I'm OK with us doing that during the
transition (as long as we go back and clean these up a bit later), but if making
an asynchronous GetProperties() call doesn't add too much complexity, that would
be preferable.

[gauravsh, 2013-06-21 01:51:09]

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/device_st...
File chromeos/network/device_state.h (right):

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/device_st...
chromeos/network/device_state.h:24: const std::string& mac_address() const {
return mac_address_; }
On 2013/06/21 01:15:11, stevenjb (chromium) wrote:
> nit: We should add a 'Cellular specific accessors' comment here to match the
> comment below.

Done. (though - seems redundant, no?)

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/network_s...
File chromeos/network/network_state.h (right):

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/network_s...
chromeos/network/network_state.h:73: const std::string& post_data() const {
return post_data_; }
On 2013/06/21 01:15:11, stevenjb (chromium) wrote:
> Ugh, do we really need instant access to all of these, or can we use
> NetworkConfigurationHandler::GetProperties() instead? I know that I have been
> guilty of letting this class bloat, and I'm OK with us doing that during the
> transition (as long as we go back and clean these up a bit later), but if
making
> an asynchronous GetProperties() call doesn't add too much complexity, that
would
> be preferable.

In this particular case, yes, it is preferable to allow direct access to these
properties while we transition.

Given MobileActivator's lack of test coverage, I'd like to keep the changes to
it as minimal as possible. Once we move over to using NetworkingPrivate API for
the activation code, I suspect async GetProperties would fit very well within
the JS way of doing thing.

I filed a TODO bug.

[stevenjb, 2013-06-21 20:28:53]

lgtm

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/network_s...
File chromeos/network/network_state.h (right):

https://codereview.chromium.org/17091006/diff/2001/chromeos/network/network_s...
chromeos/network/network_state.h:73: const std::string& post_data() const {
return post_data_; }
On 2013/06/21 01:51:09, gauravsh wrote:
> On 2013/06/21 01:15:11, stevenjb (chromium) wrote:
> > Ugh, do we really need instant access to all of these, or can we use
> > NetworkConfigurationHandler::GetProperties() instead? I know that I have
been
> > guilty of letting this class bloat, and I'm OK with us doing that during the
> > transition (as long as we go back and clean these up a bit later), but if
> making
> > an asynchronous GetProperties() call doesn't add too much complexity, that
> would
> > be preferable.
> 
> In this particular case, yes, it is preferable to allow direct access to these
> properties while we transition.
> 
> Given MobileActivator's lack of test coverage, I'd like to keep the changes to
> it as minimal as possible. Once we move over to using NetworkingPrivate API
for
> the activation code, I suspect async GetProperties would fit very well within
> the JS way of doing thing.
> 
> I filed a TODO bug.
OK, fair enough.

[commit-bot: I haz the power, 2013-06-21 20:35:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gauravsh@chromium.org/17091006/13001

[commit-bot: I haz the power, 2013-06-22 02:34:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gauravsh@chromium.org/17091006/13001

[commit-bot: I haz the power, 2013-06-22 03:03:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gauravsh@chromium.org/17091006/13001

[commit-bot: I haz the power, 2013-06-22 03:32:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gauravsh@chromium.org/17091006/13001

[commit-bot: I haz the power, 2013-06-22 06:55:56]

Change committed as 208009