[M119] [HFM] Don't bind WebContents pointer in HSTS query callback

Because the call to NetworkContext::IsHSTSActiveForHost() is async, it
is possible for the WebContents to be destroyed before the callback to
MaybeCreateLoaderOnHstsQueryCompleted() is run. This changes the
callback to get the WebContents using `frame_tree_node_id_` again and
verify that it is still valid when the callback is run, and similarly
reconstruct the Profile and TabHelper rather than needing to bind them
into the callback.

(cherry picked from commit 4208e44)

Fixed: 1499515
Change-Id: I52ffefc5586771d21f4860eefca09b933b87ec6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5014874
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Chris Thompson <cthomp@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1221998}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5021108
Auto-Submit: Chris Thompson <cthomp@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/6045@{#1291}
Cr-Branched-From: 905e8bd-refs/heads/main@{#1204232}

chrome/browser/ssl/https_upgrades_interceptor.cc

@@ -337,7 +337,7 @@ void HttpsUpgradesInterceptor::MaybeCreateLoader(
   auto query_complete_callback = base::BindOnce(
       &HttpsUpgradesInterceptor::MaybeCreateLoaderOnHstsQueryCompleted,
       weak_factory_.GetWeakPtr(), tentative_resource_request,
-      std::move(callback), profile, web_contents, tab_helper);
+      std::move(callback));
   network::mojom::NetworkContext* network_context =
       profile->GetDefaultStoragePartition()->GetNetworkContext();
   network_context->IsHSTSActiveForHost(
@@ -350,12 +350,28 @@ void HttpsUpgradesInterceptor::MaybeCreateLoader(
 void HttpsUpgradesInterceptor::MaybeCreateLoaderOnHstsQueryCompleted(
     const network::ResourceRequest& tentative_resource_request,
     content::URLLoaderRequestInterceptor::LoaderCallback callback,
-    Profile* profile,
-    content::WebContents* web_contents,
-    HttpsOnlyModeTabHelper* tab_helper,
     bool is_hsts_active_for_host) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  // Reconstruct objects here instead of binding them as parameters to this
+  // callback method.
+  //
+  // It's possible for the WebContents to be destroyed during the
+  // asynchronous HSTS query call, before this callback is run. If it no longer
+  // exists, don't upgrade and return. (See crbug.com/1499515.)
+  content::WebContents* web_contents =
+      content::WebContents::FromFrameTreeNodeId(frame_tree_node_id_);
+  if (!web_contents) {
+    std::move(callback).Run({});
+    return;
+  }
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  HttpsOnlyModeTabHelper* tab_helper =
+      HttpsOnlyModeTabHelper::FromWebContents(web_contents);
+  CHECK(profile);
+  CHECK(tab_helper);
+
   // Don't upgrade this request if HSTS is active for this host.
   if (is_hsts_active_for_host) {
     RecordNavigationRequestSecurityLevel(

chrome/browser/ssl/https_upgrades_interceptor.h

@@ -80,9 +80,6 @@ class HttpsUpgradesInterceptor : public content::URLLoaderRequestInterceptor,
   void MaybeCreateLoaderOnHstsQueryCompleted(
       const network::ResourceRequest& tentative_resource_request,
       content::URLLoaderRequestInterceptor::LoaderCallback callback,
-      Profile* profile,
-      content::WebContents* web_contents,
-      HttpsOnlyModeTabHelper* tab_helper,
       bool is_hsts_active_for_host);
 
   // Sets the ports used by the EmbeddedTestServer (which uses random ports)