Skip to content

Commit

Permalink
[Merge] CaptivePortalDetector: Restore DCHECKs and add safety test.
Browse files Browse the repository at this point in the history
This downgrades some CHECKS back to DCHECKs after the investigation
for crbug.com/1361443 has completed.

It also adds a safety check to prevent crashes if DetectCaptivePortal
is called incorrectly and a comment describing the correct usage.

BUG=1382965
TEST=No more production crashes in CaptivePortalDetector::DetectCaptivePortal

(cherry picked from commit 681f054)

Change-Id: I8c656b772411d11b68ff3ed742d6caf3abe7664a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4020985
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1070449}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4026426
Commit-Queue: Matt Menke <mmenke@chromium.org>
Auto-Submit: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/branch-heads/5414@{#35}
Cr-Branched-From: 4417ee5-refs/heads/main@{#1070088}
  • Loading branch information
stevenjb authored and Chromium LUCI CQ committed Nov 15, 2022
1 parent ccca1f5 commit 871eb45
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
7 changes: 4 additions & 3 deletions components/captive_portal/core/captive_portal_detector.cc
Expand Up @@ -36,9 +36,10 @@ void CaptivePortalDetector::DetectCaptivePortal(
const net::NetworkTrafficAnnotationTag& traffic_annotation) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!FetchingURL());
CHECK(detection_callback_.is_null());
CHECK(!detection_callback.is_null());
DCHECK(!detection_callback.is_null());

if (!detection_callback_.is_null())
LOG(ERROR) << "DetectCaptivePortal called while request is pending.";
detection_callback_ = std::move(detection_callback);

StartProbe(traffic_annotation, url);
Expand Down Expand Up @@ -83,7 +84,7 @@ void CaptivePortalDetector::OnSimpleLoaderComplete(
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
CHECK_EQ(state_, State::kProbe);
CHECK(FetchingURL());
CHECK(!detection_callback_.is_null());
DCHECK(!detection_callback_.is_null());

int response_code = 0;
net::HttpResponseHeaders* headers = nullptr;
Expand Down
5 changes: 4 additions & 1 deletion components/captive_portal/core/captive_portal_detector.h
Expand Up @@ -53,7 +53,10 @@ class CAPTIVE_PORTAL_EXPORT CaptivePortalDetector {
~CaptivePortalDetector();

// Triggers a check for a captive portal. After completion, runs the
// |callback|.
// |callback|. Only one detection attempt is expected to be in progress.
// If called again before |callback| is invoked, Cancel() should be called
// first, otherwise the first request and callback will be implicitly
// cancelled. and an ERROR will be logged
void DetectCaptivePortal(
const GURL& url,
DetectionCallback callback,
Expand Down

0 comments on commit 871eb45

Please sign in to comment.