[subresource_filter] SB throttle can send multiple speculative requests.

components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.cc

+// 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.
+
+#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/single_thread_task_runner.h"
+#include "base/timer/timer.h"
+#include "components/safe_browsing_db/util.h"
+#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace subresource_filter {
+
+constexpr base::TimeDelta
+    SubresourceFilterSafeBrowsingClientRequest::kCheckURLTimeout;
+
+SubresourceFilterSafeBrowsingClient::SubresourceFilterSafeBrowsingClient(
+    std::unique_ptr<base::Timer> timer,
+    scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> database_manager,
+    const base::WeakPtr<SubresourceFilterSafeBrowsingActivationThrottle>&
+        throttle,
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner)
+    : timer_(std::move(timer)),
+      database_manager_(std::move(database_manager)),
+      throttle_(throttle),
+      ui_task_runner_(io_task_runner) {}
+
+SubresourceFilterSafeBrowsingClient::~SubresourceFilterSafeBrowsingClient() {}
+
+void SubresourceFilterSafeBrowsingClient::CheckUrlOnIO(const GURL& url,
+                                                       int request_id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK(!url.is_empty());
+
+  // Will cancel the previous request, and it should never complete. This
+  // ensures that requests are notified in order.
+  current_request_ =

[engedy, 2017/04/20 18:56:23]

Have you checked with the Safe Browsing folks if they can handle the QPS
increase this might cause? (Or even if they have a good guess of how much of an
increase we are talking about here.)

Also, I'm wondering how much we save by `cancelling` the request. Have you
thought about just letting each of these requests run its course (only killing
them if they are still pending when the result for the final URL is back and we
are proceeding with the navigation). That would also allow us to reinstate the
redirect chain patterns that was deprecated. Probably the SB folks know more
here as well.

[engedy, 2017/04/20 18:58:17]

... redirect chain patterns *histogram* ...

+      base::MakeUnique<SubresourceFilterSafeBrowsingClientRequest>(
+          timer_.get(), url, request_id, database_manager_, this);
+  current_request_->Start();
+}
+
+void SubresourceFilterSafeBrowsingClient::OnCheckBrowseUrlResult(
+    SubresourceFilterSafeBrowsingClientRequest* request,
+    safe_browsing::SBThreatType threat_type,
+    const safe_browsing::ThreatMetadata& metadata) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK_EQ(request->request_id(), current_request_->request_id());
+  ui_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&SubresourceFilterSafeBrowsingActivationThrottle::
+                                OnCheckUrlResultOnUI,
+                            throttle_, request->url(), request->request_id(),
+                            threat_type, metadata.threat_pattern_type));
+
+  current_request_.reset();
+}
+
+SubresourceFilterSafeBrowsingClientRequest::
+    SubresourceFilterSafeBrowsingClientRequest(
+        base::Timer* timer,
+        const GURL& url,
+        int request_id,
+        scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
+            database_manager,
+        SubresourceFilterSafeBrowsingClient* client)
+    : url_(url),
+      request_id_(request_id),
+      database_manager_(std::move(database_manager)),
+      client_(client),
+      timer_(timer) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+}
+
+SubresourceFilterSafeBrowsingClientRequest::
+    ~SubresourceFilterSafeBrowsingClientRequest() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  database_manager_->CancelCheck(this);
+  timer_->Stop();
+}
+
+void SubresourceFilterSafeBrowsingClientRequest::Start() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  if (database_manager_->CheckUrlForSubresourceFilter(url_, this)) {
+    OnCheckBrowseUrlResult(url_, safe_browsing::SB_THREAT_TYPE_SAFE,
+                           safe_browsing::ThreatMetadata());
+    return;
+  }
+  timer_->Start(
+      FROM_HERE, kCheckURLTimeout,
+      base::Bind(&SubresourceFilterSafeBrowsingClientRequest::OnCheckUrlTimeout,
+                 base::Unretained(this)));
+}
+
+void SubresourceFilterSafeBrowsingClientRequest::OnCheckBrowseUrlResult(
+    const GURL& url,
+    safe_browsing::SBThreatType threat_type,
+    const safe_browsing::ThreatMetadata& metadata) {
+  DCHECK_EQ(url_, url);
+  DCHECK(!timer_->IsRunning());
+  client_->OnCheckBrowseUrlResult(this, threat_type, metadata);
+}
+
+void SubresourceFilterSafeBrowsingClientRequest::OnCheckUrlTimeout() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  database_manager_->CancelCheck(this);
+  OnCheckBrowseUrlResult(url_, safe_browsing::SB_THREAT_TYPE_SAFE,
+                         safe_browsing::ThreatMetadata());
+}
+
+}  // namespace subresource_filter