Fix use-after-free bug when hints fetch fails

Previously, when we came back from a failed fetch, we would erase the
hints fetcher (which would subsequently erase the active URL loader).
But OnURLLoadComplete wants to destruct the active URL loader on its
own and at that time, active_url_loader_ is null.

Bug: 1050481
Change-Id: I0d49796fd0d70a3ab1eb52233a44c579d84fd179
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063788Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Reviewed-by: rajendrant <rajendrant@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742432}

chrome/browser/optimization_guide/hints_fetcher_browsertest.cc

@@ -427,7 +427,6 @@ class HintsFetcherDisabledBrowserTest : public InProcessBrowserTest {
 
       std::string serialized_request = "Not a proto";
       response->set_content(serialized_request);
-
     } else {
       NOTREACHED();
     }
@@ -773,6 +772,28 @@ IN_PROC_BROWSER_TEST_F(
       0);
 }
 
+IN_PROC_BROWSER_TEST_F(
+    HintsFetcherBrowserTest,
+    DISABLE_ON_WIN_MAC_CHROMEOS(
+        HintsFetcherWithResponsesUnsuccessfulAtNavigationTime)) {
+  const base::HistogramTester* histogram_tester = GetHistogramTester();
+
+  SetResponseType(HintsFetcherRemoteResponseType::kUnsuccessful);
+
+  // Set the ECT to force a fetch at navigation time.
+  g_browser_process->network_quality_tracker()
+      ->ReportEffectiveConnectionTypeForTesting(
+          net::EFFECTIVE_CONNECTION_TYPE_2G);
+
+  ui_test_utils::NavigateToURL(browser(), GURL("https://unsuccessful.com/"));
+
+  // We expect that we requested hints for 1 URL.
+  EXPECT_GE(RetryForHistogramUntilCountReached(
+                histogram_tester,
+                "OptimizationGuide.HintsFetcher.GetHintsRequest.UrlCount", 1),
+            1);
+}
+
 IN_PROC_BROWSER_TEST_F(
     HintsFetcherBrowserTest,
     DISABLE_ON_WIN_MAC_CHROMEOS(HintsFetcherClearFetchedHints)) {

components/optimization_guide/hints_fetcher.cc

@@ -353,10 +353,6 @@ void HintsFetcher::UpdateHostsSuccessfullyFetched() {
 void HintsFetcher::OnURLLoadComplete(
     std::unique_ptr<std::string> response_body) {
   SEQUENCE_CHECKER(sequence_checker_);
-  DCHECK(active_url_loader_);
-
-  if (!active_url_loader_)
-    return;
 
   int response_code = -1;
   if (active_url_loader_->ResponseInfo() &&
@@ -364,9 +360,12 @@ void HintsFetcher::OnURLLoadComplete(
     response_code =
         active_url_loader_->ResponseInfo()->headers->response_code();
   }
-  HandleResponse(response_body ? *response_body : "",
-                 active_url_loader_->NetError(), response_code);
+  auto net_error = active_url_loader_->NetError();
+  // Reset the active URL loader here since actions happening during response
+  // handling may destroy |this|.
   active_url_loader_.reset();
+
+  HandleResponse(response_body ? *response_body : "", net_error, response_code);
 }
 
 std::vector<std::string> HintsFetcher::GetSizeLimitedHostsDueForHintsRefresh(