Only show prefer network checkbox for networks stored on user profile.

Only show prefer network checkbox for networks stored on user profile.
When the user logs in, auto connect to preferred network if appropriate.
BUG=chromuim-os:3559
TEST=manual
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=95532

[stevenjb, 2011-08-03 21:37:52]

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3752: if (active_wifi_ == NULL
|| active_wifi_->preferred())
This assumes that flimflam would always auto-connect to a preferred+auto-connect
network when not already connected to a wifi network. This is probably OK, but
we might want to explain that.

Alternately, we could be more explicit here and not rely on flimflam behavior:

if (!wifi_enabled() || (active_wifi_ && active_wifi_->preferred())
  return;

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3754: if
(wifi_networks_.empty())
nit: Extra check here isn't really necessary.

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3759: if (wifi->connected())  //
Skip connected networks.
|| connecting() (I don't think we can currently be connecting to one wifi
network while another is active, but that may change at some point).

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:4991: if (active_profile_path_
!= kSharedProfilePath)
We should check on == kUserProfilePath; there has been some discussion of adding
additional profiles, e.g. a machine policy profile.
We probably also only want to do this check when the active profile changes,
instead of every time we request a manager update.

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
File chrome/browser/chromeos/cros/network_library.h (right):

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.h:1297: // This will connect to an
preferred network if the currently connected
nit: a preferred

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.h:1300: virtual void
SwitchToPreferredNetworkIfAppropriate() = 0;
personal nit: It seems to me that "IfAppropriate" is implied and may not be
necessary?

[Charlie Lee, 2011-08-03 22:22:59]

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3752: if (active_wifi_ == NULL
|| active_wifi_->preferred())
Good advice. Thanks.

On 2011/08/03 21:37:52, Steven Bennetts wrote:
> This assumes that flimflam would always auto-connect to a
preferred+auto-connect
> network when not already connected to a wifi network. This is probably OK, but
> we might want to explain that.
> 
> Alternately, we could be more explicit here and not rely on flimflam behavior:
> 
> if (!wifi_enabled() || (active_wifi_ && active_wifi_->preferred())
>   return;

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3754: if
(wifi_networks_.empty())
On 2011/08/03 21:37:52, Steven Bennetts wrote:
> nit: Extra check here isn't really necessary.

Done.

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.cc:3759: if (wifi->connected())  //
Skip connected networks.
On 2011/08/03 21:37:52, Steven Bennetts wrote:
> || connecting() (I don't think we can currently be connecting to one wifi
> network while another is active, but that may change at some point).

Done.

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
File chrome/browser/chromeos/cros/network_library.h (right):

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.h:1297: // This will connect to an
preferred network if the currently connected
On 2011/08/03 21:37:52, Steven Bennetts wrote:
> nit: a preferred

Done.

http://codereview.chromium.org/7566015/diff/1/chrome/browser/chromeos/cros/ne...
chrome/browser/chromeos/cros/network_library.h:1300: virtual void
SwitchToPreferredNetworkIfAppropriate() = 0;
On 2011/08/03 21:37:52, Steven Bennetts wrote:
> personal nit: It seems to me that "IfAppropriate" is implied and may not be
> necessary?

Done.

[stevenjb, 2011-08-03 22:58:26]

http://codereview.chromium.org/7566015/diff/4001/chrome/browser/chromeos/cros...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/7566015/diff/4001/chrome/browser/chromeos/cros...
chrome/browser/chromeos/cros/network_library.cc:4990:
SwitchToPreferredNetwork();
We should probably only call this when active_profile_path_ changes, and we
should check on == user instead of != shared, in case we add additional profile
types later.

[Charlie Lee, 2011-08-04 20:43:02]

http://codereview.chromium.org/7566015/diff/4001/chrome/browser/chromeos/cros...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/7566015/diff/4001/chrome/browser/chromeos/cros...
chrome/browser/chromeos/cros/network_library.cc:4990:
SwitchToPreferredNetwork();
On 2011/08/03 22:58:26, Steven Bennetts wrote:
> We should probably only call this when active_profile_path_ changes, and we
> should check on == user instead of != shared, in case we add additional
profile
> types later.

What's the format of the user profile? It seems like, right now in other places,
we resort to profile type is user if profile string is not shared. So that's
what i'm doing here also.

http://codereview.chromium.org/7566015/diff/4001/chrome/browser/chromeos/cros...
chrome/browser/chromeos/cros/network_library.cc:5285: NetworkProfileType
profile_type;
See this code. I assume user profile is something like /profile/user/somestring
and it was easier to just check to see if it's not the shared profile.

[stevenjb, 2011-08-04 21:01:14]

LGTM

http://codereview.chromium.org/7566015/diff/4001/chrome/browser/chromeos/cros...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/7566015/diff/4001/chrome/browser/chromeos/cros...
chrome/browser/chromeos/cros/network_library.cc:4990:
SwitchToPreferredNetwork();
On 2011/08/04 20:43:02, Charlie Lee wrote:
> On 2011/08/03 22:58:26, Steven Bennetts wrote:
> > We should probably only call this when active_profile_path_ changes, and we
> > should check on == user instead of != shared, in case we add additional
> profile
> > types later.
> 
> What's the format of the user profile? It seems like, right now in other
places,
> we resort to profile type is user if profile string is not shared. So that's
> what i'm doing here also.

Ah, I see, I didn't add a kUserProfilePath[]. We should probably replace
active_profile_path_ with active_profile_type_ and have one piece of code that
parses the path and sets the type, but this is fine for now. If we add any more
profile types we can do the refactoring then.