Check for null callback before start captive portal probe

If CaptivePortalDetector::Cancel() is called after
CaptivePortalDetector::DetectCaptivePortal() is run but before
CaptivePortalDetector::StartProbe is run, |detection_callback_|
is reset but a probe is still started. This causes a DCHECK fail
or null dereference. Check for null |detection_callback_| before
starting a probe.

Bug: 993388
Change-Id: I46cc2a5fb226cd2a96443387103648617068b065
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1753162Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Matthew Wang <matthewmwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687338}

components/captive_portal/captive_portal_detector.cc

@@ -42,7 +42,13 @@ const char CaptivePortalDetector::kDefaultURL[] =
 
 CaptivePortalDetector::CaptivePortalDetector(
     network::mojom::URLLoaderFactory* loader_factory)
-    : loader_factory_(loader_factory), weak_factory_(this) {}
+    : loader_factory_(loader_factory)
+#if defined(OS_CHROMEOS)
+      ,
+      weak_factory_(this)
+#endif
+{
+}
 
 CaptivePortalDetector::~CaptivePortalDetector() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -55,6 +61,7 @@ void CaptivePortalDetector::DetectCaptivePortal(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!FetchingURL());
   DCHECK(detection_callback_.is_null());
+  DCHECK(!detection_callback.is_null());
 
   detection_callback_ = std::move(detection_callback);
 
@@ -95,6 +102,10 @@ void CaptivePortalDetector::StartProbe(
 void CaptivePortalDetector::Cancel() {
   simple_loader_.reset();
   detection_callback_.Reset();
+#if defined(OS_CHROMEOS)
+  // Cancel any pending calls to StartProbe().
+  weak_factory_.InvalidateWeakPtrs();
+#endif
 }
 
 void CaptivePortalDetector::OnSimpleLoaderComplete(

components/captive_portal/captive_portal_detector.h

@@ -6,13 +6,11 @@
 #define COMPONENTS_CAPTIVE_PORTAL_CAPTIVE_PORTAL_DETECTOR_H_
 
 #include <memory>
-#include <string>
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
 #include "base/time/time.h"
 #include "components/captive_portal/captive_portal_export.h"
@@ -21,6 +19,10 @@
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 
+#if defined(OS_CHROMEOS)
+#include "base/memory/weak_ptr.h"
+#endif
+
 class GURL;
 
 namespace captive_portal {
@@ -112,7 +114,9 @@ class CAPTIVE_PORTAL_EXPORT CaptivePortalDetector {
 
   SEQUENCE_CHECKER(sequence_checker_);
 
+#if defined(OS_CHROMEOS)
   base::WeakPtrFactory<CaptivePortalDetector> weak_factory_;
+#endif
 
   DISALLOW_COPY_AND_ASSIGN(CaptivePortalDetector);
 };