Add NetworkStateHandler::RequestUpdateForNetwork

Add NetworkStateHandler::RequestUpdateForNetwork

This also changes the code to signal observers after update requests, 
even if nothing changed so that NetworkPropertiesUpdated() will get
signaled after a request.

BUG=240728
R=pneubeck@chromium.org

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

[stevenjb, 2013-05-16 21:38:07]

This should help ensure that NetworkState stays in sync both with any Configure
requests and with NetworkLibrary as we transition.

Ready for review.

[pneubeck (no reviews), 2013-05-22 08:42:40]

lgtm. From a quick check, it seems that it covers all calls in NetworkLibrary,
but I'm not 100% sure.

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_c...
File chromeos/network/network_configuration_handler.cc (right):

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_c...
chromeos/network/network_configuration_handler.cc:202: }
here configuration is also modified by profile/guid and not by service; see the
calls from Managed*Handler.

We would have to call the StateHandler in the callback, where we obtain the
service path.


Alternatively, I can fix that when I __eventually__ merge it with the
Managed*Handler.
At least drop a TODO here.

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_s...
File chromeos/network/network_state_handler.cc (right):

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_s...
chromeos/network/network_state_handler.cc:437: if (network_property_updated ||
managed->update_requested()) {
a comment about your intent (guaranteed notification after request) either here
or at the Request*() methods in the header might be helpful for future readers.

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_s...
File chromeos/network/network_state_handler.h (right):

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_s...
chromeos/network/network_state_handler.h:166: // loading an ONC configuration
file that may have updated one more more
more -> or

[stevenjb, 2013-05-22 16:08:23]

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_c...
File chromeos/network/network_configuration_handler.cc (right):

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_c...
chromeos/network/network_configuration_handler.cc:202: }
On 2013/05/22 08:42:40, pneubeck wrote:
> here configuration is also modified by profile/guid and not by service; see
the
> calls from Managed*Handler.
> 
> We would have to call the StateHandler in the callback, where we obtain the
> service path.
> 
> 
> Alternatively, I can fix that when I __eventually__ merge it with the
> Managed*Handler.
> At least drop a TODO here.
I'm not entirely sure that I follow.

This creates a new configuration. If the new configuration becomes a visible
service, i.e. a member of Manager.Services, the list will be updated and since
it is a new service the properties will be requested, i.e. we do not need to
explicitly request an update.

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_s...
File chromeos/network/network_state_handler.cc (right):

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_s...
chromeos/network/network_state_handler.cc:437: if (network_property_updated ||
managed->update_requested()) {
On 2013/05/22 08:42:40, pneubeck wrote:
> a comment about your intent (guaranteed notification after request) either
here
> or at the Request*() methods in the header might be helpful for future
readers.

Good thought, comment added.

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_s...
File chromeos/network/network_state_handler.h (right):

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_s...
chromeos/network/network_state_handler.h:166: // loading an ONC configuration
file that may have updated one more more
On 2013/05/22 08:42:40, pneubeck wrote:
> more -> or

Done.

[pneubeck (no reviews), 2013-05-22 18:02:09]

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_c...
File chromeos/network/network_configuration_handler.cc (right):

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_c...
chromeos/network/network_configuration_handler.cc:202: }
On 2013/05/22 16:08:23, stevenjb (chromium) wrote:
> On 2013/05/22 08:42:40, pneubeck wrote:
> > here configuration is also modified by profile/guid and not by service; see
> the
> > calls from Managed*Handler.
> > 
> > We would have to call the StateHandler in the callback, where we obtain the
> > service path.
> > 
> > 
> > Alternatively, I can fix that when I __eventually__ merge it with the
> > Managed*Handler.
> > At least drop a TODO here.
> I'm not entirely sure that I follow.
> 
> This creates a new configuration. If the new configuration becomes a visible
> service, i.e. a member of Manager.Services, the list will be updated and since
> it is a new service the properties will be requested, i.e. we do not need to
> explicitly request an update.
> 

The name of the method is not identical to its usage:
It is currently also called by the Managed*Handler to overwrite an existing
configuration in a profile.

[stevenjb, 2013-05-22 18:45:35]

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_c...
File chromeos/network/network_configuration_handler.cc (right):

https://codereview.chromium.org/15233002/diff/3001/chromeos/network/network_c...
chromeos/network/network_configuration_handler.cc:202: }
On 2013/05/22 18:02:09, pneubeck wrote:
> On 2013/05/22 16:08:23, stevenjb (chromium) wrote:
> > On 2013/05/22 08:42:40, pneubeck wrote:
> > > here configuration is also modified by profile/guid and not by service;
see
> > the
> > > calls from Managed*Handler.
> > > 
> > > We would have to call the StateHandler in the callback, where we obtain
the
> > > service path.
> > > 
> > > 
> > > Alternatively, I can fix that when I __eventually__ merge it with the
> > > Managed*Handler.
> > > At least drop a TODO here.
> > I'm not entirely sure that I follow.
> > 
> > This creates a new configuration. If the new configuration becomes a visible
> > service, i.e. a member of Manager.Services, the list will be updated and
since
> > it is a new service the properties will be requested, i.e. we do not need to
> > explicitly request an update.
> > 
> 
> The name of the method is not identical to its usage:
> It is currently also called by the Managed*Handler to overwrite an existing
> configuration in a profile.

Ah. Right, we talked about the need to fix this, possibly after merging the two
classes? It looks like it is easy enough to trigger the call from the callback.
Done.

[stevenjb, 2013-05-22 18:45:54]

PTAL

[pneubeck (no reviews), 2013-05-22 19:36:43]

lgtm

[stevenjb, 2013-05-22 21:27:40]

Committed patchset #5 manually as r201614 (presubmit successful).