Move Shill property utility functions to a new separate file.

Move Shill property utility functions to a separate file.

This CL moves some functions to handle Shill properties to shill_property_util.*

This cleans up some dependencies (e.g. network_connection_handler.cc doesn't include managed_network_configuration_handler.h anymore) and allows to share some more code.

GetUIData was adapted slightly to make use of GetUIDataFromValue.

CopyIdentifyingProperties deviates from the original implementation in network_connection_handler (e.g. it didn't copy the SSID). This function will be used in ManagedNetConfHandler for the new "AllowAutoconnect" policy.

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

[stevenjb, 2013-08-28 15:33:54]

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/network_...
File chromeos/network/network_connection_handler.cc (right):

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/network_...
chromeos/network/network_connection_handler.cc:490: } else {
no else after return

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.cc (right):

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.cc:163: else
no else

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.cc:216: type == shill::kTypeEthernetEap) {
I think we should add something like
shill_property_util::IsMatchTypeWired(type), etc, and use that throughout. That
should be done in a follow up CL (clearly). I know at least one place where we
are doing something like (type == Cellular || type == Mobile) which would
benefit from such utility methods (i.e. IsMatchTypeMobile(type).

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.h (right):

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.h:36: const base::DictionaryValue&
shill_dictionary);
It's strange that these two functions use different result signatures. Can we
make both either return a scoped_ptr or set 'out'? Also this should be
GetUIDataFromDictionary for clarity.

[pneubeck (no reviews), 2013-09-02 13:39:54]

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/network_...
File chromeos/network/network_connection_handler.cc (right):

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/network_...
chromeos/network/network_connection_handler.cc:490: } else {
On 2013/08/28 15:33:54, stevenjb (chromium) wrote:
> no else after return

Done.

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.cc (right):

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.cc:163: else
On 2013/08/28 15:33:54, stevenjb (chromium) wrote:
> no else

Done.

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.h (right):

https://codereview.chromium.org/23551004/diff/12001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.h:36: const base::DictionaryValue&
shill_dictionary);
On 2013/08/28 15:33:54, stevenjb (chromium) wrote:
> It's strange that these two functions use different result signatures. Can we
> make both either return a scoped_ptr or set 'out'? Also this should be
> GetUIDataFromDictionary for clarity.
> 

Done.

[stevenjb, 2013-09-03 15:54:53]

lgtm w/nit and note.

https://codereview.chromium.org/23551004/diff/36001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.cc (right):

https://codereview.chromium.org/23551004/diff/36001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.cc:211: type == shill::kTypeEthernetEap) {
Let's remember to use MatchType here after the re-factor is in. I don't care
which lands first, let's just make sure to do an audit for TypeEthernet and fix
any code like this.

https://codereview.chromium.org/23551004/diff/36001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.h (right):

https://codereview.chromium.org/23551004/diff/36001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.h:35: scoped_ptr<NetworkUIData>
GetUIDataFromShillDictionary(
nit: We use [GetName]FromProperties above and FromShillDictionary here, we
should probably use one or the other. I don't have a strong preference;
ShillDictionary is more clear, but Properties is shorter. Clarity is probably
better I guess.

[pneubeck (no reviews), 2013-09-04 09:19:02]

https://codereview.chromium.org/23551004/diff/36001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.h (right):

https://codereview.chromium.org/23551004/diff/36001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.h:35: scoped_ptr<NetworkUIData>
GetUIDataFromShillDictionary(
On 2013/09/03 15:54:53, stevenjb (chromium) wrote:
> nit: We use [GetName]FromProperties above and FromShillDictionary here, we
> should probably use one or the other. I don't have a strong preference;
> ShillDictionary is more clear, but Properties is shorter. Clarity is probably
> better I guess.

Right. In this case i think the namespace "shill_property_util" might make it
clear enough.

[commit-bot: I haz the power, 2013-09-04 16:35:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@chromium.org/23551004/53001

[stevenjb, 2013-09-04 18:00:28]

https://codereview.chromium.org/23551004/diff/36001/chromeos/network/shill_pr...
File chromeos/network/shill_property_util.h (right):

https://codereview.chromium.org/23551004/diff/36001/chromeos/network/shill_pr...
chromeos/network/shill_property_util.h:35: scoped_ptr<NetworkUIData>
GetUIDataFromShillDictionary(
On 2013/09/04 09:19:02, pneubeck wrote:
> On 2013/09/03 15:54:53, stevenjb (chromium) wrote:
> > nit: We use [GetName]FromProperties above and FromShillDictionary here, we
> > should probably use one or the other. I don't have a strong preference;
> > ShillDictionary is more clear, but Properties is shorter. Clarity is
probably
> > better I guess.
> 
> Right. In this case i think the namespace "shill_property_util" might make it
> clear enough.
Good point.

[commit-bot: I haz the power, 2013-09-04 20:39:41]

Change committed as 221260