[iOS] Associate SB allow list decisions with navigation URL

This CL introduces SafeBrowsingQueryManager, which manages URL
queries on the IO thread and notifies observers on the UI thread
when they are finished.  This was created to have a model object
that stored the NavigationItem ID associated with a particular
URL check request.  The previous approach assumed that when a
sub frame UnsafeResource was encountered, it was always from the
last-committed NavigationItem, but this heuristic is not always
correct, so the ID is stored in the query.

Using the query manager, UnsafeResource.navigation_url can be
reliably set to the correct value and used to drive the allow-
list decisions for those resources. In particular, the
navigation_url for main frame queries is simply the query url,
but the navigation_url for subframe queries is the url of the
corresponding main frame, obtained from the main frame's
NavigationItem. This means that a user decision to allow a
navigation to an unsafe subframe is associated with the
main frame URL rather than the subframe URL, matching other
platforms.

Patch originally by kkhorimoto@chromium.org, rebased and
landed by ajuma@.

Bug: 1084741, 1084735
Change-Id: I789535991a6ec05ec635d93b878a3cadd20dfc7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2217092
Commit-Queue: Ali Juma <ajuma@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Ali Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814899}

components/safe_browsing/ios/browser/BUILD.gn

@@ -10,6 +10,7 @@ source_set("allow_list") {
 
   deps = [
     "//components/safe_browsing/core/db:v4_protocol_manager_util",
+    "//components/security_interstitials/core:unsafe_resource",
     "//ios/web/public",
     "//url",
   ]

components/safe_browsing/ios/browser/safe_browsing_url_allow_list.h

@@ -11,6 +11,7 @@
 #include "base/observer_list.h"
 #include "base/observer_list_types.h"
 #include "components/safe_browsing/core/db/v4_protocol_manager_util.h"
+#include "components/security_interstitials/core/unsafe_resource.h"
 #import "ios/web/public/web_state_user_data.h"
 #include "url/gurl.h"
 
@@ -50,6 +51,11 @@ class SafeBrowsingUrlAllowList
 
   ~SafeBrowsingUrlAllowList() override;
 
+  // Returns the URL under which allow list decisions should be stored for
+  // |resource|.
+  static GURL GetDecisionUrl(
+      const security_interstitials::UnsafeResource& resource);
+
   // Adds and removes observers.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);

components/safe_browsing/ios/browser/safe_browsing_url_allow_list.mm

@@ -14,6 +14,13 @@ using safe_browsing::SBThreatType;
 
 WEB_STATE_USER_DATA_KEY_IMPL(SafeBrowsingUrlAllowList)
 
+// static
+GURL SafeBrowsingUrlAllowList::GetDecisionUrl(
+    const security_interstitials::UnsafeResource& resource) {
+  return resource.navigation_url.is_valid() ? resource.navigation_url
+                                            : resource.url;
+}
+
 SafeBrowsingUrlAllowList::SafeBrowsingUrlAllowList(web::WebState* web_state)
     : web_state_(web_state) {}
 

ios/chrome/browser/safe_browsing/BUILD.gn

@@ -15,6 +15,8 @@ source_set("safe_browsing") {
     "safe_browsing_blocking_page.mm",
     "safe_browsing_error.h",
     "safe_browsing_error.mm",
+    "safe_browsing_query_manager.h",
+    "safe_browsing_query_manager.mm",
     "safe_browsing_service.h",
     "safe_browsing_service_impl.h",
     "safe_browsing_service_impl.mm",
@@ -141,6 +143,7 @@ source_set("unit_tests") {
     "pending_unsafe_resource_storage_unittest.mm",
     "real_time_url_lookup_service_factory_unittest.mm",
     "safe_browsing_blocking_page_unittest.mm",
+    "safe_browsing_query_manager_unittest.mm",
     "safe_browsing_service_unittest.mm",
     "safe_browsing_tab_helper_unittest.mm",
     "safe_browsing_unsafe_resource_container_unittest.mm",

ios/chrome/browser/safe_browsing/pending_unsafe_resource_storage.mm

@@ -18,9 +18,10 @@ namespace {
 // Returns whether a pending decision exists for |resource|.
 bool IsUnsafeResourcePending(const UnsafeResource& resource) {
   SafeBrowsingUrlAllowList* allow_list = GetAllowListForResource(resource);
+  GURL decision_url = SafeBrowsingUrlAllowList::GetDecisionUrl(resource);
   std::set<SBThreatType> pending_threat_types;
   return allow_list &&
-         allow_list->IsUnsafeNavigationDecisionPending(resource.url,
+         allow_list->IsUnsafeNavigationDecisionPending(decision_url,
                                                        &pending_threat_types) &&
          pending_threat_types.find(resource.threat_type) !=
              pending_threat_types.end();
@@ -104,7 +105,7 @@ void PendingUnsafeResourceStorage::ResourcePolicyObserver::ThreatPolicyUpdated(
     SafeBrowsingUrlAllowList::Policy policy) {
   const UnsafeResource* resource = storage_->resource();
   if (policy == SafeBrowsingUrlAllowList::Policy::kPending ||
-      url != resource->url || threat_type != resource->threat_type) {
+      url != resource->navigation_url || threat_type != resource->threat_type) {
     return;
   }
 
@@ -120,7 +121,7 @@ void PendingUnsafeResourceStorage::ResourcePolicyObserver::
         SafeBrowsingUrlAllowList::Policy policy) {
   const UnsafeResource* resource = storage_->resource();
   if (policy == SafeBrowsingUrlAllowList::Policy::kPending ||
-      url != resource->url ||
+      url != resource->navigation_url ||
       threat_types.find(resource->threat_type) == threat_types.end()) {
     return;
   }

ios/chrome/browser/safe_browsing/pending_unsafe_resource_storage_unittest.mm

@@ -23,6 +23,7 @@ class PendingUnsafeResourceStorageTest : public PlatformTest {
     // Create a resource and add it as a pending decision.
     UnsafeResource resource;
     resource.url = url_;
+    resource.navigation_url = url_;
     resource.web_state_getter = web_state_.CreateDefaultGetter();
     resource.threat_type = threat_type_;
     resource.callback =

ios/chrome/browser/safe_browsing/safe_browsing_blocking_page.mm

@@ -173,7 +173,7 @@ SafeBrowsingBlockingPage::SafeBrowsingControllerClient::
           resource.web_state_getter.Run(),
           CreateMetricsHelper(resource),
           GetApplicationContext()->GetApplicationLocale()),
-      url_(resource.url),
+      url_(SafeBrowsingUrlAllowList::GetDecisionUrl(resource)),
       threat_type_(resource.threat_type) {}
 
 SafeBrowsingBlockingPage::SafeBrowsingControllerClient::

ios/chrome/browser/safe_browsing/safe_browsing_query_manager.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_QUERY_MANAGER_H_
+#define IOS_CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_QUERY_MANAGER_H_
+
+#include <map>
+
+#include "base/containers/flat_map.h"
+#include "base/containers/unique_ptr_adapters.h"
+#include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
+#include "base/optional.h"
+#include "components/safe_browsing/core/browser/safe_browsing_url_checker_impl.h"
+#include "components/safe_browsing/core/db/database_manager.h"
+#include "components/safe_browsing/core/db/v4_protocol_manager_util.h"
+#include "components/security_interstitials/core/unsafe_resource.h"
+#import "ios/web/public/navigation/web_state_policy_decider.h"
+#import "ios/web/public/web_state_user_data.h"
+#include "url/gurl.h"
+
+namespace web {
+class NavigationItem;
+}
+
+// A helper object that manages the Safe Browsing URL queries for a single
+// WebState.
+class SafeBrowsingQueryManager
+    : public web::WebStateUserData<SafeBrowsingQueryManager> {
+ public:
+  // Struct used to trigger URL check queries.
+  struct Query {
+    explicit Query(const GURL& url,
+                   const std::string& http_method,
+                   int main_frame_item_id = -1);
+    Query() = delete;
+    Query(const Query&);
+    virtual ~Query();
+
+    // Operator overloaded so it can be compared with std::less.
+    bool operator<(const Query& other) const;
+
+    // Whether the query is for the main frame.
+    bool IsMainFrame() const;
+
+    // The URL whose safety is being checked.
+    const GURL url;
+    // The HTTP method.
+    const std::string http_method;
+    // The ID of the NavigationItem triggering the URL check.  -1 for main-frame
+    // URL checks.
+    const int main_frame_item_id;
+    // The unique ID for the query.
+    const size_t query_id;
+  };
+
+  // Struct used to store URL check results.
+  struct Result {
+    Result();
+    Result(const Result&);
+    Result& operator=(const Result&);
+    ~Result();
+
+    // Whether navigations to the URL should proceed.
+    bool proceed = false;
+    // Whether an error page should be shown for the URL.
+    bool show_error_page = false;
+    // The UnsafeResource created for the URL check, if any.
+    base::Optional<security_interstitials::UnsafeResource> resource;
+  };
+
+  // Observer class for the query manager.
+  class Observer : public base::CheckedObserver {
+   public:
+    // Notifies observers that |query| has completed with |result|.
+    virtual void SafeBrowsingQueryFinished(SafeBrowsingQueryManager* manager,
+                                           const Query& query,
+                                           const Result& result) {}
+
+    // Called when |manager| is about to be destroyed.
+    virtual void SafeBrowsingQueryManagerDestroyed(
+        SafeBrowsingQueryManager* manager) {}
+  };
+
+  ~SafeBrowsingQueryManager() override;
+
+  SafeBrowsingQueryManager(const SafeBrowsingQueryManager&) = delete;
+  SafeBrowsingQueryManager& operator=(const SafeBrowsingQueryManager&) = delete;
+
+  // Adds and removes observers.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  // Starts the URL check for |query|.
+  void StartQuery(const Query& query);
+
+  // Stores |resource| in the result for the query that triggered the URL check.
+  // No-ops if there is no active query for |resource|'s URL.
+  void StoreUnsafeResource(
+      const security_interstitials::UnsafeResource& resource);
+
+ private:
+  friend class web::WebStateUserData<SafeBrowsingQueryManager>;
+
+  // Queries the Safe Browsing database using SafeBrowsingUrlCheckerImpls. This
+  // class may be constructed on the UI thread but otherwise must only be used
+  // and destroyed on the IO thread.
+  class UrlCheckerClient : public base::SupportsWeakPtr<UrlCheckerClient> {
+   public:
+    UrlCheckerClient();
+    ~UrlCheckerClient();
+
+    UrlCheckerClient(const UrlCheckerClient&) = delete;
+    UrlCheckerClient& operator=(const UrlCheckerClient&) = delete;
+
+    // Queries the database using the given |url_checker|, for a request with
+    // the given |url| and the given HTTP |method|. After receiving a result
+    // from the database, runs the given |callback| on the UI thread with the
+    // result.
+    void CheckUrl(
+        std::unique_ptr<safe_browsing::SafeBrowsingUrlCheckerImpl> url_checker,
+        const GURL& url,
+        const std::string& method,
+        base::OnceCallback<void(bool proceed, bool show_error_page)> callback);
+
+   private:
+    // Called by |url_checker| with the initial result of performing a url
+    // check. |url_checker| must be non-null. This is an implementation of
+    // SafeBrowsingUrlCheckerImpl::NativeUrlCheckCallBack. |slow_check_notifier|
+    // is an out-parameter; when a non-null value is passed in, it is set to a
+    // callback that receives the final result of the url check.
+    void OnCheckUrlResult(
+        safe_browsing::SafeBrowsingUrlCheckerImpl* url_checker,
+        safe_browsing::SafeBrowsingUrlCheckerImpl::NativeUrlCheckNotifier*
+            slow_check_notifier,
+        bool proceed,
+        bool showed_interstitial);
+
+    // Called by |url_checker| with the final result of performing a url check.
+    // |url_checker| must be non-null. This is an implementation of
+    // SafeBrowsingUrlCheckerImpl::NativeUrlCheckNotifier.
+    void OnCheckComplete(safe_browsing::SafeBrowsingUrlCheckerImpl* url_checker,
+                         bool proceed,
+                         bool showed_interstitial);
+
+    // This maps SafeBrowsingUrlCheckerImpls that have started but not completed
+    // a url check to the callback that should be invoked once the url check is
+    // complete.
+    base::flat_map<std::unique_ptr<safe_browsing::SafeBrowsingUrlCheckerImpl>,
+                   base::OnceCallback<void(bool proceed, bool show_error_page)>,
+                   base::UniquePtrComparator>
+        active_url_checkers_;
+  };
+
+  explicit SafeBrowsingQueryManager(web::WebState* web_state);
+
+  // Used as the completion callback for URL queries executed by
+  // |url_checker_client_|.
+  void UrlCheckFinished(const Query query, bool proceed, bool show_error_page);
+
+  // The WebState whose URL queries are being managed.
+  web::WebState* web_state_ = nullptr;
+  // The checker client.  Used to communicate with the database on the IO
+  // thread.
+  std::unique_ptr<UrlCheckerClient> url_checker_client_;
+  // The results for each active query.
+  std::map<const Query, Result> results_;
+  // The observers.
+  base::ObserverList<Observer, /*check_empty=*/true> observers_;
+  // The weak pointer factory.
+  base::WeakPtrFactory<SafeBrowsingQueryManager> weak_factory_{this};
+
+  WEB_STATE_USER_DATA_KEY_DECL();
+};
+
+#endif  // IOS_CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_QUERY_MANAGER_H_

ios/chrome/browser/safe_browsing/safe_browsing_query_manager.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/safe_browsing/safe_browsing_query_manager.h"
+
+#include "base/bind_helpers.h"
+#include "base/check_op.h"
+#include "base/task/post_task.h"
+#include "ios/chrome/browser/application_context.h"
+#include "ios/chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "ios/web/public/thread/web_task_traits.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+using safe_browsing::ResourceType;
+using security_interstitials::UnsafeResource;
+
+WEB_STATE_USER_DATA_KEY_IMPL(SafeBrowsingQueryManager)
+
+namespace {
+// Creates a unique ID for a new Query.
+size_t CreateQueryID() {
+  static size_t query_id = 0;
+  return query_id++;
+}
+}  // namespace
+
+#pragma mark - SafeBrowsingQueryManager
+
+SafeBrowsingQueryManager::SafeBrowsingQueryManager(web::WebState* web_state)
+    : web_state_(web_state),
+      url_checker_client_(std::make_unique<UrlCheckerClient>()) {
+  DCHECK(web_state_);
+}
+
+SafeBrowsingQueryManager::~SafeBrowsingQueryManager() {
+  for (auto& observer : observers_) {
+    observer.SafeBrowsingQueryManagerDestroyed(this);
+  }
+
+  base::DeleteSoon(FROM_HERE, {web::WebThread::IO},
+                   url_checker_client_.release());
+}
+
+void SafeBrowsingQueryManager::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void SafeBrowsingQueryManager::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void SafeBrowsingQueryManager::StartQuery(const Query& query) {
+  // Store the query request.
+  results_.insert({query, Result()});
+
+  // Create a URL checker and perform the query on the IO thread.
+  ResourceType resource_type =
+      query.IsMainFrame() ? ResourceType::kMainFrame : ResourceType::kSubFrame;
+  std::unique_ptr<safe_browsing::SafeBrowsingUrlCheckerImpl> url_checker =
+      GetApplicationContext()->GetSafeBrowsingService()->CreateUrlChecker(
+          resource_type, web_state_);
+  base::OnceCallback<void(bool proceed, bool show_error_page)> callback =
+      base::BindOnce(&SafeBrowsingQueryManager::UrlCheckFinished,
+                     weak_factory_.GetWeakPtr(), query);
+  base::PostTask(
+      FROM_HERE, {web::WebThread::IO},
+      base::BindOnce(&UrlCheckerClient::CheckUrl,
+                     url_checker_client_->AsWeakPtr(), std::move(url_checker),
+                     query.url, query.http_method, std::move(callback)));
+}
+
+void SafeBrowsingQueryManager::StoreUnsafeResource(
+    const UnsafeResource& resource) {
+  bool is_main_frame =
+      resource.resource_type == safe_browsing::ResourceType::kMainFrame;
+  auto it = std::find_if(results_.begin(), results_.end(),
+                         [&resource, &is_main_frame](const auto& pair) {
+                           return pair.first.url == resource.url &&
+                                  is_main_frame == pair.first.IsMainFrame();
+                         });
+  if (it == results_.end())
+    return;
+
+  UnsafeResource resource_copy = resource;
+  resource_copy.callback = base::DoNothing();
+  it->second.resource = resource;
+}
+
+#pragma mark Private
+
+void SafeBrowsingQueryManager::UrlCheckFinished(const Query query,
+                                                bool proceed,
+                                                bool show_error_page) {
+  auto query_result_pair = results_.find(query);
+  DCHECK(query_result_pair != results_.end());
+
+  // Store the query result.
+  Result& result = query_result_pair->second;
+  result.proceed = proceed;
+  result.show_error_page = show_error_page;
+
+  // If an error page is requested, an UnsafeResource must be stored before the
+  // execution of its completion block.
+  DCHECK(!show_error_page || result.resource);
+
+  // Notify observers of the completed URL check.
+  for (auto& observer : observers_) {
+    observer.SafeBrowsingQueryFinished(this, query, result);
+  }
+
+  // Clear out the state since the query is finished.
+  results_.erase(query_result_pair);
+}
+
+#pragma mark - SafeBrowsingQueryManager::Query
+
+SafeBrowsingQueryManager::Query::Query(const GURL& url,
+                                       const std::string& http_method,
+                                       int main_frame_item_id)
+    : url(url),
+      http_method(http_method),
+      main_frame_item_id(main_frame_item_id),
+      query_id(CreateQueryID()) {}
+
+SafeBrowsingQueryManager::Query::Query(const Query&) = default;
+
+SafeBrowsingQueryManager::Query::~Query() = default;
+
+bool SafeBrowsingQueryManager::Query::operator<(const Query& other) const {
+  return query_id < other.query_id;
+}
+
+bool SafeBrowsingQueryManager::Query::IsMainFrame() const {
+  return main_frame_item_id == -1;
+}
+
+#pragma mark - SafeBrowsingQueryManager::Result
+
+SafeBrowsingQueryManager::Result::Result() = default;
+
+SafeBrowsingQueryManager::Result::Result(const Result&) = default;
+
+SafeBrowsingQueryManager::Result& SafeBrowsingQueryManager::Result::operator=(
+    const Result&) = default;
+
+SafeBrowsingQueryManager::Result::~Result() = default;
+
+#pragma mark - SafeBrowsingQueryManager::UrlCheckerClient
+
+SafeBrowsingQueryManager::UrlCheckerClient::UrlCheckerClient() = default;
+
+SafeBrowsingQueryManager::UrlCheckerClient::~UrlCheckerClient() {
+  DCHECK_CURRENTLY_ON(web::WebThread::IO);
+}
+
+void SafeBrowsingQueryManager::UrlCheckerClient::CheckUrl(
+    std::unique_ptr<safe_browsing::SafeBrowsingUrlCheckerImpl> url_checker,
+    const GURL& url,
+    const std::string& method,
+    base::OnceCallback<void(bool proceed, bool show_error_page)> callback) {
+  DCHECK_CURRENTLY_ON(web::WebThread::IO);
+  safe_browsing::SafeBrowsingUrlCheckerImpl* url_checker_ptr =
+      url_checker.get();
+  active_url_checkers_[std::move(url_checker)] = std::move(callback);
+  url_checker_ptr->CheckUrl(url, method,
+                            base::BindOnce(&UrlCheckerClient::OnCheckUrlResult,
+                                           AsWeakPtr(), url_checker_ptr));
+}
+
+void SafeBrowsingQueryManager::UrlCheckerClient::OnCheckUrlResult(
+    safe_browsing::SafeBrowsingUrlCheckerImpl* url_checker,
+    safe_browsing::SafeBrowsingUrlCheckerImpl::NativeUrlCheckNotifier*
+        slow_check_notifier,
+    bool proceed,
+    bool showed_interstitial) {
+  DCHECK_CURRENTLY_ON(web::WebThread::IO);
+  DCHECK(url_checker);
+  if (slow_check_notifier) {
+    *slow_check_notifier = base::BindOnce(&UrlCheckerClient::OnCheckComplete,
+                                          AsWeakPtr(), url_checker);
+    return;
+  }
+
+  OnCheckComplete(url_checker, proceed, showed_interstitial);
+}
+
+void SafeBrowsingQueryManager::UrlCheckerClient::OnCheckComplete(
+    safe_browsing::SafeBrowsingUrlCheckerImpl* url_checker,
+    bool proceed,
+    bool showed_interstitial) {
+  DCHECK_CURRENTLY_ON(web::WebThread::IO);
+  DCHECK(url_checker);
+
+  auto it = active_url_checkers_.find(url_checker);
+  base::PostTask(
+      FROM_HERE, {web::WebThread::UI},
+      base::BindOnce(std::move(it->second), proceed, showed_interstitial));
+
+  active_url_checkers_.erase(it);
+}

ios/chrome/browser/safe_browsing/safe_browsing_query_manager_unittest.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/safe_browsing/safe_browsing_query_manager.h"
+
+#import <Foundation/Foundation.h>
+
+#include "base/test/scoped_feature_list.h"
+#include "components/safe_browsing/core/features.h"
+#include "components/security_interstitials/core/unsafe_resource.h"
+#import "ios/chrome/browser/safe_browsing/fake_safe_browsing_service.h"
+#import "ios/web/public/test/fakes/test_web_state.h"
+#include "ios/web/public/test/web_task_environment.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+using security_interstitials::UnsafeResource;
+using testing::_;
+
+namespace {
+// Mock observer for tests.
+class MockQueryManagerObserver : public SafeBrowsingQueryManager::Observer {
+ public:
+  MockQueryManagerObserver() {}
+  ~MockQueryManagerObserver() {}
+
+  MOCK_METHOD3(SafeBrowsingQueryFinished,
+               void(SafeBrowsingQueryManager*,
+                    const SafeBrowsingQueryManager::Query&,
+                    const SafeBrowsingQueryManager::Result&));
+
+  // Override rather than mocking so that the observer can remove itself.
+  void SafeBrowsingQueryManagerDestroyed(
+      SafeBrowsingQueryManager* manager) override {
+    manager_destroyed_ = true;
+    manager->RemoveObserver(this);
+  }
+  bool manager_destroyed() const { return manager_destroyed_; }
+
+ private:
+  bool manager_destroyed_ = false;
+};
+
+// Verifies the expected values passed to the SafeBrowsingQueryFinished()
+// callback.
+ACTION_P4(VerifyQueryFinished,
+          expected_url,
+          expected_http_method,
+          expected_main_frame_item_id,
+          is_url_safe) {
+  const SafeBrowsingQueryManager::Query& query = arg1;
+  EXPECT_EQ(expected_url, query.url);
+  EXPECT_EQ(expected_http_method, query.http_method);
+  EXPECT_EQ(expected_main_frame_item_id, query.main_frame_item_id);
+
+  const SafeBrowsingQueryManager::Result& result = arg2;
+  EXPECT_EQ(is_url_safe, result.proceed);
+  EXPECT_EQ(is_url_safe, !result.show_error_page);
+  if (is_url_safe) {
+    EXPECT_FALSE(result.resource);
+  } else {
+    ASSERT_TRUE(result.resource);
+    UnsafeResource resource = result.resource.value();
+    EXPECT_EQ(expected_url, resource.url);
+    EXPECT_NE(safe_browsing::SB_THREAT_TYPE_SAFE, resource.threat_type);
+  }
+}
+}  // namespace
+
+class SafeBrowsingQueryManagerTest
+    : public testing::TestWithParam<safe_browsing::ResourceType> {
+ protected:
+  SafeBrowsingQueryManagerTest()
+      : task_environment_(web::WebTaskEnvironment::IO_MAINLOOP),
+        web_state_(std::make_unique<web::TestWebState>()),
+        http_method_("GET"),
+        navigation_item_id_(
+            GetParam() == safe_browsing::ResourceType::kMainFrame ? -1 : 0) {
+    feature_list_.InitAndEnableFeature(
+        safe_browsing::kSafeBrowsingAvailableOnIOS);
+    SafeBrowsingQueryManager::CreateForWebState(web_state_.get());
+    manager()->AddObserver(&observer_);
+  }
+
+  SafeBrowsingQueryManager* manager() {
+    return SafeBrowsingQueryManager::FromWebState(web_state_.get());
+  }
+
+  web::WebTaskEnvironment task_environment_;
+  base::test::ScopedFeatureList feature_list_;
+  MockQueryManagerObserver observer_;
+  std::unique_ptr<web::WebState> web_state_;
+  std::string http_method_;
+  int navigation_item_id_ = 0;
+};
+
+// Tests a query for a safe URL.
+TEST_P(SafeBrowsingQueryManagerTest, SafeURLQuery) {
+  GURL url("http://chromium.test");
+  EXPECT_CALL(observer_, SafeBrowsingQueryFinished(manager(), _, _))
+      .WillOnce(VerifyQueryFinished(url, http_method_, navigation_item_id_,
+                                    /*is_url_safe=*/true));
+
+  // Start a URL check query for the safe URL and run the runloop until the
+  // result is received.
+  manager()->StartQuery(
+      SafeBrowsingQueryManager::Query(url, http_method_, navigation_item_id_));
+  base::RunLoop().RunUntilIdle();
+}
+
+// Tests a query for an unsafe URL.
+TEST_P(SafeBrowsingQueryManagerTest, UnsafeURLQuery) {
+  GURL url("http://" + FakeSafeBrowsingService::kUnsafeHost);
+  EXPECT_CALL(observer_, SafeBrowsingQueryFinished(manager(), _, _))
+      .WillOnce(VerifyQueryFinished(url, http_method_, navigation_item_id_,
+                                    /*is_url_safe=*/false));
+
+  // Start a URL check query for the unsafe URL and run the runloop until the
+  // result is received.  An UnsafeResource is stored before the query finishes
+  // to simulate the production behavior that adds a resource that will be used
+  // to populate the error page.
+  manager()->StartQuery(
+      SafeBrowsingQueryManager::Query(url, http_method_, navigation_item_id_));
+  UnsafeResource resource;
+  resource.url = url;
+  resource.threat_type = safe_browsing::SB_THREAT_TYPE_URL_PHISHING;
+  resource.resource_type = GetParam();
+  manager()->StoreUnsafeResource(resource);
+  base::RunLoop().RunUntilIdle();
+}
+
+// Tests observer callbacks for manager destruction.
+TEST_P(SafeBrowsingQueryManagerTest, ManagerDestruction) {
+  web_state_ = nullptr;
+  EXPECT_TRUE(observer_.manager_destroyed());
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    /* No InstantiationName */,
+    SafeBrowsingQueryManagerTest,
+    testing::Values(safe_browsing::ResourceType::kMainFrame,
+                    safe_browsing::ResourceType::kSubFrame));

ios/chrome/browser/safe_browsing/safe_browsing_service_unittest.mm

@@ -31,6 +31,7 @@
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
 #import "ios/chrome/browser/prerender/fake_prerender_service.h"
 #import "ios/chrome/browser/prerender/prerender_service_factory.h"
+#import "ios/chrome/browser/safe_browsing/safe_browsing_query_manager.h"
 #import "ios/chrome/browser/safe_browsing/safe_browsing_unsafe_resource_container.h"
 #import "ios/chrome/browser/safe_browsing/verdict_cache_manager_factory.h"
 #import "ios/chrome/test/testing_application_context.h"
@@ -59,6 +60,7 @@ class TestUrlCheckerClient {
   TestUrlCheckerClient(SafeBrowsingService* safe_browsing_service,
                        web::BrowserState* browser_state)
       : safe_browsing_service_(safe_browsing_service) {
+    SafeBrowsingQueryManager::CreateForWebState(&web_state_);
     SafeBrowsingUrlAllowList::CreateForWebState(&web_state_);
     SafeBrowsingUnsafeResourceContainer::CreateForWebState(&web_state_);
     web_state_.SetBrowserState(browser_state);

ios/chrome/browser/safe_browsing/safe_browsing_tab_helper.h

@@ -17,13 +17,14 @@
 #include "components/safe_browsing/core/browser/safe_browsing_url_checker_impl.h"
 #include "components/safe_browsing/core/db/database_manager.h"
 #include "components/safe_browsing/core/db/v4_protocol_manager_util.h"
+#import "ios/chrome/browser/safe_browsing/safe_browsing_query_manager.h"
 #import "ios/web/public/navigation/web_state_policy_decider.h"
 #include "ios/web/public/web_state_observer.h"
 #import "ios/web/public/web_state_user_data.h"
 #include "url/gurl.h"
 
-namespace safe_browsing {
-class SafeBrowsingUrlCheckerImpl;
+namespace web {
+class NavigationItem;
 }
 
 // A tab helper that uses Safe Browsing to check whether URLs that are being
@@ -39,59 +40,6 @@ class SafeBrowsingTabHelper
  private:
   friend class web::WebStateUserData<SafeBrowsingTabHelper>;
 
-  // Queries the Safe Browsing database using SafeBrowsingUrlCheckerImpls but
-  // doesn't do anything with the result yet. This class may be constructed on
-  // the UI thread but otherwise must only be used and destroyed on the IO
-  // thread.
-  class UrlCheckerClient : public base::SupportsWeakPtr<UrlCheckerClient> {
-   public:
-    UrlCheckerClient();
-    ~UrlCheckerClient();
-
-    UrlCheckerClient(const UrlCheckerClient&) = delete;
-    UrlCheckerClient& operator=(const UrlCheckerClient&) = delete;
-
-    // Queries the database using the given |url_checker|, for a request with
-    // the given |url| and the given HTTP |method|. After receiving a result
-    // from the database, runs the given |callback| on the UI thread with the
-    // result.
-    void CheckUrl(
-        std::unique_ptr<safe_browsing::SafeBrowsingUrlCheckerImpl> url_checker,
-        const GURL& url,
-        const std::string& method,
-        base::OnceCallback<void(web::WebStatePolicyDecider::PolicyDecision)>
-            callback);
-
-   private:
-    // Called by |url_checker| with the initial result of performing a url
-    // check. |url_checker| must be non-null. This is an implementation of
-    // SafeBrowsingUrlCheckerImpl::NativeUrlCheckCallBack. |slow_check_notifier|
-    // is an out-parameter; when a non-null value is passed in, it is set to a
-    // callback that receives the final result of the url check.
-    void OnCheckUrlResult(
-        safe_browsing::SafeBrowsingUrlCheckerImpl* url_checker,
-        safe_browsing::SafeBrowsingUrlCheckerImpl::NativeUrlCheckNotifier*
-            slow_check_notifier,
-        bool proceed,
-        bool showed_interstitial);
-
-    // Called by |url_checker| with the final result of performing a url check.
-    // |url_checker| must be non-null. This is an implementation of
-    // SafeBrowsingUrlCheckerImpl::NativeUrlCheckNotifier.
-    void OnCheckComplete(safe_browsing::SafeBrowsingUrlCheckerImpl* url_checker,
-                         bool proceed,
-                         bool showed_interstitial);
-
-    // This maps SafeBrowsingUrlCheckerImpls that have started but not completed
-    // a url check to the callback that should be invoked once the url check is
-    // complete.
-    base::flat_map<
-        std::unique_ptr<safe_browsing::SafeBrowsingUrlCheckerImpl>,
-        base::OnceCallback<void(web::WebStatePolicyDecider::PolicyDecision)>,
-        base::UniquePtrComparator>
-        active_url_checkers_;
-  };
-
   // A WebStatePolicyDecider that queries the SafeBrowsing database on each
   // request, always allows the request, but uses the result of the
   // SafeBrowsing check to determine whether to allow the corresponding
@@ -99,11 +47,18 @@ class SafeBrowsingTabHelper
   class PolicyDecider : public web::WebStatePolicyDecider,
                         public base::SupportsWeakPtr<PolicyDecider> {
    public:
-    PolicyDecider(web::WebState* web_state,
-                  UrlCheckerClient* url_checker_client);
-
+    PolicyDecider(web::WebState* web_state);
     ~PolicyDecider() override;
 
+    // Returns whether |query| is still relevant.  May return false if
+    // navigations occurred before the URL check has finished.
+    bool IsQueryStale(const SafeBrowsingQueryManager::Query& query);
+
+    // Stores |policy_decision| for |query|.  |query| must not be stale.
+    void HandlePolicyDecision(
+        const SafeBrowsingQueryManager::Query& query,
+        const web::WebStatePolicyDecider::PolicyDecision& policy_decision);
+
     // Notifies the policy decider that a new main frame document has been
     // loaded.
     void UpdateForMainFrameDocumentChange();
@@ -161,10 +116,8 @@ class SafeBrowsingTabHelper
         const GURL& url,
         web::WebStatePolicyDecider::PolicyDecisionCallback callback);
 
-    // Returns the appropriate UrlCheck callback for |request_info|.
-    web::WebStatePolicyDecider::PolicyDecisionCallback GetUrlCheckCallback(
-        const GURL& url,
-        const web::WebStatePolicyDecider::RequestInfo& request_info);
+    // Returns the pending main frame query for |url|.
+    MainFrameUrlQuery* GetPendingMainFrameQuery(const GURL& url);
 
     // Callback invoked when a main frame query for |url| has finished with
     // |decision|.
@@ -176,7 +129,6 @@ class SafeBrowsingTabHelper
     // |navigation_item_id| has finished with |decision|.
     void OnSubFrameUrlQueryDecided(
         const GURL& url,
-        int navigation_item_id,
         web::WebStatePolicyDecider::PolicyDecision decision);
 
     // Returns the policy decision determined by the results of queries for URLs
@@ -190,8 +142,8 @@ class SafeBrowsingTabHelper
     base::Optional<web::WebStatePolicyDecider::PolicyDecision>
     MainFrameRedirectChainDecision();
 
-    // The URL checker client used to check navigation safety on the IO thread.
-    UrlCheckerClient* url_checker_client_;
+    // The URL check query manager.
+    SafeBrowsingQueryManager* query_manager_;
     // The pending query for the main frame navigation, if any.
     base::Optional<MainFrameUrlQuery> pending_main_frame_query_;
     // The previous query for main frame, navigation, if any. This is tracked
@@ -207,6 +159,27 @@ class SafeBrowsingTabHelper
     std::map<const GURL, SubFrameUrlQuery> pending_sub_frame_queries_;
   };
 
+  // Helper object that observes results of URL check queries.
+  class QueryObserver : public SafeBrowsingQueryManager::Observer {
+   public:
+    QueryObserver(web::WebState* web_state, PolicyDecider* decider);
+    ~QueryObserver() override;
+
+   private:
+    // SafeBrowsingQueryManager::Observer:
+    void SafeBrowsingQueryFinished(
+        SafeBrowsingQueryManager* manager,
+        const SafeBrowsingQueryManager::Query& query,
+        const SafeBrowsingQueryManager::Result& result) override;
+    void SafeBrowsingQueryManagerDestroyed(
+        SafeBrowsingQueryManager* manager) override;
+
+    web::WebState* web_state_ = nullptr;
+    PolicyDecider* policy_decider_ = nullptr;
+    ScopedObserver<SafeBrowsingQueryManager, SafeBrowsingQueryManager::Observer>
+        scoped_observer_{this};
+  };
+
   // Helper object that resets state for the policy decider when a navigation is
   // finished, and notifies the policy decider about navigation redirects so
   // that the decider can associate queries that are part of a redirection
@@ -232,8 +205,8 @@ class SafeBrowsingTabHelper
 
   explicit SafeBrowsingTabHelper(web::WebState* web_state);
 
-  std::unique_ptr<UrlCheckerClient> url_checker_client_;
   PolicyDecider policy_decider_;
+  QueryObserver query_observer_;
   NavigationObserver navigation_observer_;
 
   WEB_STATE_USER_DATA_KEY_DECL();

ios/chrome/browser/safe_browsing/safe_browsing_unsafe_resource_container.h

@@ -30,8 +30,11 @@ class SafeBrowsingUnsafeResourceContainer
 
   // Stores a copy of |resource| in the container.  An allow list decision must
   // be pending for |resource| before it is added to the container.
-  void StoreUnsafeResource(
+  void StoreMainFrameUnsafeResource(
       const security_interstitials::UnsafeResource& resource);
+  void StoreSubFrameUnsafeResource(
+      const security_interstitials::UnsafeResource& resource,
+      web::NavigationItem* main_frame_item);
 
   // Returns the pending main frame UnsafeResource, or null if one has not been
   // stored.

ios/chrome/browser/safe_browsing/safe_browsing_unsafe_resource_container.mm

@@ -86,25 +86,32 @@ SafeBrowsingUnsafeResourceContainer::operator=(
 SafeBrowsingUnsafeResourceContainer::~SafeBrowsingUnsafeResourceContainer() =
     default;
 
-void SafeBrowsingUnsafeResourceContainer::StoreUnsafeResource(
-    const UnsafeResource& resource) {
+void SafeBrowsingUnsafeResourceContainer::StoreMainFrameUnsafeResource(
+    const security_interstitials::UnsafeResource& resource) {
   DCHECK(!resource.web_state_getter.is_null() &&
          resource.web_state_getter.Run() == web_state_);
-  if (resource.resource_type == safe_browsing::ResourceType::kMainFrame) {
-    // For main frame navigations, the copy is stored in
-    // |main_frame_unsafe_resource_|.  It corresponds with the pending
-    // NavigationItem, which may have not been created yet and will be discarded
-    // after navigation failures.
-    main_frame_unsafe_resource_ = PendingUnsafeResourceStorage(resource);
-  } else {
-    // Unsafe sub frame resources are caused by loads triggered by the committed
-    // main frame navigation.  These are associated with the NavigationItem so
-    // that they persist past reloads.
-    web::NavigationItem* item =
-        web_state_->GetNavigationManager()->GetLastCommittedItem();
-    UnsafeSubresourceContainer::FromNavigationItem(item)->StoreUnsafeResource(
-        resource);
-  }
+  DCHECK_EQ(safe_browsing::ResourceType::kMainFrame, resource.resource_type);
+
+  // For main frame navigations, the copy is stored in
+  // |main_frame_unsafe_resource_|.  It corresponds with the pending
+  // NavigationItem, which may have not been created yet and will be discarded
+  // after navigation failures.
+  main_frame_unsafe_resource_ = PendingUnsafeResourceStorage(resource);
+}
+
+void SafeBrowsingUnsafeResourceContainer::StoreSubFrameUnsafeResource(
+    const security_interstitials::UnsafeResource& resource,
+    web::NavigationItem* main_frame_item) {
+  DCHECK(!resource.web_state_getter.is_null() &&
+         resource.web_state_getter.Run() == web_state_);
+  DCHECK_EQ(safe_browsing::ResourceType::kSubFrame, resource.resource_type);
+  DCHECK(main_frame_item);
+
+  // Unsafe sub frame resources are caused by loads triggered by a committed
+  // main frame navigation.  These are associated with the NavigationItem so
+  // that they persist past reloads.
+  UnsafeSubresourceContainer::FromNavigationItem(main_frame_item)
+      ->StoreUnsafeResource(resource);
 }
 
 const security_interstitials::UnsafeResource*

ios/chrome/browser/safe_browsing/safe_browsing_unsafe_resource_container_unittest.mm

@@ -33,6 +33,7 @@ class SafeBrowsingUnsafeResourceContainerTest : public PlatformTest {
   UnsafeResource MakePendingUnsafeResource(bool is_main_frame) {
     UnsafeResource resource;
     resource.url = GURL("http://www.chromium.test");
+    resource.navigation_url = resource.url;
     resource.threat_type = safe_browsing::SB_THREAT_TYPE_URL_PHISHING;
     resource.callback =
         base::BindRepeating([](bool proceed, bool showed_interstitial) {});
@@ -66,7 +67,7 @@ TEST_F(SafeBrowsingUnsafeResourceContainerTest, MainFrameResource) {
   EXPECT_FALSE(container()->GetMainFrameUnsafeResource());
 
   // Store |resource| in the container.
-  container()->StoreUnsafeResource(resource);
+  container()->StoreMainFrameUnsafeResource(resource);
   const UnsafeResource* resource_copy =
       container()->GetMainFrameUnsafeResource();
   ASSERT_TRUE(resource_copy);
@@ -87,7 +88,7 @@ TEST_F(SafeBrowsingUnsafeResourceContainerTest, SubFrameResource) {
   EXPECT_FALSE(container()->GetSubFrameUnsafeResource(item_.get()));
 
   // Store |resource| in the container.
-  container()->StoreUnsafeResource(resource);
+  container()->StoreSubFrameUnsafeResource(resource, item_.get());
   const UnsafeResource* resource_copy =
       container()->GetSubFrameUnsafeResource(item_.get());
   ASSERT_TRUE(resource_copy);

ios/chrome/browser/safe_browsing/url_checker_delegate_impl.mm

@@ -12,7 +12,7 @@
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #import "ios/chrome/browser/prerender/prerender_service.h"
 #import "ios/chrome/browser/prerender/prerender_service_factory.h"
-#import "ios/chrome/browser/safe_browsing/safe_browsing_unsafe_resource_container.h"
+#import "ios/chrome/browser/safe_browsing/safe_browsing_query_manager.h"
 #import "ios/chrome/browser/safe_browsing/unsafe_resource_util.h"
 #include "ios/web/public/thread/web_task_traits.h"
 #import "ios/web/public/thread/web_thread.h"
@@ -66,13 +66,9 @@ void HandleBlockingPageRequestOnUIThread(
     }
   }
 
-  // Record the pending unsafe navigation decision.
-  allow_list->AddPendingUnsafeNavigationDecision(url, threat_type);
-
-  // Add a copy of the unsafe resource to the WebState's stack.  This will be
-  // popped later to populate the error page.
-  SafeBrowsingUnsafeResourceContainer::FromWebState(web_state)
-      ->StoreUnsafeResource(resource);
+  // Store the unsafe resource in the query manager.
+  SafeBrowsingQueryManager::FromWebState(web_state)->StoreUnsafeResource(
+      resource);
 
   // Send the do-not-proceed signal to cancel the navigation.  This will cause
   // the error page to be displayed using the stored UnsafeResource copy.

ios/chrome/browser/safe_browsing/url_checker_delegate_impl_unittest.mm

@@ -15,6 +15,7 @@
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
 #import "ios/chrome/browser/prerender/fake_prerender_service.h"
 #import "ios/chrome/browser/prerender/prerender_service_factory.h"
+#import "ios/chrome/browser/safe_browsing/safe_browsing_query_manager.h"
 #import "ios/chrome/browser/safe_browsing/safe_browsing_unsafe_resource_container.h"
 #import "ios/web/public/navigation/navigation_item.h"
 #import "ios/web/public/test/fakes/test_navigation_manager.h"
@@ -65,6 +66,7 @@ class UrlCheckerDelegateImplTest : public PlatformTest {
     // Construct the allow list and unsafe resource container.
     SafeBrowsingUnsafeResourceContainer::CreateForWebState(web_state_.get());
     SafeBrowsingUrlAllowList::CreateForWebState(web_state_.get());
+    SafeBrowsingQueryManager::CreateForWebState(web_state_.get());
     // Set up the test prerender service factory.
     PrerenderServiceFactory::GetInstance()->SetTestingFactory(
         browser_state_.get(),
@@ -198,97 +200,3 @@ TEST_F(UrlCheckerDelegateImplTest, ProceedForAllowedUnsafeNavigation) {
   EXPECT_TRUE(callback_state.proceed);
   EXPECT_FALSE(callback_state.show_interstitial);
 }
-
-// Tests that the delegate registers a pending decision, stores the unsafe
-// resource and does not allow unsafe resources to proceed for disallowed
-// navigations on the main frame.
-TEST_F(UrlCheckerDelegateImplTest,
-       DontProceedAndSetUpInterstitialStateForMainFrame) {
-  // Construct the UnsafeResource.
-  safe_browsing::SBThreatType threat_type =
-      safe_browsing::SB_THREAT_TYPE_URL_PHISHING;
-  UnsafeResourceCallbackState callback_state;
-  UnsafeResource resource = CreateUnsafeResource(&callback_state);
-  resource.is_subresource = false;
-  resource.is_subframe = false;
-  resource.resource_type = safe_browsing::ResourceType::kMainFrame;
-  resource.threat_type = threat_type;
-
-  // Instruct the delegate to display the blocking page.
-  delegate_->StartDisplayingBlockingPageHelper(resource, /*method=*/"",
-                                               net::HttpRequestHeaders(),
-                                               /*is_main_frame*/ true,
-                                               /*has_user_gesture=*/true);
-  EXPECT_TRUE(WaitForUnsafeResourceCallbackExecution(&callback_state));
-
-  // Verify the callback state.
-  EXPECT_FALSE(callback_state.proceed);
-  EXPECT_TRUE(callback_state.show_interstitial);
-
-  // Verify that the pending decision is recorded
-  std::set<safe_browsing::SBThreatType> pending_threats;
-  EXPECT_TRUE(allow_list()->IsUnsafeNavigationDecisionPending(
-      resource.url, &pending_threats));
-  EXPECT_EQ(1U, pending_threats.size());
-  EXPECT_FALSE(pending_threats.find(threat_type) == pending_threats.end());
-
-  // Verify that a copy of |resource| without its callback has been added to the
-  // container.
-  EXPECT_TRUE(container()->GetMainFrameUnsafeResource());
-  const UnsafeResource* resource_copy =
-      container()->GetMainFrameUnsafeResource();
-  ASSERT_TRUE(resource_copy);
-  EXPECT_EQ(resource.url, resource_copy->url);
-  EXPECT_EQ(resource.callback_thread, resource_copy->callback_thread);
-  EXPECT_EQ(resource.web_state_getter, resource_copy->web_state_getter);
-  EXPECT_EQ(resource.is_subresource, resource_copy->is_subresource);
-  EXPECT_EQ(resource.is_subframe, resource_copy->is_subframe);
-  EXPECT_EQ(resource.threat_type, resource_copy->threat_type);
-}
-
-// Tests that the delegate registers a pending decision, stores the unsafe
-// resource and does not allow unsafe resources to proceed for disallowed
-// navigations on a sub frame.
-TEST_F(UrlCheckerDelegateImplTest,
-       DontProceedAndSetUpInterstitialStateForSubFrame) {
-  // Construct the UnsafeResource.
-  safe_browsing::SBThreatType threat_type =
-      safe_browsing::SB_THREAT_TYPE_URL_PHISHING;
-  UnsafeResourceCallbackState callback_state;
-  UnsafeResource resource = CreateUnsafeResource(&callback_state);
-  resource.is_subresource = true;
-  resource.is_subframe = true;
-  resource.resource_type = safe_browsing::ResourceType::kSubFrame;
-  resource.threat_type = threat_type;
-
-  // Instruct the delegate to display the blocking page.
-  delegate_->StartDisplayingBlockingPageHelper(resource, /*method=*/"",
-                                               net::HttpRequestHeaders(),
-                                               /*is_main_frame*/ false,
-                                               /*has_user_gesture=*/true);
-  EXPECT_TRUE(WaitForUnsafeResourceCallbackExecution(&callback_state));
-
-  // Verify the callback state.
-  EXPECT_FALSE(callback_state.proceed);
-  EXPECT_TRUE(callback_state.show_interstitial);
-
-  // Verify that the pending decision is recorded
-  std::set<safe_browsing::SBThreatType> pending_threats;
-  EXPECT_TRUE(allow_list()->IsUnsafeNavigationDecisionPending(
-      resource.url, &pending_threats));
-  EXPECT_EQ(1U, pending_threats.size());
-  EXPECT_FALSE(pending_threats.find(threat_type) == pending_threats.end());
-
-  // Verify that a copy of |resource| without its callback has been added to the
-  // container.
-  EXPECT_TRUE(container()->GetSubFrameUnsafeResource(item_.get()));
-  const UnsafeResource* resource_copy =
-      container()->GetSubFrameUnsafeResource(item_.get());
-  ASSERT_TRUE(resource_copy);
-  EXPECT_EQ(resource.url, resource_copy->url);
-  EXPECT_EQ(resource.callback_thread, resource_copy->callback_thread);
-  EXPECT_EQ(resource.web_state_getter, resource_copy->web_state_getter);
-  EXPECT_EQ(resource.is_subresource, resource_copy->is_subresource);
-  EXPECT_EQ(resource.is_subframe, resource_copy->is_subframe);
-  EXPECT_EQ(resource.threat_type, resource_copy->threat_type);
-}

ios/chrome/browser/tabs/tab_helper_util.mm

@@ -53,6 +53,7 @@
 #import "ios/chrome/browser/policy_url_blocking/policy_url_blocking_tab_helper.h"
 #include "ios/chrome/browser/reading_list/reading_list_model_factory.h"
 #import "ios/chrome/browser/reading_list/reading_list_web_state_observer.h"
+#import "ios/chrome/browser/safe_browsing/safe_browsing_query_manager.h"
 #import "ios/chrome/browser/safe_browsing/safe_browsing_tab_helper.h"
 #import "ios/chrome/browser/safe_browsing/safe_browsing_unsafe_resource_container.h"
 #import "ios/chrome/browser/search_engines/search_engine_tab_helper.h"
@@ -140,6 +141,7 @@ void AttachTabHelpers(web::WebState* web_state, bool for_prerender) {
 
   if (base::FeatureList::IsEnabled(
           safe_browsing::kSafeBrowsingAvailableOnIOS)) {
+    SafeBrowsingQueryManager::CreateForWebState(web_state);
     SafeBrowsingTabHelper::CreateForWebState(web_state);
     SafeBrowsingUrlAllowList::CreateForWebState(web_state);
     SafeBrowsingUnsafeResourceContainer::CreateForWebState(web_state);

ios/chrome/browser/web/chrome_web_client_unittest.mm

@@ -351,7 +351,7 @@ TEST_F(ChromeWebClientTest, PrepareErrorPageForSafeBrowsingError) {
   SafeBrowsingUrlAllowList::FromWebState(&web_state)
       ->AddPendingUnsafeNavigationDecision(resource.url, resource.threat_type);
   SafeBrowsingUnsafeResourceContainer::FromWebState(&web_state)
-      ->StoreUnsafeResource(resource);
+      ->StoreMainFrameUnsafeResource(resource);
 
   NSError* error = [NSError errorWithDomain:kSafeBrowsingErrorDomain
                                        code:kUnsafeResourceErrorCode