Bug 1332271 - Do not cancel timer when captive portal request times out r=mcmanus

The fact that nsICaptivePortalServiceCallback.complete got called with a true
argument made it difficult to be sure when the you were actually in a captive
portal, and when the network timed out.
Moreover, one artefact of the initial plan for the captive portal service was
that we'd cancel the timer after the first request succeeded, making the backoff
mechanism not run at all, and only checked for CP when instructed by nsIOService.
This patch changes captivedetect.js to send back success=false when the retry
count is exceeded - it's equivalent to an aborted request anyway - doesn't
cancel the timeer, and changes how we set the current state of the captive portal.

MozReview-Commit-ID: 4RV50KPbEdt
This commit is contained in:
Valentin Gosu 2017-01-24 15:43:30 +01:00
parent 03f24f75c7
commit 5650342313
3 changed files with 16 additions and 13 deletions

View File

@ -45,6 +45,8 @@ CaptivePortalService::CaptivePortalService()
CaptivePortalService::~CaptivePortalService()
{
LOG(("CaptivePortalService::~CaptivePortalService isParentProcess:%d\n",
XRE_GetProcessType() == GeckoProcessType_Default));
}
nsresult
@ -76,6 +78,7 @@ CaptivePortalService::PerformCheck()
nsresult
CaptivePortalService::RearmTimer()
{
LOG(("CaptivePortalService::RearmTimer\n"));
// Start a timer to recheck
MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
if (mTimer) {
@ -292,13 +295,12 @@ CaptivePortalService::Observe(nsISupports *aSubject,
// The user has successfully logged in. We have connectivity.
mState = UNLOCKED_PORTAL;
mLastChecked = TimeStamp::Now();
mRequestInProgress = false;
mSlackCount = 0;
mDelay = mMinInterval;
RearmTimer();
} else if (!strcmp(aTopic, kAbortCaptivePortalLoginEvent)) {
// The login has been aborted
mRequestInProgress = false;
mState = UNKNOWN;
mLastChecked = TimeStamp::Now();
mSlackCount = 0;
@ -336,15 +338,16 @@ CaptivePortalService::Complete(bool success)
LOG(("CaptivePortalService::Complete(success=%d) mState=%d\n", success, mState));
MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
mLastChecked = TimeStamp::Now();
if ((mState == UNKNOWN || mState == NOT_CAPTIVE) && success) {
mState = NOT_CAPTIVE;
// If this check succeeded and we have never been in a captive portal
// since the service was started, there is no need to keep polling
if (!mEverBeenCaptive) {
mDelay = 0;
if (mTimer) {
mTimer->Cancel();
}
// Note: this callback gets called when:
// 1. the request is completed, and content is valid (success == true)
// 2. when the request is aborted or times out (success == false)
if (success) {
if (mEverBeenCaptive) {
mState = UNLOCKED_PORTAL;
} else {
mState = NOT_CAPTIVE;
}
}

View File

@ -353,7 +353,7 @@ CaptivePortalDetector.prototype = {
debug("retry-Detection: " + this._runningRequest.retryCount + "/" + this._maxRetryCount);
this._startDetection();
} else {
this.executeCallback(true);
this.executeCallback(false);
}
},

View File

@ -35,7 +35,7 @@ function test_portal_not_found() {
},
complete: function complete(success) {
do_check_eq(++step, 2);
do_check_true(success);
do_check_false(success);
do_check_eq(attempt, 6);
gServer.stop(do_test_finished);
},