[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.

Fixed: 1499515
Change-Id: I52ffefc5586771d21f4860eefca09b933b87ec6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5014874
Reviewed-by: Mustafa Emre Acer <[email protected]>
Commit-Queue: Chris Thompson <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1221998}

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)