Use Name not SSID in CopyIdentifyingProperties

Use Name not SSID in CopyIdentifyingProperties

This also makes a minor change to WifiConfigView to update the 'Connect'
button enabled state after receiving wifi properties by calling
UpdateDialogButtons().

BUG=289642
R=pneubeck@chromium.org

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

[pneubeck (no reviews), 2013-09-13 07:15:10]

https://codereview.chromium.org/23872021/diff/1/chromeos/network/shill_proper...
File chromeos/network/shill_property_util.cc (right):

https://codereview.chromium.org/23872021/diff/1/chromeos/network/shill_proper...
chromeos/network/shill_property_util.cc:196: service_properties,
flimflam::kNameProperty, dest)) {
I think that this change makes it even worse:
From all previous discussions with Paul, a Wifi service is identified by
Security+SSID+Mode.
This is also what is documented here:
https://cs.corp.google.com/#chromeos_public/src/platform/shill/doc/manager-ap...
and implemented here:
https://cs.corp.google.com/#chromeos_public/src/platform/shill/wifi_provider....

The properties that we set are ok. But, it seems that we ran again into the
issue that Shill doesn't set kSSIDProperty reliably.
This problem should as well be fixed if we change everything to HexSSID.

A temporary fix could be to copy service_properties.kNameProperty to
dest.kSSIDProperty, if service_properties.kSSIDProperty is empty/unset.

[stevenjb, 2013-09-13 23:54:24]

On 2013/09/13 07:15:10, pneubeck wrote:
>
https://codereview.chromium.org/23872021/diff/1/chromeos/network/shill_proper...
> File chromeos/network/shill_property_util.cc (right):
> 
>
https://codereview.chromium.org/23872021/diff/1/chromeos/network/shill_proper...
> chromeos/network/shill_property_util.cc:196: service_properties,
> flimflam::kNameProperty, dest)) {
> I think that this change makes it even worse:
> From all previous discussions with Paul, a Wifi service is identified by
> Security+SSID+Mode.
> This is also what is documented here:
>
https://cs.corp.google.com/#chromeos_public/src/platform/shill/doc/manager-ap...
> and implemented here:
>
https://cs.corp.google.com/#chromeos_public/src/platform/shill/wifi_provider....
> 
> The properties that we set are ok. But, it seems that we ran again into the
> issue that Shill doesn't set kSSIDProperty reliably.
> This problem should as well be fixed if we change everything to HexSSID.
> 
> A temporary fix could be to copy service_properties.kNameProperty to
> dest.kSSIDProperty, if service_properties.kSSIDProperty is empty/unset.

OK, I am kind of mystified why I added this code in the first place. This only
gets called when configuring an existing network. SetProperties doesn't need to
provide any identifying properties, they are already set and immutable. 

I tested connecting to Google-A manually (from an un-enrolled account) and it
works fine. I can only surmise that somewhere during the re-factoring I got some
configuration code confused with the connection code.

PTAL

[pneubeck (no reviews), 2013-09-14 16:04:24]

lgtm, except the removal of the CopyIdentifyingProperties.

https://codereview.chromium.org/23872021/diff/6001/chromeos/network/network_c...
File chromeos/network/network_connection_handler.cc (right):

https://codereview.chromium.org/23872021/diff/6001/chromeos/network/network_c...
chromeos/network/network_connection_handler.cc:457:
network_configuration_handler_->SetProperties(
ha! yes that makes more sense.

https://codereview.chromium.org/23872021/diff/6001/chromeos/network/shill_pro...
File chromeos/network/shill_property_util.h (left):

https://codereview.chromium.org/23872021/diff/6001/chromeos/network/shill_pro...
chromeos/network/shill_property_util.h:49: bool CopyIdentifyingProperties(const
base::DictionaryValue& service_properties,
please keep this function as it was. I made it visible in this file, because I
need it for the autoconnect policy.

[pneubeck (no reviews), 2013-09-14 16:07:57]

please also adapt the commit message.

[stevenjb, 2013-09-16 16:29:49]

https://codereview.chromium.org/23872021/diff/6001/chromeos/network/shill_pro...
File chromeos/network/shill_property_util.h (left):

https://codereview.chromium.org/23872021/diff/6001/chromeos/network/shill_pro...
chromeos/network/shill_property_util.h:49: bool CopyIdentifyingProperties(const
base::DictionaryValue& service_properties,
On 2013/09/14 16:04:25, pneubeck wrote:
> please keep this function as it was. I made it visible in this file, because I
> need it for the autoconnect policy.

OK, but let's make sure we use it soon or remove it; we can always restore it if
we need it later.

[stevenjb, 2013-09-16 22:01:46]

Committed patchset #4 manually as r223435 (presubmit successful).