Cancel browse url check only if the check was sent.

It is not needed to cancel the check if the request is never sent.
Also introduce a helper function to send the check to the
database_manager. This helper function will be reused in another
refactoring CL.

Bug: 1050859
Change-Id: I64f1d721f77c60c93d0829dd0c0bf8c791b4f053
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2067462Reviewed-by: Daniel Rubery <drubery@chromium.org>
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743411}

components/safe_browsing/core/browser/safe_browsing_url_checker_impl.cc

@@ -219,7 +219,9 @@ void SafeBrowsingUrlCheckerImpl::OnUrlResult(const GURL& url,
 void SafeBrowsingUrlCheckerImpl::OnTimeout() {
   RecordCheckUrlTimeout(/*timed_out=*/true);
 
-  database_manager_->CancelCheck(this);
+  if (browse_url_check_sent_) {
+    database_manager_->CancelCheck(this);
+  }
 
   // Any pending callbacks on this URL check should be skipped.
   weak_factory_.InvalidateWeakPtrs();
@@ -340,6 +342,7 @@ void SafeBrowsingUrlCheckerImpl::ProcessUrls() {
     } else {
       safe_synchronously = database_manager_->CheckBrowseUrl(
           url, url_checker_delegate_->GetThreatTypes(), this);
+      browse_url_check_sent_ = true;
     }
 
     if (safe_synchronously) {
@@ -428,12 +431,7 @@ void SafeBrowsingUrlCheckerImpl::OnCheckUrlForHighConfidenceAllowlist(
   if (did_match_allowlist) {
     // If the URL matches the high-confidence allowlist, still do the hash based
     // checks.
-    if (database_manager_->CheckBrowseUrl(
-            url, url_checker_delegate_->GetThreatTypes(), this)) {
-      // No match found in the local database. Safe to call |OnUrlResult| here
-      // directly.
-      OnUrlResult(url, SB_THREAT_TYPE_SAFE, ThreatMetadata());
-    }
+    PerformHashBasedCheck(url);
     return;
   }
 
@@ -512,4 +510,15 @@ void SafeBrowsingUrlCheckerImpl::SetWebUIToken(int token) {
   url_web_ui_token_ = token;
 }
 
+void SafeBrowsingUrlCheckerImpl::PerformHashBasedCheck(const GURL& url) {
+  DCHECK(CurrentlyOnThread(ThreadID::IO));
+  browse_url_check_sent_ = true;
+  if (database_manager_->CheckBrowseUrl(
+          url, url_checker_delegate_->GetThreatTypes(), this)) {
+    // No match found in the local database. Safe to call |OnUrlResult| here
+    // directly.
+    OnUrlResult(url, SB_THREAT_TYPE_SAFE, ThreatMetadata());
+  }
+}
+
 }  // namespace safe_browsing

components/safe_browsing/core/browser/safe_browsing_url_checker_impl.h

@@ -176,6 +176,9 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
   // case none of the members of this object should be touched again.
   bool RunNextCallback(bool proceed, bool showed_interstitial);
 
+  // Perform the hash based check for the url.
+  void PerformHashBasedCheck(const GURL& url);
+
   // Called when the |request| from the real-time lookup service is sent.
   void OnRTLookupRequest(std::unique_ptr<RTLookupRequest> request);
 
@@ -233,6 +236,13 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
   // Whether real time lookup is enabled for this request.
   bool real_time_lookup_enabled_;
 
+  // Whether the browse url check request is sent to |database_manager_|.
+  // This boolean is set to true once the first url check is sent, and never
+  // reset to false, because there are separate pending checks for each request
+  // to |database_manager_|. As long as the redirection is still happening,
+  // there is at least one check that needs to be cancelled.
+  bool browse_url_check_sent_ = false;
+
   // Unowned object used for getting and storing real time url check cache.
   // Must be NOT nullptr when real time url check is enabled and profile is not
   // delete. Can only be accessed in UI thread.