Fix reconnect in case of empty global network policy.

Fix reconnect in case of empty global network policy.

While there, remove some error messages because of incomplete test setup or incorrect fake behavior.

BUG=425049
Committed: https://crrev.com/6071e0f89dc2b8042e74ce26386c125d5b2e6f69
Cr-Commit-Position: refs/heads/master@{#300284}

[pneubeck (no reviews), 2014-10-20 09:43:37]

Patchset #1 (id:1) has been deleted

[pneubeck (no reviews), 2014-10-20 09:44:55]

pneubeck@chromium.org changed reviewers:
+ stevenjb@chromium.org

[pneubeck (no reviews), 2014-10-20 09:44:55]

pneubeck@chromium.org changed required reviewers:
+ stevenjb@chromium.org

[pneubeck (no reviews), 2014-10-20 09:44:56]

ptal

[pneubeck (no reviews), 2014-10-20 09:46:12]

Patchset #2 (id:40001) has been deleted

[stevenjb, 2014-10-20 16:15:35]

lgtm

https://codereview.chromium.org/648623004/diff/20001/chromeos/dbus/fake_shill...
File chromeos/dbus/fake_shill_service_client.cc (right):

https://codereview.chromium.org/648623004/diff/20001/chromeos/dbus/fake_shill...
chromeos/dbus/fake_shill_service_client.cc:453:
profile_test->AddService(profile_path, service_path);
If profile_path is set to empty, should we remove the service?

[pneubeck (no reviews), 2014-10-20 16:21:24]

https://codereview.chromium.org/648623004/diff/20001/chromeos/dbus/fake_shill...
File chromeos/dbus/fake_shill_service_client.cc (right):

https://codereview.chromium.org/648623004/diff/20001/chromeos/dbus/fake_shill...
chromeos/dbus/fake_shill_service_client.cc:453:
profile_test->AddService(profile_path, service_path);
On 2014/10/20 16:15:35, stevenjb wrote:
> If profile_path is set to empty, should we remove the service?

Not sure. Removing the profile path could also mean that the network is still
visible but in the 'ephemeral' profile.

[stevenjb, 2014-10-20 16:54:54]

On 2014/10/20 16:21:24, pneubeck wrote:
>
https://codereview.chromium.org/648623004/diff/20001/chromeos/dbus/fake_shill...
> File chromeos/dbus/fake_shill_service_client.cc (right):
> 
>
https://codereview.chromium.org/648623004/diff/20001/chromeos/dbus/fake_shill...
> chromeos/dbus/fake_shill_service_client.cc:453:
> profile_test->AddService(profile_path, service_path);
> On 2014/10/20 16:15:35, stevenjb wrote:
> > If profile_path is set to empty, should we remove the service?
> 
> Not sure. Removing the profile path could also mean that the network is still
> visible but in the 'ephemeral' profile.

I have no idea what 'emphemeral' profile refers to. Given that this is just a
convenience hack and doesn't represent normal usage (i.e. we normally explicitly
add/remove services from profiles, we don't change the Profile property of the
service) I don't feel strongly either way.

[pneubeck (no reviews), 2014-10-20 16:58:06]

On 2014/10/20 16:54:54, stevenjb wrote:
> On 2014/10/20 16:21:24, pneubeck wrote:
> >
>
https://codereview.chromium.org/648623004/diff/20001/chromeos/dbus/fake_shill...
> > File chromeos/dbus/fake_shill_service_client.cc (right):
> > 
> >
>
https://codereview.chromium.org/648623004/diff/20001/chromeos/dbus/fake_shill...
> > chromeos/dbus/fake_shill_service_client.cc:453:
> > profile_test->AddService(profile_path, service_path);
> > On 2014/10/20 16:15:35, stevenjb wrote:
> > > If profile_path is set to empty, should we remove the service?
> > 
> > Not sure. Removing the profile path could also mean that the network is
still
> > visible but in the 'ephemeral' profile.
> 
> I have no idea what 'emphemeral' profile refers to. Given that this is just a
> convenience hack and doesn't represent normal usage (i.e. we normally
explicitly
> add/remove services from profiles, we don't change the Profile property of the
> service) I don't feel strongly either way.

Shill assigns unconfigured Wifi networks that are visible to an ephemeral
profile:
https://cs.corp.google.com/#search/&type=cs&q=ephemeral%20package:%5Echromeos...

I don't fully understand the implications of doing any additional removal of the
service here and would like to not touch it beyond what is necessary.

[pneubeck (no reviews), 2014-10-20 16:58:09]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-10-20 16:58:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/648623004/20001

[stevenjb, 2014-10-20 16:59:47]

The Fake implementation has no concept of this, and probably doesn't need
to. I think removing the service from any profile would be the Right Thing
To Do, but until / unless this becomes a problem I am fine leaving this
as-is.


On Mon, Oct 20, 2014 at 9:58 AM, <pneubeck@chromium.org> wrote:

> On 2014/10/20 16:54:54, stevenjb wrote:
>
>> On 2014/10/20 16:21:24, pneubeck wrote:
>> >
>>
>
> https://codereview.chromium.org/648623004/diff/20001/
> chromeos/dbus/fake_shill_service_client.cc
>
>> > File chromeos/dbus/fake_shill_service_client.cc (right):
>> >
>> >
>>
>
> https://codereview.chromium.org/648623004/diff/20001/
> chromeos/dbus/fake_shill_service_client.cc#newcode453
>
>> > chromeos/dbus/fake_shill_service_client.cc:453:
>> > profile_test->AddService(profile_path, service_path);
>> > On 2014/10/20 16:15:35, stevenjb wrote:
>> > > If profile_path is set to empty, should we remove the service?
>> >
>> > Not sure. Removing the profile path could also mean that the network is
>>
> still
>
>> > visible but in the 'ephemeral' profile.
>>
>
>  I have no idea what 'emphemeral' profile refers to. Given that this is
>> just a
>> convenience hack and doesn't represent normal usage (i.e. we normally
>>
> explicitly
>
>> add/remove services from profiles, we don't change the Profile property
>> of the
>> service) I don't feel strongly either way.
>>
>
> Shill assigns unconfigured Wifi networks that are visible to an ephemeral
> profile:
> https://cs.corp.google.com/#search/&type=cs&q=ephemeral%
> 20package:%5Echromeos_public$%20file:(%5Esrc/platform2/shill/)
>
> I don't fully understand the implications of doing any additional removal
> of the
> service here and would like to not touch it beyond what is necessary.
>
> https://codereview.chromium.org/648623004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-10-20 17:09:44]

Committed patchset #1 (id:20001)

[commit-bot: I haz the power, 2014-10-20 17:10:22]

Patchset 1 (id:??) landed as
https://crrev.com/6071e0f89dc2b8042e74ce26386c125d5b2e6f69
Cr-Commit-Position: refs/heads/master@{#300284}