Wire DevTools instrumentation signals for browser agent through devtools_instrumentation

Bug: 899303
Change-Id: Ibefd04bab772724105d64b9d83a2fafddd1c85a5
Reviewed-on: https://chromium-review.googlesource.com/c/1305494
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603606}

content/browser/devtools/browser_devtools_agent_host.cc

@@ -8,6 +8,7 @@
 #include "base/guid.h"
 #include "base/json/json_reader.h"
 #include "base/memory/ptr_util.h"
+#include "base/no_destructor.h"
 #include "base/single_thread_task_runner.h"
 #include "content/browser/devtools/devtools_session.h"
 #include "content/browser/devtools/protocol/browser_handler.h"
@@ -36,6 +37,19 @@ scoped_refptr<DevToolsAgentHost> DevToolsAgentHost::CreateForDiscovery() {
   return new BrowserDevToolsAgentHost(nullptr, std::move(null_callback), true);
 }
 
+namespace {
+std::set<BrowserDevToolsAgentHost*>& BrowserDevToolsAgentHostInstances() {
+  static base::NoDestructor<std::set<BrowserDevToolsAgentHost*>> instances;
+  return *instances;
+}
+}  // namespace
+
+// static
+const std::set<BrowserDevToolsAgentHost*>&
+BrowserDevToolsAgentHost::Instances() {
+  return BrowserDevToolsAgentHostInstances();
+}
+
 BrowserDevToolsAgentHost::BrowserDevToolsAgentHost(
     scoped_refptr<base::SingleThreadTaskRunner> tethering_task_runner,
     const CreateServerSocketCallback& socket_callback,
@@ -45,9 +59,11 @@ BrowserDevToolsAgentHost::BrowserDevToolsAgentHost(
       socket_callback_(socket_callback),
       only_discovery_(only_discovery) {
   NotifyCreated();
+  BrowserDevToolsAgentHostInstances().insert(this);
 }
 
 BrowserDevToolsAgentHost::~BrowserDevToolsAgentHost() {
+  BrowserDevToolsAgentHostInstances().erase(this);
 }
 
 bool BrowserDevToolsAgentHost::AttachSession(DevToolsSession* session,

content/browser/devtools/browser_devtools_agent_host.h

@@ -13,6 +13,10 @@ namespace content {
 class TargetRegistry;
 
 class BrowserDevToolsAgentHost : public DevToolsAgentHostImpl {
+ public:
+  // TODO(caseq,dgozman): this should probably be a singleton.
+  static const std::set<BrowserDevToolsAgentHost*>& Instances();
+
  private:
   friend class DevToolsAgentHost;
   BrowserDevToolsAgentHost(

content/browser/devtools/devtools_agent_host_impl.cc

@@ -36,34 +36,6 @@ base::LazyInstance<DevToolsMap>::Leaky g_devtools_instances =
 
 base::LazyInstance<base::ObserverList<DevToolsAgentHostObserver>::Unchecked>::
     Leaky g_devtools_observers = LAZY_INSTANCE_INITIALIZER;
-
-// Returns a list of all active hosts on browser targets.
-DevToolsAgentHost::List GetBrowserAgentHosts() {
-  DevToolsAgentHost::List result;
-  for (const auto& id_host : g_devtools_instances.Get()) {
-    if (id_host.second->GetType() == DevToolsAgentHost::kTypeBrowser)
-      result.push_back(id_host.second);
-  }
-  return result;
-}
-
-// Notify the provided agent host of a certificate error. Returns true if one of
-// the host's handlers will handle the certificate error.
-bool NotifyCertificateError(
-    DevToolsAgentHost* host,
-    int cert_error,
-    const GURL& request_url,
-    const DevToolsAgentHostImpl::CertErrorCallback& callback) {
-  DevToolsAgentHostImpl* host_impl = static_cast<DevToolsAgentHostImpl*>(host);
-  for (auto* security_handler :
-       protocol::SecurityHandler::ForAgentHost(host_impl)) {
-    if (security_handler->NotifyCertificateError(cert_error, request_url,
-                                                 callback)) {
-      return true;
-    }
-  }
-  return false;
-}
 }  // namespace
 
 const char DevToolsAgentHost::kTypePage[] = "page";
@@ -145,31 +117,6 @@ scoped_refptr<DevToolsAgentHost> DevToolsAgentHost::Forward(
   return new ForwardingAgentHost(id, std::move(delegate));
 }
 
-// static
-bool DevToolsAgentHostImpl::HandleCertificateError(WebContents* web_contents,
-                                                   int cert_error,
-                                                   const GURL& request_url,
-                                                   CertErrorCallback callback) {
-  scoped_refptr<DevToolsAgentHost> agent_host =
-      DevToolsAgentHost::GetOrCreateFor(web_contents).get();
-  if (NotifyCertificateError(agent_host.get(), cert_error, request_url,
-                             callback)) {
-    // Only allow a single agent host to handle the error.
-    callback.Reset();
-  }
-
-  for (scoped_refptr<DevToolsAgentHost> browser_agent_host :
-       GetBrowserAgentHosts()) {
-    if (NotifyCertificateError(browser_agent_host.get(), cert_error,
-                               request_url, callback)) {
-      // Only allow a single agent host to handle the error.
-      callback.Reset();
-    }
-  }
-
-  return !callback;
-}
-
 DevToolsSession* DevToolsAgentHostImpl::SessionByClient(
     DevToolsAgentHostClient* client) {
   auto it = session_by_client_.find(client);

content/browser/devtools/devtools_agent_host_impl.h

@@ -29,15 +29,6 @@ class TargetRegistry;
 // Describes interface for managing devtools agents from the browser process.
 class CONTENT_EXPORT DevToolsAgentHostImpl : public DevToolsAgentHost {
  public:
-  // Asks any interested agents to handle the given certificate error. Returns
-  // |true| if the error was handled, |false| otherwise.
-  using CertErrorCallback =
-      base::RepeatingCallback<void(content::CertificateRequestResultType)>;
-  static bool HandleCertificateError(WebContents* web_contents,
-                                     int cert_error,
-                                     const GURL& request_url,
-                                     CertErrorCallback callback);
-
   // DevToolsAgentHost implementation.
   bool AttachClient(DevToolsAgentHostClient* client) override;
   bool DetachClient(DevToolsAgentHostClient* client) override;

content/browser/devtools/devtools_instrumentation.cc

@@ -3,9 +3,11 @@
 // found in the LICENSE file.
 #include "content/browser/devtools/devtools_instrumentation.h"
 
+#include "content/browser/devtools/browser_devtools_agent_host.h"
 #include "content/browser/devtools/protocol/emulation_handler.h"
 #include "content/browser/devtools/protocol/network_handler.h"
 #include "content/browser/devtools/protocol/page_handler.h"
+#include "content/browser/devtools/protocol/security_handler.h"
 #include "content/browser/devtools/protocol/target_handler.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #include "content/browser/frame_host/frame_tree_node.h"
@@ -218,6 +220,45 @@ void OnNavigationRequestWillBeSent(
                    navigation_request);
 }
 
+// Notify the provided agent host of a certificate error. Returns true if one of
+// the host's handlers will handle the certificate error.
+bool NotifyCertificateError(DevToolsAgentHost* host,
+                            int cert_error,
+                            const GURL& request_url,
+                            const CertErrorCallback& callback) {
+  DevToolsAgentHostImpl* host_impl = static_cast<DevToolsAgentHostImpl*>(host);
+  for (auto* security_handler :
+       protocol::SecurityHandler::ForAgentHost(host_impl)) {
+    if (security_handler->NotifyCertificateError(cert_error, request_url,
+                                                 callback)) {
+      return true;
+    }
+  }
+  return false;
+}
+
+bool HandleCertificateError(WebContents* web_contents,
+                            int cert_error,
+                            const GURL& request_url,
+                            CertErrorCallback callback) {
+  scoped_refptr<DevToolsAgentHost> agent_host =
+      DevToolsAgentHost::GetOrCreateFor(web_contents).get();
+  if (NotifyCertificateError(agent_host.get(), cert_error, request_url,
+                             callback)) {
+    // Only allow a single agent host to handle the error.
+    callback.Reset();
+  }
+
+  for (auto* browser_agent_host : BrowserDevToolsAgentHost::Instances()) {
+    if (NotifyCertificateError(browser_agent_host, cert_error, request_url,
+                               callback)) {
+      // Only allow a single agent host to handle the error.
+      callback.Reset();
+    }
+  }
+  return !callback;
+}
+
 }  // namespace devtools_instrumentation
 
 }  // namespace content
\ No newline at end of file

content/browser/devtools/devtools_instrumentation.h

@@ -13,6 +13,7 @@
 
 #include "base/optional.h"
 #include "content/common/navigation_params.mojom.h"
+#include "content/public/browser/certificate_request_result_type.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 
 class GURL;
@@ -37,6 +38,8 @@ class NavigationHandleImpl;
 class NavigationRequest;
 class NavigationThrottle;
 class RenderFrameHostImpl;
+class WebContents;
+
 struct SignedExchangeError;
 
 namespace devtools_instrumentation {
@@ -88,6 +91,15 @@ void OnSignedExchangeCertificateRequestCompleted(
 std::vector<std::unique_ptr<NavigationThrottle>> CreateNavigationThrottles(
     NavigationHandleImpl* navigation_handle);
 
+// Asks any interested agents to handle the given certificate error. Returns
+// |true| if the error was handled, |false| otherwise.
+using CertErrorCallback =
+    base::RepeatingCallback<void(content::CertificateRequestResultType)>;
+bool HandleCertificateError(WebContents* web_contents,
+                            int cert_error,
+                            const GURL& request_url,
+                            CertErrorCallback callback);
+
 }  // namespace devtools_instrumentation
 
 }  // namespace content

content/browser/ssl/ssl_manager.cc

@@ -13,7 +13,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/supports_user_data.h"
 #include "base/task/post_task.h"
-#include "content/browser/devtools/devtools_agent_host_impl.h"
+#include "content/browser/devtools/devtools_instrumentation.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_request_info_impl.h"
@@ -336,7 +336,7 @@ void SSLManager::OnCertErrorInternal(std::unique_ptr<SSLErrorHandler> handler,
       base::Bind(&OnAllowCertificate, base::Owned(handler.release()),
                  ssl_host_state_delegate_);
 
-  if (DevToolsAgentHostImpl::HandleCertificateError(
+  if (devtools_instrumentation::HandleCertificateError(
           web_contents, cert_error, request_url,
           base::BindRepeating(&OnAllowCertificateWithRecordDecision, false,
                               callback))) {