DevTools: limit Browser.* protocol methods to the clients with universal only.

This is done proactively, in case we want to add features to Browser.* that we
don't want to be exposed over extensions API, etc.

Change-Id: Ice2dc0ffbec783b429daae8e950a5861228a95cf
TBR: updated API call site
Reviewed-on: https://chromium-review.googlesource.com/c/1334003Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607812}

chrome/browser/devtools/chrome_devtools_session.cc

@@ -27,7 +27,10 @@ ChromeDevToolsSession::ChromeDevToolsSession(
                                                   dispatcher_.get());
   }
   target_handler_ = std::make_unique<TargetHandler>(dispatcher_.get());
-  browser_handler_ = std::make_unique<BrowserHandler>(dispatcher_.get());
+  if (client->MayAttachToBrowser()) {
+    browser_handler_ = std::make_unique<BrowserHandler>(dispatcher_.get(),
+                                                        agent_host->GetId());
+  }
 #if defined(OS_CHROMEOS)
   window_manager_protocl_handler_ =
       std::make_unique<WindowManagerHandler>(dispatcher_.get());

chrome/browser/devtools/devtools_sanity_interactive_browsertest.cc

@@ -68,7 +68,7 @@ class DevToolsManagerDelegateTest : public InProcessBrowserTest {
   void SendCommand(const std::string& state) {
     auto window_bounds =
         protocol::Browser::Bounds::Create().SetWindowState(state).Build();
-    BrowserHandler handler(nullptr);
+    BrowserHandler handler(nullptr, "");
     handler.SetWindowBounds(browser()->session_id().id(),
                             std::move(window_bounds));
   }
@@ -79,7 +79,7 @@ class DevToolsManagerDelegateTest : public InProcessBrowserTest {
                              .SetLeft(200)
                              .SetHeight(400)
                              .Build();
-    BrowserHandler handler(nullptr);
+    BrowserHandler handler(nullptr, "");
     handler.SetWindowBounds(browser()->session_id().id(),
                             std::move(window_bounds));
   }

chrome/browser/devtools/protocol/browser_handler.cc

@@ -102,7 +102,9 @@ Response FromProtocolPermissionType(
 
 }  // namespace
 
-BrowserHandler::BrowserHandler(protocol::UberDispatcher* dispatcher) {
+BrowserHandler::BrowserHandler(protocol::UberDispatcher* dispatcher,
+                               const std::string& target_id)
+    : target_id_(target_id) {
   // Dispatcher can be null in tests.
   if (dispatcher)
     protocol::Browser::Dispatcher::wire(dispatcher, this);
@@ -111,10 +113,11 @@ BrowserHandler::BrowserHandler(protocol::UberDispatcher* dispatcher) {
 BrowserHandler::~BrowserHandler() = default;
 
 Response BrowserHandler::GetWindowForTarget(
-    const std::string& target_id,
+    protocol::Maybe<std::string> target_id,
     int* out_window_id,
     std::unique_ptr<protocol::Browser::Bounds>* out_bounds) {
-  auto host = content::DevToolsAgentHost::GetForId(target_id);
+  auto host =
+      content::DevToolsAgentHost::GetForId(target_id.fromMaybe(target_id_));
   if (!host)
     return Response::Error("No target with given id");
   content::WebContents* web_contents = host->GetWebContents();

chrome/browser/devtools/protocol/browser_handler.h

@@ -12,12 +12,13 @@ class Profile;
 
 class BrowserHandler : public protocol::Browser::Backend {
  public:
-  explicit BrowserHandler(protocol::UberDispatcher* dispatcher);
+  BrowserHandler(protocol::UberDispatcher* dispatcher,
+                 const std::string& target_id);
   ~BrowserHandler() override;
 
   // Browser::Backend:
   protocol::Response GetWindowForTarget(
-      const std::string& target_id,
+      protocol::Maybe<std::string> target_id,
       int* out_window_id,
       std::unique_ptr<protocol::Browser::Bounds>* out_bounds) override;
   protocol::Response GetWindowBounds(
@@ -42,6 +43,7 @@ class BrowserHandler : public protocol::Browser::Backend {
       Profile** profile);
 
   base::flat_set<std::string> contexts_with_overridden_permissions_;
+  std::string target_id_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserHandler);
 };

chrome/browser/extensions/api/debugger/debugger_api.cc

@@ -148,7 +148,6 @@ class ExtensionDevToolsClientHost : public content::DevToolsAgentHostClient,
   bool MayAttachToRenderer(content::RenderFrameHost* render_frame_host,
                            bool is_webui) override;
   bool MayAttachToBrowser() override;
-  bool MayDiscoverTargets() override;
   bool MayAffectLocalFiles() override;
 
  private:
@@ -383,10 +382,6 @@ bool ExtensionDevToolsClientHost::MayAttachToBrowser() {
   return false;
 }
 
-bool ExtensionDevToolsClientHost::MayDiscoverTargets() {
-  return false;
-}
-
 bool ExtensionDevToolsClientHost::MayAffectLocalFiles() {
   return false;
 }

content/browser/devtools/render_frame_devtools_agent_host.cc

@@ -242,7 +242,7 @@ bool RenderFrameDevToolsAgentHost::AttachSession(DevToolsSession* session) {
   session->AddHandler(base::WrapUnique(new protocol::ServiceWorkerHandler()));
   session->AddHandler(base::WrapUnique(new protocol::StorageHandler()));
   session->AddHandler(base::WrapUnique(new protocol::TargetHandler(
-      session->client()->MayDiscoverTargets()
+      session->client()->MayAttachToBrowser()
           ? protocol::TargetHandler::AccessMode::kRegular
           : protocol::TargetHandler::AccessMode::kAutoAttachOnly,
       GetId(), GetRendererChannel(), session->GetRootSession())));

content/public/browser/devtools_agent_host_client.cc

@@ -16,10 +16,6 @@ bool DevToolsAgentHostClient::MayAttachToBrowser() {
   return true;
 }
 
-bool DevToolsAgentHostClient::MayDiscoverTargets() {
-  return true;
-}
-
 bool DevToolsAgentHostClient::MayAffectLocalFiles() {
   return true;
 }

content/public/browser/devtools_agent_host_client.h

@@ -33,12 +33,10 @@ class CONTENT_EXPORT DevToolsAgentHostClient {
                                    bool is_webui);
 
   // Returns true if the client is allowed to attach to the browser agent host.
+  // Browser client is allowed to discover other DevTools targets and generally
+  // manipulate browser altogether.
   virtual bool MayAttachToBrowser();
 
-  // Returns true if the client is allowed to discover other DevTools targets.
-  // If not, it will be restricted to auto-attaching to related targets.
-  virtual bool MayDiscoverTargets();
-
   // Returns true if the client is allowed to affect local files over the
   // protocol. Example would be manipulating a deault downloads path.
   virtual bool MayAffectLocalFiles();

headless/lib/browser/protocol/browser_handler.cc

@@ -30,8 +30,10 @@ std::unique_ptr<Browser::Bounds> CreateBrowserBounds(
 
 }  // namespace
 
-BrowserHandler::BrowserHandler(base::WeakPtr<HeadlessBrowserImpl> browser)
-    : DomainHandler(Browser::Metainfo::domainName, browser) {}
+BrowserHandler::BrowserHandler(base::WeakPtr<HeadlessBrowserImpl> browser,
+                               const std::string& target_id)
+    : DomainHandler(Browser::Metainfo::domainName, browser),
+      target_id_(target_id) {}
 
 BrowserHandler::~BrowserHandler() = default;
 
@@ -40,11 +42,12 @@ void BrowserHandler::Wire(UberDispatcher* dispatcher) {
 }
 
 Response BrowserHandler::GetWindowForTarget(
-    const std::string& target_id,
+    Maybe<std::string> target_id,
     int* out_window_id,
     std::unique_ptr<Browser::Bounds>* out_bounds) {
   HeadlessWebContentsImpl* web_contents = HeadlessWebContentsImpl::From(
-      browser()->GetWebContentsForDevToolsAgentHostId(target_id));
+      browser()->GetWebContentsForDevToolsAgentHostId(
+          target_id.fromMaybe(target_id_)));
   if (!web_contents)
     return Response::Error("No web contents for the given target id");
 

headless/lib/browser/protocol/browser_handler.h

@@ -13,14 +13,15 @@ namespace protocol {
 
 class BrowserHandler : public DomainHandler, public Browser::Backend {
  public:
-  explicit BrowserHandler(base::WeakPtr<HeadlessBrowserImpl> browser);
+  BrowserHandler(base::WeakPtr<HeadlessBrowserImpl> browser,
+                 const std::string& target_id);
   ~BrowserHandler() override;
 
   void Wire(UberDispatcher* dispatcher) override;
 
   // Browser::Backend implementation
   Response GetWindowForTarget(
-      const std::string& target_id,
+      Maybe<std::string> target_id,
       int* out_window_id,
       std::unique_ptr<Browser::Bounds>* out_bounds) override;
   Response GetWindowBounds(
@@ -32,6 +33,7 @@ class BrowserHandler : public DomainHandler, public Browser::Backend {
       std::unique_ptr<Browser::Bounds> out_bounds) override;
 
  private:
+  std::string target_id_;
   DISALLOW_COPY_AND_ASSIGN(BrowserHandler);
 };
 

headless/lib/browser/protocol/headless_devtools_session.cc

@@ -29,8 +29,8 @@ HeadlessDevToolsSession::HeadlessDevToolsSession(
     AddHandler(
         std::make_unique<PageHandler>(browser_, agent_host->GetWebContents()));
   }
-  if (agent_host->GetType() == content::DevToolsAgentHost::kTypeBrowser)
-    AddHandler(std::make_unique<BrowserHandler>(browser_));
+  if (client->MayAttachToBrowser())
+    AddHandler(std::make_unique<BrowserHandler>(browser_, agent_host->GetId()));
   AddHandler(std::make_unique<TargetHandler>(browser_));
 }
 

third_party/blink/renderer/core/inspector/browser_protocol.pdl

@@ -617,8 +617,8 @@ domain Browser
   # Get the browser window that contains the devtools target.
   experimental command getWindowForTarget
     parameters
-      # Devtools agent host id.
-      Target.TargetID targetId
+      # Devtools agent host id. If called as a part of the session, associated targetId is used.
+      optional Target.TargetID targetId
     returns
       # Browser window id.
       WindowID windowId