Add DevicePropertiesUpdated to NetworkStateHandlerObserver

Add DevicePropertiesUpdated to NetworkStateHandlerObserver

We will need this for networkingPrivate so that we can notify observers
when Device properties of a Cellular network change.

BUG=none

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

[stevenjb, 2014-07-16 19:58:24]

ping

[pneubeck (no reviews), 2014-07-16 20:29:48]

https://codereview.chromium.org/395523004/diff/1/chromeos/network/network_sta...
File chromeos/network/network_state_handler.h (right):

https://codereview.chromium.org/395523004/diff/1/chromeos/network/network_sta...
chromeos/network/network_state_handler.h:354: // Notifies observers about
changes to |device|.
Sounds like all changes should trigger notifications. I'm not sure that ipconfig
changes work already.
Maybe you want to add a notification to UpdateIPConfigProperties(), or adapt
this comment to exclude IPconfig (because the notifcation is already sent
through the default network).

[pneubeck (no reviews), 2014-07-16 20:42:34]

lgtm with the previous comment for consideration

[stevenjb, 2014-07-16 20:51:34]

https://codereview.chromium.org/395523004/diff/1/chromeos/network/network_sta...
File chromeos/network/network_state_handler.h (right):

https://codereview.chromium.org/395523004/diff/1/chromeos/network/network_sta...
chromeos/network/network_state_handler.h:354: // Notifies observers about
changes to |device|.
On 2014/07/16 20:29:48, pneubeck wrote:
> Sounds like all changes should trigger notifications. I'm not sure that
ipconfig
> changes work already.
> Maybe you want to add a notification to UpdateIPConfigProperties(), or adapt
> this comment to exclude IPconfig (because the notifcation is already sent
> through the default network).
It's likely going to be redundant since the network should receive an IPConfig
update also, but I would rather be thorough in case somewhere down the line we
add some device specific UI and rely on this, so I added a call for devices to
UpdateIPConfigProperties and explicitly commented for each if these that they
include IPCongig[s] updates. Good catch.

[stevenjb, 2014-07-16 21:10:52]

Added an update for device IPConfigs and a test, PTAL

[pneubeck (no reviews), 2014-07-16 21:43:35]

https://codereview.chromium.org/395523004/diff/20001/chromeos/network/network...
File chromeos/network/network_state_handler_unittest.cc (right):

https://codereview.chromium.org/395523004/diff/20001/chromeos/network/network...
chromeos/network/network_state_handler_unittest.cc:555: EXPECT_EQ(1,
test_observer_->ConnectionStateChangesForService(eth1));
I'm confused, maybe because it's to late. Why did this result change?

[pneubeck (no reviews), 2014-07-16 21:56:29]

apart from my minor confusion on the test,
still lgtm.

[stevenjb, 2014-07-16 22:14:15]

https://codereview.chromium.org/395523004/diff/20001/chromeos/network/network...
File chromeos/network/network_state_handler_unittest.cc (right):

https://codereview.chromium.org/395523004/diff/20001/chromeos/network/network...
chromeos/network/network_state_handler_unittest.cc:555: EXPECT_EQ(1,
test_observer_->ConnectionStateChangesForService(eth1));
On 2014/07/16 21:43:35, pneubeck wrote:
> I'm confused, maybe because it's to late. Why did this result change?

reset_change_counts() which gets called in SetUp() now clears
connection_state_changes_. There is a single connection state change in SetUp
which we were implicitly accounting for here before (my bad).

[armansito, 2014-07-16 22:17:38]

lgtm

[stevenjb, 2014-07-17 16:53:49]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2014-07-17 16:54:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/stevenjb@chromium.org/395523004/20001

[commit-bot: I haz the power, 2014-07-17 20:46:20]

Change committed as 283863