Enhance portal check code to return connectivity_state

Enhance the portal check code to return an enum expressing the connectivity state rather than a boolean indicating being inside a restricted pool.

This is based on the observation that in a non-trivial percentage of times when in the restricted pool, the connection is so restricted as to not even have connectivity to the online portal or the DNS server.

Change-Id: I5089f7c2deeaaa0949a68946b605f0bf01c94fdc

BUG=chromium-os:9228
TEST=

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=4abfab7

[Jason Glasgow, 2010-11-19 00:13:56]

Zel is on this so he knows where we are going
Eric you can do the initial review
Sam you can drive by and provide comments.

This is the first step -- I want to agree on interfaces and names.  I'm then
going to add the implementation in portal_check.c what will do a better job of
figuring out the connectivity state.  I will upload those changes to this CL
soon.

[Jason Glasgow, 2010-11-19 02:02:21]

PTAL

[Jason Glasgow, 2010-11-19 02:20:43]

Adding Vince so Cashew can track...

[Sam Leffler, 2010-11-19 16:06:52]

There look to be a bunch of unrelated (style) changes in the portal code.  I
prefer splitting them to a separate CL but your call.

Otherwise LGTM modulo request for explanation on the meaning of the connectivity
states.

http://codereview.chromium.org/5167004/diff/11001/include/service.h
File include/service.h (right):

http://codereview.chromium.org/5167004/diff/11001/include/service.h#newcode81
include/service.h:81: enum connman_service_connectivity_state {
please add an explanation of the different states here or in the code (I prefer
at the decl but don't see it for other enums in this file)

http://codereview.chromium.org/5167004/diff/11001/plugins/portal_check.c
File plugins/portal_check.c (right):

http://codereview.chromium.org/5167004/diff/11001/plugins/portal_check.c#newc...
plugins/portal_check.c:90: * Returns true iff we received an HTTP 204.
stale comment

http://codereview.chromium.org/5167004/diff/11001/plugins/portal_check.c#newc...
plugins/portal_check.c:100: /* TODO(jglasgow): test resolving the olp instead */
don't know what "olp" is; please expand

[Eric Shienbrood, 2010-11-19 17:52:56]

http://codereview.chromium.org/5167004/diff/11001/plugins/portal_check.c
File plugins/portal_check.c (right):

http://codereview.chromium.org/5167004/diff/11001/plugins/portal_check.c#newc...
plugins/portal_check.c:299: /* The timer will fire once per second until the
request
Need newline after /*

http://codereview.chromium.org/5167004/diff/11001/src/service.c
File src/service.c (right):

http://codereview.chromium.org/5167004/diff/11001/src/service.c#newcode68
src/service.c:68: enum connman_service_connectivity_state connectivity_state;
Can you move this up to be with the other enums?

http://codereview.chromium.org/5167004/diff/11001/src/service.c#newcode1640
src/service.c:1640: "ConnectivityState",
Can you update service-api.txt to document this property, and remove the
description of RestrictedPool?

[Jason Glasgow, 2010-11-19 19:48:19]

http://codereview.chromium.org/5167004/diff/11001/include/service.h
File include/service.h (right):

http://codereview.chromium.org/5167004/diff/11001/include/service.h#newcode81
include/service.h:81: enum connman_service_connectivity_state {
On 2010/11/19 16:06:52, Sam Leffler wrote:
> please add an explanation of the different states here or in the code (I
prefer
> at the decl but don't see it for other enums in this file)

Done.

http://codereview.chromium.org/5167004/diff/11001/plugins/portal_check.c
File plugins/portal_check.c (right):

http://codereview.chromium.org/5167004/diff/11001/plugins/portal_check.c#newc...
plugins/portal_check.c:90: * Returns true iff we received an HTTP 204.
On 2010/11/19 16:06:52, Sam Leffler wrote:
> stale comment

Done.

http://codereview.chromium.org/5167004/diff/11001/plugins/portal_check.c#newc...
plugins/portal_check.c:299: /* The timer will fire once per second until the
request
On 2010/11/19 17:52:56, Eric Shienbrood wrote:
> Need newline after /*

Done.

http://codereview.chromium.org/5167004/diff/11001/src/service.c
File src/service.c (right):

http://codereview.chromium.org/5167004/diff/11001/src/service.c#newcode68
src/service.c:68: enum connman_service_connectivity_state connectivity_state;
On 2010/11/19 17:52:56, Eric Shienbrood wrote:
> Can you move this up to be with the other enums?

Done.

http://codereview.chromium.org/5167004/diff/11001/src/service.c#newcode1640
src/service.c:1640: "ConnectivityState",
Thank you.
Done.

[Eric Shienbrood, 2010-11-19 20:00:31]

LGTM