Added detection timeouts and usage of Retry-After HTTP header.

Added detection timeouts and usage of Retry-After HTTP header.

* added detection timeouts and usage of Retry-After HTTP header;
* added network portal detector unit tests for cases when detection times out.
* refactored testing utils and captive portal service unit tests;

BUG=134206
TEST=unit_tests:NetworkPortalDetector*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=169246

[ygorshenin1, 2012-11-19 17:41:15]

PTAL

mmenke@: chrome/browser/captive_portal/*
nkostylev@: chrome/browser/chromeos/*

[mmenke, 2012-11-19 19:03:44]

chrome/browser/captive_portal/ LGTM.  Like the refactoring.

On 2012/11/19 17:41:15, ygorshenin1 wrote:
> PTAL
> 
> mmenke@: chrome/browser/captive_portal/*
> nkostylev@: chrome/browser/chromeos/*

[Nikita (slow), 2012-11-20 12:58:44]

lgtm, please improve logging so that feedback logs would contain useful info for
captive portal.

http://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/net...
File chrome/browser/chromeos/net/network_portal_detector.cc (right):

http://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/net...
chrome/browser/chromeos/net/network_portal_detector.cc:24: const int
kMaxRequestAttempts = 3;
nit: Please add comments for these contants

http://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/net...
chrome/browser/chromeos/net/network_portal_detector.cc:179: 
I think it makes sense to clear tasks here just in case.

detection_task_.Cancel()
detection_timeout_.Cancel();

http://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/net...
chrome/browser/chromeos/net/network_portal_detector.cc:209: LOG(INFO) << "Portal
detection starting attempt " << attempt_count_
Note that by default LOG(INFO) is not captured in Chrome log.

Makes sense to change logging to VLOG() for portal service and increase log
level for this module.

http://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/net...
chrome/browser/chromeos/net/network_portal_detector.cc:241: LOG(ERROR) << "On
portal detection completed";
LOG(INFO) or VLOG - see above.

http://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/net...
File chrome/browser/chromeos/net/network_portal_detector.h (right):

http://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/net...
chrome/browser/chromeos/net/network_portal_detector.h:97: void
PortalDetectionTimeout();
nit: please add short comment describing when this happens

[ygorshenin1, 2012-11-21 09:08:05]

Nikita and Matt, PTAL due to some additional refactoring in
chrome/browser/captive_portal/* and changes in logging.

https://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/ne...
File chrome/browser/chromeos/net/network_portal_detector.cc (right):

https://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/ne...
chrome/browser/chromeos/net/network_portal_detector.cc:24: const int
kMaxRequestAttempts = 3;
On 2012/11/20 12:58:45, Nikita Kostylev wrote:
> nit: Please add comments for these contants

Done.

https://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/ne...
chrome/browser/chromeos/net/network_portal_detector.cc:179: 
On 2012/11/20 12:58:45, Nikita Kostylev wrote:
> I think it makes sense to clear tasks here just in case.
> 
> detection_task_.Cancel()
> detection_timeout_.Cancel();

Done.

https://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/ne...
chrome/browser/chromeos/net/network_portal_detector.cc:209: LOG(INFO) << "Portal
detection starting attempt " << attempt_count_
On 2012/11/20 12:58:45, Nikita Kostylev wrote:
> Note that by default LOG(INFO) is not captured in Chrome log.
> 
> Makes sense to change logging to VLOG() for portal service and increase log
> level for this module.

Done.

https://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/ne...
chrome/browser/chromeos/net/network_portal_detector.cc:241: LOG(ERROR) << "On
portal detection completed";
On 2012/11/20 12:58:45, Nikita Kostylev wrote:
> LOG(INFO) or VLOG - see above.

Done.

https://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/ne...
File chrome/browser/chromeos/net/network_portal_detector.h (right):

https://codereview.chromium.org/11419070/diff/2001/chrome/browser/chromeos/ne...
chrome/browser/chromeos/net/network_portal_detector.h:97: void
PortalDetectionTimeout();
On 2012/11/20 12:58:45, Nikita Kostylev wrote:
> nit: please add short comment describing when this happens

Done.

[Nikita (slow), 2012-11-21 11:32:10]

lgtm

https://codereview.chromium.org/11419070/diff/9/chrome/browser/captive_portal...
File chrome/browser/captive_portal/captive_portal_detector.h (right):

https://codereview.chromium.org/11419070/diff/9/chrome/browser/captive_portal...
chrome/browser/captive_portal/captive_portal_detector.h:69: static std::string
CaptivePortalResultToString(Result result);
Static should go first.

[ygorshenin1, 2012-11-21 12:11:11]

https://codereview.chromium.org/11419070/diff/9/chrome/browser/captive_portal...
File chrome/browser/captive_portal/captive_portal_detector.h (right):

https://codereview.chromium.org/11419070/diff/9/chrome/browser/captive_portal...
chrome/browser/captive_portal/captive_portal_detector.h:69: static std::string
CaptivePortalResultToString(Result result);
On 2012/11/21 11:32:10, Nikita Kostylev wrote:
> Static should go first.

Done.

[commit-bot: I haz the power, 2012-11-22 08:30:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/11419070/12012

[commit-bot: I haz the power, 2012-11-22 11:22:21]

Change committed as 169246