DevTools: allow auto-attaching to page targets

Target.setAutoAttach on the top level Target handler now allows
attaching to all new pages. Top level TargetHandler will attach
to each main frame as soon as it is created (right after sending
targetCreated event). All subsequent navigations will be throttled
until Runtime.runIfWaitingForDebugger is received.

Experimental parameter 'windowOpen' is removed in favour of the
new more generic functionality.

Bug: 1051687
Change-Id: Ia9b58133b97999383cd44016f8d3041f71b5d3d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2042302
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Auto-Submit: Yury Semikhatsky <yurys@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741363}

content/browser/devtools/devtools_instrumentation.cc

@@ -190,14 +190,23 @@ void OnSignedExchangeCertificateRequestCompleted(
                    protocol::Network::ResourceTypeEnum::Other, status);
 }
 
+void CreateThrottlesForAgentHost(
+    DevToolsAgentHostImpl* agent_host,
+    NavigationHandle* navigation_handle,
+    std::vector<std::unique_ptr<NavigationThrottle>>* result) {
+  for (auto* target_handler :
+       protocol::TargetHandler::ForAgentHost(agent_host)) {
+    std::unique_ptr<NavigationThrottle> throttle =
+        target_handler->CreateThrottleForNavigation(navigation_handle);
+    if (throttle)
+      result->push_back(std::move(throttle));
+  }
+}
+
 std::vector<std::unique_ptr<NavigationThrottle>> CreateNavigationThrottles(
     NavigationHandle* navigation_handle) {
-  std::vector<std::unique_ptr<NavigationThrottle>> result;
   FrameTreeNode* frame_tree_node =
       NavigationRequest::From(navigation_handle)->frame_tree_node();
-
-  DevToolsAgentHostImpl* agent_host =
-      RenderFrameDevToolsAgentHost::GetFor(frame_tree_node);
   FrameTreeNode* parent = frame_tree_node->parent();
   if (!parent) {
     if (WebContentsImpl::FromFrameTreeNode(frame_tree_node)->IsPortal() &&
@@ -207,22 +216,19 @@ std::vector<std::unique_ptr<NavigationThrottle>> CreateNavigationThrottles(
                    ->GetOuterWebContents()
                    ->GetFrameTree()
                    ->root();
-    } else {
-      parent = frame_tree_node->original_opener();
     }
   }
-  if (!parent)
-    return result;
 
-  agent_host = RenderFrameDevToolsAgentHost::GetFor(parent);
-  if (agent_host) {
-    for (auto* target_handler :
-         protocol::TargetHandler::ForAgentHost(agent_host)) {
-      std::unique_ptr<NavigationThrottle> throttle =
-          target_handler->CreateThrottleForNavigation(navigation_handle);
-      if (throttle)
-        result.push_back(std::move(throttle));
-    }
+  std::vector<std::unique_ptr<NavigationThrottle>> result;
+  if (parent) {
+    DevToolsAgentHostImpl* agent_host =
+        RenderFrameDevToolsAgentHost::GetFor(parent);
+    if (agent_host)
+      CreateThrottlesForAgentHost(agent_host, navigation_handle, &result);
+  } else {
+    for (auto* browser_agent_host : BrowserDevToolsAgentHost::Instances())
+      CreateThrottlesForAgentHost(browser_agent_host, navigation_handle,
+                                  &result);
   }
 
   return result;

content/browser/devtools/devtools_session.cc

@@ -92,6 +92,10 @@ void DevToolsSession::SetRuntimeResumeCallback(
   runtime_resume_ = std::move(runtime_resume);
 }
 
+bool DevToolsSession::IsWaitingForDebuggerOnStart() const {
+  return !runtime_resume_.is_null();
+}
+
 void DevToolsSession::Dispose() {
   dispatcher_.reset();
   for (auto& pair : handlers_)

content/browser/devtools/devtools_session.h

@@ -44,6 +44,7 @@ class DevToolsSession : public protocol::FrontendChannel,
 
   void SetAgentHost(DevToolsAgentHostImpl* agent_host);
   void SetRuntimeResumeCallback(base::OnceClosure runtime_resume);
+  bool IsWaitingForDebuggerOnStart() const;
   void Dispose();
 
   // content::DevToolsAgentHostClientChannel implementation.

content/browser/devtools/protocol/target_auto_attacher.cc

@@ -119,11 +119,9 @@ base::flat_set<GURL> GetFrameUrls(RenderFrameHostImpl* render_frame_host) {
 }  // namespace
 
 TargetAutoAttacher::TargetAutoAttacher(
-    AttachCallback attach_callback,
-    DetachCallback detach_callback,
+    Delegate* delegate,
     DevToolsRendererChannel* renderer_channel)
-    : attach_callback_(attach_callback),
-      detach_callback_(detach_callback),
+    : delegate_(delegate),
       renderer_channel_(renderer_channel),
       render_frame_host_(nullptr),
       auto_attach_(false),
@@ -206,6 +204,13 @@ bool TargetAutoAttacher::ShouldThrottleFramesNavigation() {
   return auto_attach_;
 }
 
+void TargetAutoAttacher::AttachToAgentHost(DevToolsAgentHost* host) {
+  scoped_refptr<DevToolsAgentHost> agent_host(host);
+  DCHECK(auto_attached_hosts_.find(agent_host) == auto_attached_hosts_.end());
+  delegate_->AutoAttach(agent_host.get(), wait_for_debugger_on_start_);
+  auto_attached_hosts_.insert(agent_host);
+}
+
 DevToolsAgentHost* TargetAutoAttacher::AutoAttachToFrame(
     NavigationRequest* navigation_request) {
   if (!ShouldThrottleFramesNavigation())
@@ -223,20 +228,6 @@ DevToolsAgentHost* TargetAutoAttacher::AutoAttachToFrame(
   scoped_refptr<DevToolsAgentHost> agent_host =
       RenderFrameDevToolsAgentHost::FindForDangling(frame_tree_node);
 
-  // Process the window.open auto-attaches for new targets.
-  if (frame_tree_node->original_opener()) {
-    if (!agent_host) {
-      agent_host =
-          RenderFrameDevToolsAgentHost::CreateForCrossProcessNavigation(
-              navigation_request);
-    }
-    if (auto_attached_hosts_.find(agent_host) != auto_attached_hosts_.end())
-      return nullptr;
-    attach_callback_.Run(agent_host.get(), wait_for_debugger_on_start_);
-    auto_attached_hosts_.insert(agent_host);
-    return wait_for_debugger_on_start_ ? agent_host.get() : nullptr;
-  }
-
   bool old_cross_process = !!agent_host;
   bool is_portal_main_frame =
       frame_tree_node->IsMainFrame() &&
@@ -251,8 +242,7 @@ DevToolsAgentHost* TargetAutoAttacher::AutoAttachToFrame(
   if (new_cross_process) {
     agent_host = RenderFrameDevToolsAgentHost::CreateForCrossProcessNavigation(
         navigation_request);
-    attach_callback_.Run(agent_host.get(), wait_for_debugger_on_start_);
-    auto_attached_hosts_.insert(agent_host);
+    AttachToAgentHost(agent_host.get());
     return wait_for_debugger_on_start_ ? agent_host.get() : nullptr;
   }
 
@@ -263,7 +253,7 @@ DevToolsAgentHost* TargetAutoAttacher::AutoAttachToFrame(
   if (it == auto_attached_hosts_.end())
     return nullptr;
   auto_attached_hosts_.erase(it);
-  detach_callback_.Run(agent_host.get());
+  delegate_->AutoDetach(agent_host.get());
   return nullptr;
 }
 
@@ -291,12 +281,12 @@ void TargetAutoAttacher::ReattachTargetsOfType(const Hosts& new_hosts,
   for (auto& host : old_hosts) {
     if (host->GetType() == type && new_hosts.find(host) == new_hosts.end()) {
       auto_attached_hosts_.erase(host);
-      detach_callback_.Run(host.get());
+      delegate_->AutoDetach(host.get());
     }
   }
   for (auto& host : new_hosts) {
     if (old_hosts.find(host) == old_hosts.end()) {
-      attach_callback_.Run(host.get(), waiting_for_debugger);
+      delegate_->AutoAttach(host.get(), waiting_for_debugger);
       auto_attached_hosts_.insert(host);
     }
   }
@@ -371,7 +361,7 @@ void TargetAutoAttacher::WorkerDestroyed(ServiceWorkerDevToolsAgentHost* host) {
 
 void TargetAutoAttacher::ChildWorkerCreated(DevToolsAgentHostImpl* agent_host,
                                             bool waiting_for_debugger) {
-  attach_callback_.Run(agent_host, waiting_for_debugger);
+  delegate_->AutoAttach(agent_host, waiting_for_debugger);
   auto_attached_hosts_.insert(scoped_refptr<DevToolsAgentHost>(agent_host));
 }
 

content/browser/devtools/protocol/target_auto_attacher.h

@@ -20,13 +20,17 @@ namespace protocol {
 
 class TargetAutoAttacher : public ServiceWorkerDevToolsManager::Observer {
  public:
-  // Second parameter is |waiting_for_debugger|, returns whether it succeeded.
-  using AttachCallback =
-      base::RepeatingCallback<void(DevToolsAgentHost*, bool)>;
-  using DetachCallback = base::RepeatingCallback<void(DevToolsAgentHost*)>;
+  class Delegate {
+   public:
+    virtual void AutoAttach(DevToolsAgentHost* host,
+                            bool waiting_for_debugger) = 0;
+    virtual void AutoDetach(DevToolsAgentHost* host) = 0;
+
+   protected:
+    virtual ~Delegate() = default;
+  };
 
-  TargetAutoAttacher(AttachCallback attach_callback,
-                     DetachCallback detach_callback,
+  TargetAutoAttacher(Delegate* delegate,
                      DevToolsRendererChannel* renderer_channel);
   ~TargetAutoAttacher() override;
 
@@ -40,6 +44,7 @@ class TargetAutoAttacher : public ServiceWorkerDevToolsManager::Observer {
   void AgentHostClosed(DevToolsAgentHost* host);
 
   bool ShouldThrottleFramesNavigation();
+  void AttachToAgentHost(DevToolsAgentHost* host);
   DevToolsAgentHost* AutoAttachToFrame(NavigationRequest* navigation_request);
   void ChildWorkerCreated(DevToolsAgentHostImpl* agent_host,
                           bool waiting_for_debugger);
@@ -61,8 +66,7 @@ class TargetAutoAttacher : public ServiceWorkerDevToolsManager::Observer {
 
   void UpdateFrames();
 
-  AttachCallback attach_callback_;
-  DetachCallback detach_callback_;
+  Delegate* delegate_;
   DevToolsRendererChannel* renderer_channel_;
   RenderFrameHostImpl* render_frame_host_;
 

content/browser/devtools/protocol/target_handler.cc

@@ -17,7 +17,9 @@
 #include "content/browser/devtools/browser_devtools_agent_host.h"
 #include "content/browser/devtools/devtools_agent_host_impl.h"
 #include "content/browser/devtools/devtools_manager.h"
+#include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #include "content/browser/frame_host/navigation_request.h"
+#include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/devtools_agent_host_client.h"
 #include "content/public/browser/navigation_throttle.h"
@@ -258,16 +260,35 @@ class TargetHandler::Throttle : public content::NavigationThrottle {
   NavigationThrottle::ThrottleCheckResult WillFailRequest() override;
   const char* GetNameForLogging() override;
 
+ protected:
+  void SetThrottledAgentHost(DevToolsAgentHost* agent_host);
+
  private:
-  NavigationThrottle::ThrottleCheckResult MaybeAttach();
+  virtual void ComputeThrottledAgentHost();
   void CleanupPointers();
 
   base::WeakPtr<protocol::TargetHandler> target_handler_;
   scoped_refptr<DevToolsAgentHost> agent_host_;
+  bool is_deferring_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(Throttle);
 };
 
+class TargetHandler::MainFrameThrottle : public TargetHandler::Throttle {
+ public:
+  MainFrameThrottle(base::WeakPtr<protocol::TargetHandler> target_handler,
+                    content::NavigationHandle* navigation_handle,
+                    DevToolsAgentHost* throttled_agent_host)
+      : Throttle(target_handler, navigation_handle) {
+    SetThrottledAgentHost(throttled_agent_host);
+  }
+  ~MainFrameThrottle() override = default;
+
+ private:
+  // For top frames agent host is always set in the constructor.
+  void ComputeThrottledAgentHost() override {}
+};
+
 class TargetHandler::Session : public DevToolsAgentHostClient {
  public:
   static std::string Attach(TargetHandler* handler,
@@ -288,9 +309,10 @@ class TargetHandler::Session : public DevToolsAgentHostClient {
       DevToolsSession* devtools_session =
           handler->root_session_->AttachChildSession(id, agent_host_impl,
                                                      session);
-      if (devtools_session) {
+      if (waiting_for_debugger && devtools_session) {
         devtools_session->SetRuntimeResumeCallback(base::BindOnce(
             &Session::ResumeIfThrottled, base::Unretained(session)));
+        session->devtools_session_ = devtools_session;
       }
     } else {
       agent_host_impl->AttachClient(session);
@@ -317,10 +339,16 @@ class TargetHandler::Session : public DevToolsAgentHostClient {
     else
       agent_host_->DetachClient(this);
     handler_->auto_attached_sessions_.erase(agent_host_.get());
+    devtools_session_ = nullptr;
     agent_host_ = nullptr;
     handler_->attached_sessions_.erase(id_);
   }
 
+  bool IsWaitingForDebuggerOnStart() const {
+    return devtools_session_ &&
+           devtools_session_->IsWaitingForDebuggerOnStart();
+  }
+
   void SetThrottle(Throttle* throttle) { throttle_ = throttle; }
 
   void ResumeIfThrottled() {
@@ -401,6 +429,7 @@ class TargetHandler::Session : public DevToolsAgentHostClient {
   scoped_refptr<DevToolsAgentHost> agent_host_;
   std::string id_;
   bool flatten_protocol_;
+  DevToolsSession* devtools_session_ = nullptr;
   Throttle* throttle_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(Session);
@@ -432,24 +461,33 @@ void TargetHandler::Throttle::CleanupPointers() {
 
 NavigationThrottle::ThrottleCheckResult
 TargetHandler::Throttle::WillProcessResponse() {
-  return MaybeAttach();
+  ComputeThrottledAgentHost();
+  is_deferring_ = !!agent_host_;
+  return is_deferring_ ? DEFER : PROCEED;
 }
 
 NavigationThrottle::ThrottleCheckResult
 TargetHandler::Throttle::WillFailRequest() {
-  return MaybeAttach();
+  ComputeThrottledAgentHost();
+  is_deferring_ = !!agent_host_;
+  return is_deferring_ ? DEFER : PROCEED;
 }
 
-NavigationThrottle::ThrottleCheckResult TargetHandler::Throttle::MaybeAttach() {
-  if (!target_handler_)
-    return PROCEED;
-  agent_host_ = target_handler_->auto_attacher_.AutoAttachToFrame(
-      NavigationRequest::From(navigation_handle()));
-  if (!agent_host_.get())
-    return PROCEED;
+void TargetHandler::Throttle::SetThrottledAgentHost(
+    DevToolsAgentHost* agent_host) {
+  agent_host_ = agent_host;
+  if (agent_host_) {
     target_handler_->auto_attached_sessions_[agent_host_.get()]->SetThrottle(
         this);
-  return DEFER;
+  }
+}
+
+void TargetHandler::Throttle::ComputeThrottledAgentHost() {
+  if (!target_handler_)
+    return;
+  NavigationRequest* request = NavigationRequest::From(navigation_handle());
+  SetThrottledAgentHost(
+      target_handler_->auto_attacher_.AutoAttachToFrame(request));
 }
 
 const char* TargetHandler::Throttle::GetNameForLogging() {
@@ -458,10 +496,10 @@ const char* TargetHandler::Throttle::GetNameForLogging() {
 
 void TargetHandler::Throttle::Clear() {
   CleanupPointers();
-  if (agent_host_) {
   agent_host_ = nullptr;
+  if (is_deferring_)
     Resume();
-  }
+  is_deferring_ = false;
 }
 
 TargetHandler::TargetHandler(AccessMode access_mode,
@@ -469,11 +507,7 @@ TargetHandler::TargetHandler(AccessMode access_mode,
                              DevToolsRendererChannel* renderer_channel,
                              DevToolsSession* root_session)
     : DevToolsDomainHandler(Target::Metainfo::domainName),
-      auto_attacher_(base::BindRepeating(&TargetHandler::AutoAttach,
-                                         base::Unretained(this)),
-                     base::BindRepeating(&TargetHandler::AutoDetach,
-                                         base::Unretained(this)),
-                     renderer_channel),
+      auto_attacher_(this, renderer_channel),
       discover_(false),
       access_mode_(access_mode),
       owner_target_id_(owner_target_id),
@@ -499,7 +533,7 @@ void TargetHandler::SetRenderer(int process_host_id,
 }
 
 Response TargetHandler::Disable() {
-  SetAutoAttachInternal(false, false, false, false, base::DoNothing());
+  SetAutoAttachInternal(false, false, false, base::DoNothing());
   SetDiscoverTargets(false);
   auto_attached_sessions_.clear();
   attached_sessions_.clear();
@@ -530,11 +564,25 @@ std::unique_ptr<NavigationThrottle> TargetHandler::CreateThrottleForNavigation(
     NavigationHandle* navigation_handle) {
   if (!auto_attacher_.ShouldThrottleFramesNavigation())
     return nullptr;
+  if (access_mode_ == AccessMode::kBrowser) {
     FrameTreeNode* frame_tree_node =
         NavigationRequest::From(navigation_handle)->frame_tree_node();
-  bool is_window_open = frame_tree_node->original_opener();
-  if (is_window_open && !attach_to_window_open_)
+    // Top-level target handler is expected to create throttles only for new
+    // pages.
+    DCHECK(!frame_tree_node->parent());
+    DevToolsAgentHost* host =
+        RenderFrameDevToolsAgentHost::GetFor(frame_tree_node);
+    // For new pages create Throttle only if the session is still paused.
+    if (!host)
+      return nullptr;
+    auto it = auto_attached_sessions_.find(host);
+    if (it == auto_attached_sessions_.end())
+      return nullptr;
+    if (!it->second->IsWaitingForDebuggerOnStart())
       return nullptr;
+    return std::make_unique<MainFrameThrottle>(weak_factory_.GetWeakPtr(),
+                                               navigation_handle, host);
+  }
   return std::make_unique<Throttle>(weak_factory_.GetWeakPtr(),
                                     navigation_handle);
 }
@@ -553,14 +601,27 @@ void TargetHandler::ClearThrottles() {
 void TargetHandler::SetAutoAttachInternal(bool auto_attach,
                                           bool wait_for_debugger_on_start,
                                           bool flatten,
-                                          bool window_open,
                                           base::OnceClosure callback) {
   flatten_auto_attach_ = flatten;
-  attach_to_window_open_ = window_open;
   auto_attacher_.SetAutoAttach(auto_attach, wait_for_debugger_on_start,
                                std::move(callback));
   if (!auto_attacher_.ShouldThrottleFramesNavigation())
     ClearThrottles();
+
+  UpdateAgentHostObserver();
+}
+
+void TargetHandler::UpdateAgentHostObserver() {
+  bool should_observe =
+      discover_ || (access_mode_ == AccessMode::kBrowser &&
+                    auto_attacher_.ShouldThrottleFramesNavigation());
+  if (should_observe == observing_agent_hosts_)
+    return;
+  observing_agent_hosts_ = should_observe;
+  if (should_observe)
+    DevToolsAgentHost::AddObserver(this);
+  else
+    DevToolsAgentHost::RemoveObserver(this);
 }
 
 void TargetHandler::AutoAttach(DevToolsAgentHost* host,
@@ -611,12 +672,9 @@ Response TargetHandler::SetDiscoverTargets(bool discover) {
   if (discover_ == discover)
     return Response::OK();
   discover_ = discover;
-  if (discover_) {
-    DevToolsAgentHost::AddObserver(this);
-  } else {
-    DevToolsAgentHost::RemoveObserver(this);
+  UpdateAgentHostObserver();
+  if (!discover_)
     reported_hosts_.clear();
-  }
   return Response::OK();
 }
 
@@ -624,11 +682,14 @@ void TargetHandler::SetAutoAttach(
     bool auto_attach,
     bool wait_for_debugger_on_start,
     Maybe<bool> flatten,
-    Maybe<bool> window_open,
     std::unique_ptr<SetAutoAttachCallback> callback) {
+  if (access_mode_ == AccessMode::kBrowser && !flatten.fromMaybe(false)) {
+    callback->sendFailure(Response::InvalidParams(
+        "Only flatten protocol is supported with browser level auto-attach"));
+    return;
+  }
   SetAutoAttachInternal(
       auto_attach, wait_for_debugger_on_start, flatten.fromMaybe(false),
-      window_open.fromMaybe(false),
       base::BindOnce(&SetAutoAttachCallback::sendSuccess, std::move(callback)));
 }
 
@@ -794,13 +855,34 @@ bool TargetHandler::ShouldForceDevToolsAgentHostCreation() {
   return true;
 }
 
+static bool IsMainFrameHost(DevToolsAgentHost* host) {
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(host->GetWebContents());
+  if (!web_contents)
+    return false;
+  FrameTreeNode* frame_tree_node = web_contents->GetFrameTree()->root();
+  if (!frame_tree_node)
+    return false;
+  return host == RenderFrameDevToolsAgentHost::GetFor(frame_tree_node);
+}
+
 void TargetHandler::DevToolsAgentHostCreated(DevToolsAgentHost* host) {
+  if (discover_) {
     // If we start discovering late, all existing agent hosts will be reported,
     // but we could have already attached to some.
-  if (reported_hosts_.find(host) != reported_hosts_.end())
-    return;
+    if (reported_hosts_.find(host) == reported_hosts_.end()) {
       frontend_->TargetCreated(CreateInfo(host));
       reported_hosts_.insert(host);
+    }
+  }
+  // In the top level target handler auto-attach to pages as soon as they
+  // are created, otherwise if they don't incur any network activity we'll
+  // never get a chance to throttle them (and auto-attach there).
+  if (access_mode_ == AccessMode::kBrowser &&
+      auto_attacher_.ShouldThrottleFramesNavigation() &&
+      IsMainFrameHost(host)) {
+    auto_attacher_.AttachToAgentHost(host);
+  }
 }
 
 void TargetHandler::DevToolsAgentHostNavigated(DevToolsAgentHost* host) {

content/browser/devtools/protocol/target_handler.h

@@ -28,7 +28,8 @@ namespace protocol {
 
 class TargetHandler : public DevToolsDomainHandler,
                       public Target::Backend,
-                      public DevToolsAgentHostObserver {
+                      public DevToolsAgentHostObserver,
+                      public TargetAutoAttacher::Delegate {
  public:
   enum class AccessMode {
     // Only setAutoAttach is supported. Any non-related target are not
@@ -63,7 +64,6 @@ class TargetHandler : public DevToolsDomainHandler,
   void SetAutoAttach(bool auto_attach,
                      bool wait_for_debugger_on_start,
                      Maybe<bool> flatten,
-                     Maybe<bool> window_open,
                      std::unique_ptr<SetAutoAttachCallback> callback) override;
   Response SetRemoteLocations(
       std::unique_ptr<protocol::Array<Target::RemoteLocation>>) override;
@@ -106,9 +106,12 @@ class TargetHandler : public DevToolsDomainHandler,
  private:
   class Session;
   class Throttle;
+  class MainFrameThrottle;
+
+  // TargetAutoAttacher::Delegate implementation.
+  void AutoAttach(DevToolsAgentHost* host, bool waiting_for_debugger) override;
+  void AutoDetach(DevToolsAgentHost* host) override;
 
-  void AutoAttach(DevToolsAgentHost* host, bool waiting_for_debugger);
-  void AutoDetach(DevToolsAgentHost* host);
   Response FindSession(Maybe<std::string> session_id,
                        Maybe<std::string> target_id,
                        Session** session);
@@ -116,8 +119,8 @@ class TargetHandler : public DevToolsDomainHandler,
   void SetAutoAttachInternal(bool auto_attach,
                              bool wait_for_debugger_on_start,
                              bool flatten,
-                             bool window_open,
                              base::OnceClosure callback);
+  void UpdateAgentHostObserver();
 
   // DevToolsAgentHostObserver implementation.
   bool ShouldForceDevToolsAgentHostCreation() override;
@@ -132,8 +135,8 @@ class TargetHandler : public DevToolsDomainHandler,
   std::unique_ptr<Target::Frontend> frontend_;
   TargetAutoAttacher auto_attacher_;
   bool flatten_auto_attach_ = false;
-  bool attach_to_window_open_ = false;
   bool discover_;
+  bool observing_agent_hosts_ = false;
   std::map<std::string, std::unique_ptr<Session>> attached_sessions_;
   std::map<DevToolsAgentHost*, Session*> auto_attached_sessions_;
   std::set<DevToolsAgentHost*> reported_hosts_;

content/browser/devtools/render_frame_devtools_agent_host.cc

@@ -245,6 +245,12 @@ RenderFrameDevToolsAgentHost::RenderFrameDevToolsAgentHost(
   SetFrameTreeNode(frame_tree_node);
   ChangeFrameHostAndObservedProcess(frame_host);
   render_frame_alive_ = frame_host_ && frame_host_->IsRenderFrameLive();
+  if (frame_tree_node->parent()) {
+    render_frame_crashed_ = !render_frame_alive_;
+  } else {
+    WebContents* web_contents = WebContents::FromRenderFrameHost(frame_host);
+    render_frame_crashed_ = web_contents && web_contents->IsCrashed();
+  }
   AddRef();  // Balanced in DestroyOnRenderFrameGone.
   NotifyCreated();
 }
@@ -441,12 +447,8 @@ void RenderFrameDevToolsAgentHost::DidFinishNavigation(
 void RenderFrameDevToolsAgentHost::UpdateFrameHost(
     RenderFrameHostImpl* frame_host) {
   if (frame_host == frame_host_) {
-    if (frame_host && !render_frame_alive_) {
-      render_frame_alive_ = true;
-      for (auto* inspector : protocol::InspectorHandler::ForAgentHost(this))
-        inspector->TargetReloadedAfterCrash();
-      UpdateRendererChannel(IsAttached());
-    }
+    if (frame_host && !render_frame_alive_)
+      UpdateFrameAlive();
     return;
   }
 
@@ -469,13 +471,7 @@ void RenderFrameDevToolsAgentHost::UpdateFrameHost(
   if (!restricted_sessions.empty())
     ForceDetachRestrictedSessions(restricted_sessions);
 
-  if (!render_frame_alive_) {
-    render_frame_alive_ = true;
-    for (auto* inspector : protocol::InspectorHandler::ForAgentHost(this))
-      inspector->TargetReloadedAfterCrash();
-  }
-
-  UpdateRendererChannel(IsAttached());
+  UpdateFrameAlive();
 }
 
 void RenderFrameDevToolsAgentHost::DidStartNavigation(
@@ -558,6 +554,16 @@ void RenderFrameDevToolsAgentHost::ChangeFrameHostAndObservedProcess(
     frame_host_->GetProcess()->AddObserver(this);
 }
 
+void RenderFrameDevToolsAgentHost::UpdateFrameAlive() {
+  render_frame_alive_ = frame_host_ && frame_host_->IsRenderFrameLive();
+  if (render_frame_alive_ && render_frame_crashed_) {
+    render_frame_crashed_ = false;
+    for (auto* inspector : protocol::InspectorHandler::ForAgentHost(this))
+      inspector->TargetReloadedAfterCrash();
+  }
+  UpdateRendererChannel(IsAttached());
+}
+
 void RenderFrameDevToolsAgentHost::RenderProcessExited(
     RenderProcessHost* host,
     const ChildProcessTerminationInfo& info) {
@@ -575,6 +581,7 @@ void RenderFrameDevToolsAgentHost::RenderProcessExited(
       for (auto* inspector : protocol::InspectorHandler::ForAgentHost(this))
         inspector->TargetCrashed();
       NotifyCrashed(info.status);
+      render_frame_crashed_ = true;
       break;
     default:
       for (auto* inspector : protocol::InspectorHandler::ForAgentHost(this))

content/browser/devtools/render_frame_devtools_agent_host.h

@@ -139,6 +139,7 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost
   void UpdateFrameHost(RenderFrameHostImpl* frame_host);
   void SetFrameTreeNode(FrameTreeNode* frame_tree_node);
   void ChangeFrameHostAndObservedProcess(RenderFrameHostImpl* frame_host);
+  void UpdateFrameAlive();
 
   bool ShouldAllowSession(DevToolsSession* session);
 
@@ -159,6 +160,7 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost
   RenderFrameHostImpl* frame_host_ = nullptr;
   base::flat_set<NavigationRequest*> navigation_requests_;
   bool render_frame_alive_ = false;
+  bool render_frame_crashed_ = false;
 
   // The FrameTreeNode associated with this agent.
   FrameTreeNode* frame_tree_node_;

third_party/blink/public/devtools_protocol/browser_protocol.pdl

@@ -6833,8 +6833,6 @@ domain Target
       # We plan to make this the default, deprecate non-flattened mode,
       # and eventually retire it. See crbug.com/991325.
       optional boolean flatten
-      # Auto-attach to the targets created via window.open from current target.
-      experimental optional boolean windowOpen
 
   # Controls whether to discover available targets and notify via
   # `targetCreated/targetInfoChanged/targetDestroyed` events.

third_party/blink/web_tests/http/tests/inspector-protocol/target/target-setAutoAttach-about-blank-expected.txt

+Tests that browser.Target.setAutoAttach() attaches to new about:blank page.
+Created new page from another session
+Auto-attached to the new page: {
+    method : Target.attachedToTarget
+    params : {
+        sessionId : <string>
+        targetInfo : {
+            attached : true
+            browserContextId : <string>
+            targetId : <string>
+            title : 
+            type : page
+            url : 
+        }
+        waitingForDebugger : true
+    }
+}
+Resumed
+Received new target info: {
+    method : Target.targetInfoChanged
+    params : {
+        targetInfo : {
+            attached : true
+            browserContextId : <string>
+            targetId : <string>
+            title : about:blank#newpage
+            type : page
+            url : about:blank#newpage
+        }
+    }
+}
+

third_party/blink/web_tests/http/tests/inspector-protocol/target/target-setAutoAttach-about-blank.js

+(async function(testRunner) {
+  var {page, session, dp} = await testRunner.startBlank(
+      `Tests that browser.Target.setAutoAttach() attaches to new about:blank page.`);
+
+  const target = testRunner.browserP().Target;
+  await target.setDiscoverTargets({discover: true});
+  await target.setAutoAttach(
+      {autoAttach: true, waitForDebuggerOnStart: true, flatten: true});
+  const response = await target.attachToBrowserTarget();
+
+  const newBrowserSession = new TestRunner.Session(testRunner, response.result.sessionId);
+  newBrowserSession.protocol.Target.createTarget({url: 'about:blank#newpage'});
+  testRunner.log('Created new page from another session');
+
+  const attachedEvent = await target.onceAttachedToTarget();
+  testRunner.log(attachedEvent, 'Auto-attached to the new page: ');
+
+  // Navigate elsewhere and test that the request will be paused.
+  const newSession = new TestRunner.Session(testRunner, attachedEvent.params.sessionId);
+  const logSpuriousEvent = event => testRunner.log(event, 'FAIL: received spurious event while paused ');
+  target.onTargetInfoChanged(logSpuriousEvent);
+  newSession.navigate(testRunner.url('../resources/test-page.html?newpage'));
+  // Do a roundtrip to the browser to wait for some time when
+  // TargetInfoChanged could come.
+  await target.getTargets();
+  target.offTargetInfoChanged(logSpuriousEvent);
+  const [infoChangedEvent] = await Promise.all([
+    target.onceTargetInfoChanged(),
+    newSession.protocol.Runtime.runIfWaitingForDebugger()
+  ]);
+  testRunner.log('Resumed');
+  testRunner.log(infoChangedEvent, 'Received new target info: ');
+
+  await newBrowserSession.disconnect();
+  testRunner.completeTest();
+})

third_party/blink/web_tests/http/tests/inspector-protocol/target/target-setAutoAttach-flatten-only-expected.txt

+Tests that browser.Target.setAutoAttach() supports only flatten protocol.
+Tried to auto-attach with not fatten protocol{
+    error : {
+        code : -32602
+        message : Only flatten protocol is supported with browser level auto-attach
+    }
+    id : <number>
+}
+

third_party/blink/web_tests/http/tests/inspector-protocol/target/target-setAutoAttach-flatten-only.js

+(async function(testRunner) {
+  var {page, session, dp} = await testRunner.startBlank(
+      `Tests that browser.Target.setAutoAttach() supports only flatten protocol.`);
+
+  const target = testRunner.browserP().Target;
+  const response = await target.setAutoAttach(
+      {autoAttach: true, waitForDebuggerOnStart: true, flatten: false});
+  testRunner.log(response, 'Tried to auto-attach with not fatten protocol');
+
+  testRunner.completeTest();
+})

third_party/blink/web_tests/http/tests/inspector-protocol/target/target-setAutoAttach-new-page-expected.txt

+Tests that browser.Target.setAutoAttach() attaches to new page targets.
+Attached to the new page: {
+    method : Target.attachedToTarget
+    params : {
+        sessionId : <string>
+        targetInfo : {
+            attached : true
+            browserContextId : <string>
+            targetId : <string>
+            title : 
+            type : page
+            url : 
+        }
+        waitingForDebugger : true
+    }
+}
+Resumed
+
+
+New page location: {
+    id : <number>
+    result : {
+        result : {
+            type : string
+            value : http://127.0.0.1:8000/inspector-protocol/resources/test-page.html?newpage
+        }
+    }
+    sessionId : <string>
+}
+

third_party/blink/web_tests/http/tests/inspector-protocol/target/target-setAutoAttach-new-page.js

+(async function(testRunner) {
+  var {page, session, dp} = await testRunner.startBlank(
+      `Tests that browser.Target.setAutoAttach() attaches to new page targets.`);
+
+  const target = testRunner.browserP().Target;
+  await target.setDiscoverTargets({discover: true});
+  await target.setAutoAttach({autoAttach: true, waitForDebuggerOnStart: true, flatten: true});
+  const response = await target.attachToBrowserTarget();
+
+  const newBrowserSession =
+      new TestRunner.Session(testRunner, response.result.sessionId);
+  const newUrl = testRunner.url('../resources/test-page.html?newpage');
+  newBrowserSession.protocol.Target.createTarget({url: newUrl});
+  const attachedEvent = await target.onceAttachedToTarget();
+  testRunner.log(attachedEvent, 'Attached to the new page: ');
+
+  const newSession = new TestRunner.Session(testRunner, attachedEvent.params.sessionId);
+  newSession.protocol.Inspector.onTargetReloadedAfterCrash(
+      event => testRunner.log(event, 'FAIL: received spurious event '));
+  await newSession.protocol.Runtime.runIfWaitingForDebugger();
+  testRunner.log('Resumed\n\n');
+  const path = await newSession.protocol.Runtime.evaluate({expression: 'location.href'});
+  testRunner.log(path, 'New page location: ');
+
+  await newBrowserSession.disconnect();
+  testRunner.completeTest();
+})

third_party/blink/web_tests/http/tests/inspector-protocol/target/target-setAutoAttach-windowOpen-expected.txt

-Tests that Target.setAutoAttach(windowOpen=true) attaches to window.open targets.
+Tests that browser.Target.setAutoAttach() attaches to window.open targets.
 Opened the window
-Attached to window
+Attached to window, waitingForDebugger=true
+Resumed popup window
+Popup window URL changed to http://127.0.0.1:8000/inspector-protocol/resources/inspector-protocol-page.html
 Navigated the window
-Target info changed
+Target info changed, new URL is http://127.0.0.1:8000/inspector-protocol/resources/test-page.html
 Closed the window
 Detached from window
 

third_party/blink/web_tests/http/tests/inspector-protocol/target/target-setAutoAttach-windowOpen.js

 (async function(testRunner) {
   var {page, session, dp} = await testRunner.startBlank(
-      `Tests that Target.setAutoAttach(windowOpen=true) attaches to window.open targets.`);
+      `Tests that browser.Target.setAutoAttach() attaches to window.open targets.`);
 
-  await dp.Target.setDiscoverTargets({discover: true});
+  const target = testRunner.browserP().Target;
+  await target.setDiscoverTargets({discover: true});
+  await target.setAutoAttach({autoAttach: true, waitForDebuggerOnStart: true, flatten: true});
 
-  await dp.Target.setAutoAttach({autoAttach: true, waitForDebuggerOnStart: true, flatten: true, windowOpen: true});
-
-  const attachedPromise = dp.Target.onceAttachedToTarget();
   session.evaluate(`
     window.myWindow = window.open('../resources/inspector-protocol-page.html'); undefined;
   `);
   testRunner.log('Opened the window');
-  await attachedPromise;
-  testRunner.log('Attached to window');
+  const attachedEvent = await target.onceAttachedToTarget();
+  testRunner.log('Attached to window, waitingForDebugger=' + attachedEvent.params.waitingForDebugger);
+  const popupSession = new TestRunner.Session(testRunner, attachedEvent.params.sessionId);
+  const changedPromise = target.onceTargetInfoChanged();
+  await popupSession.protocol.Runtime.runIfWaitingForDebugger();
+  testRunner.log('Resumed popup window');
+  const changeEvent = await changedPromise;
+  testRunner.log('Popup window URL changed to ' + changeEvent.params.targetInfo.url);
 
-  const changedPromise = dp.Target.onceTargetInfoChanged();
+  const secondChangedPromise = target.onceTargetInfoChanged();
   session.evaluate(`
-    window.myWindow.location.assign('../resources/inspector-protocol-page.html?foo'); undefined;
+    window.myWindow.location.assign('../resources/test-page.html'); undefined;
   `);
   testRunner.log('Navigated the window');
-  await changedPromise;
-  testRunner.log('Target info changed');
+  const secondChangeEvent = await secondChangedPromise;
+  testRunner.log('Target info changed, new URL is ' + secondChangeEvent.params.targetInfo.url);
 
-  const detachedPromise = dp.Target.onceDetachedFromTarget();
+  const detachedPromise = target.onceDetachedFromTarget();
   session.evaluate(`
     window.myWindow.close(); undefined;
   `);