[subresource_filter] SB throttle can send multiple speculative requests.

components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.h

+// Copyright 2017 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 COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_SUBRESOURCE_FILTER_SAFE_BROWSING_CLIENT_H_
+#define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_SUBRESOURCE_FILTER_SAFE_BROWSING_CLIENT_H_
+
+#include <map>
+#include <memory>
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+#include "components/safe_browsing_db/v4_local_database_manager.h"
+#include "url/gurl.h"
+
+namespace base {
+class SingleThreadTaskRunner;
+}  // namespace base
+
+namespace subresource_filter {
+
+class SubresourceFilterSafeBrowsingActivationThrottle;
+class SubresourceFilterSafeBrowsingClientRequest;
+
+// Created on the UI thread but used on the IO thread to communicate with the
+// safe browsing service.
+//
+// The class is expected to accompany a single navigation. If a check request
+// comes in for URL B while URL A is in flight, we cancel the check to URL A.

[engedy, 2017/04/25 22:03:54]

comment nit: Update this line.

[Charlie Harrison, 2017/04/26 14:30:25]

Done.

+class SubresourceFilterSafeBrowsingClient {
+ public:
+  SubresourceFilterSafeBrowsingClient(
+      scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
+          database_manager,
+      const base::WeakPtr<SubresourceFilterSafeBrowsingActivationThrottle>&

[engedy, 2017/04/25 22:03:54]

nit: I believe the latest style is to not pass smart pointers by const&. How
about taking this by value, and moving it into the member on the initializer
list?

[Charlie Harrison, 2017/04/26 14:30:25]

Done.

+          throttle,
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
+
+  ~SubresourceFilterSafeBrowsingClient();
+
+  void CheckUrlOnIO(const GURL& url, size_t request_id);

[engedy, 2017/04/25 22:03:54]

#include <stddef.h> for size_t (in all places)

[Charlie Harrison, 2017/04/26 14:30:25]

Done.

+
+  void OnCheckBrowseUrlResult(
+      SubresourceFilterSafeBrowsingClientRequest* request,
+      safe_browsing::SBThreatType threat_type,
+      const safe_browsing::ThreatMetadata& metadata);
+
+ private:
+  // This is stored as a map to allow for ergonomic deletion.
+  std::map<SubresourceFilterSafeBrowsingClientRequest*,
+           std::unique_ptr<SubresourceFilterSafeBrowsingClientRequest>>
+      requests_;
+
+  scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> database_manager_;
+
+  base::WeakPtr<SubresourceFilterSafeBrowsingActivationThrottle> throttle_;
+  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
+  scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(SubresourceFilterSafeBrowsingClient);
+};
+
+// This class is scoped to a single database check, and it lives on the IO
+// thread exclusively.
+class SubresourceFilterSafeBrowsingClientRequest

[engedy, 2017/04/25 22:03:54]

Can this be moved to the .cc file?

[Charlie Harrison, 2017/04/26 14:30:25]

Do you mind if I move it to a separate file (that's what I've done). I've been
frustrated recently untangling anonymous classes.

[engedy, 2017/04/26 14:56:39]

Fine by me!

+    : public safe_browsing::SafeBrowsingDatabaseManager::Client {
+ public:
+  SubresourceFilterSafeBrowsingClientRequest(
+      const GURL& url,
+      size_t request_id,
+      scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
+          database_manager,
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+      SubresourceFilterSafeBrowsingClient* client);
+  ~SubresourceFilterSafeBrowsingClientRequest() override;
+
+  void Start();
+
+  void OnCheckBrowseUrlResult(
+      const GURL& url,
+      safe_browsing::SBThreatType threat_type,
+      const safe_browsing::ThreatMetadata& metadata) override;
+
+  const GURL& url() const { return url_; }
+  size_t request_id() const { return request_id_; }
+
+  // Maximum time in milliseconds to wait for the Safe Browsing service to
+  // verify a URL. After this amount of time the outstanding check will be
+  // aborted, and the URL will be treated as if it didn't belong to the
+  // Subresource Filter only list.
+  static constexpr base::TimeDelta kCheckURLTimeout =
+      base::TimeDelta::FromSeconds(5);
+
+ private:
+  // Callback for when the safe browsing check has taken longer than
+  // kCheckURLTimeout.
+  void OnCheckUrlTimeout();
+
+  const GURL url_;
+  const size_t request_id_;
+
+  scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> database_manager_;
+  SubresourceFilterSafeBrowsingClient* client_ = nullptr;
+
+  // Timer to abort the safe browsing check if it takes too long.
+  base::OneShotTimer timer_;
+
+  base::TimeTicks start_time_;
+
+  DISALLOW_COPY_AND_ASSIGN(SubresourceFilterSafeBrowsingClientRequest);
+};
+
+}  // namespace subresource_filter
+
+#endif  // COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_SUBRESOURCE_FILTER_SAFE_BROWSING_CLIENT_H_