Pipe the devTools FrameId from blink into the browser for headless

headless/lib/browser/headless_browser_context_impl.h

 // Copyright 2015 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 HEADLESS_LIB_BROWSER_HEADLESS_BROWSER_CONTEXT_IMPL_H_
 #define HEADLESS_LIB_BROWSER_HEADLESS_BROWSER_CONTEXT_IMPL_H_
 
 #include <memory>
 #include <string>
 #include <unordered_map>
@@ skipping 27 matching lines @@
       HeadlessBrowserContext::Builder* builder);
 
   // HeadlessBrowserContext implementation:
   HeadlessWebContents::Builder CreateWebContentsBuilder() override;
   std::vector<HeadlessWebContents*> GetAllWebContents() override;
   HeadlessWebContents* GetWebContentsForDevToolsAgentHostId(
       const std::string& devtools_agent_host_id) override;
   void Close() override;
   const std::string& Id() const override;
 
+  void SetFrameTreeNodeId(int render_process_id,
+                          int render_frame_id,

[Charlie Reis, 2017/04/26 19:52:55]

routing_id or render_frame_routing_id would be less ambiguous, since we now have
at least 3 possible meanings for "frame id" (routing ID, frame_tree_node_id, and
the DevTools frameId).

[alex clarke (OOO till 29th), 2017/04/27 08:49:44]

Done.

+                          int frame_tree_node_id);
+
+  void RemoveFrameTreeNode(int render_process_id, int render_frame_id);

[Charlie Reis, 2017/04/26 19:52:55]

Same here.

[alex clarke (OOO till 29th), 2017/04/27 08:49:43]

Done.

+
   // BrowserContext implementation:
   std::unique_ptr<content::ZoomLevelDelegate> CreateZoomLevelDelegate(
       const base::FilePath& partition_path) override;
   base::FilePath GetPath() const override;
   bool IsOffTheRecord() const override;
   content::ResourceContext* GetResourceContext() override;
   content::DownloadManagerDelegate* GetDownloadManagerDelegate() override;
   content::BrowserPluginGuestManager* GetGuestManager() override;
   storage::SpecialStoragePolicy* GetSpecialStoragePolicy() override;
   content::PushMessagingService* GetPushMessagingService() override;
@@ skipping 16 matching lines @@
   HeadlessWebContents* CreateWebContents(HeadlessWebContents::Builder* builder);
   // Register web contents which were created not through Headless API
   // (calling window.open() is a best example for this).
   void RegisterWebContents(
       std::unique_ptr<HeadlessWebContentsImpl> web_contents);
   void DestroyWebContents(HeadlessWebContentsImpl* web_contents);
 
   HeadlessBrowserImpl* browser() const;
   const HeadlessBrowserContextOptions* options() const;
 
+  // Returns the frame tree node id for the corresponding RenderFrameHost or -1

[Charlie Reis, 2017/04/26 19:52:55]

nit: FrameTreeNode id

[alex clarke (OOO till 29th), 2017/04/27 08:49:43]

Done.

+  // if it can't be found. Can be called on any thread.
+  int GetFrameTreeNodeId(int render_process_id, int render_frame_id) const;
+
  private:
   HeadlessBrowserContextImpl(
       HeadlessBrowserImpl* browser,
       std::unique_ptr<HeadlessBrowserContextOptions> context_options);
 
   // Performs initialization of the HeadlessBrowserContextImpl while IO is still
   // allowed on the current thread.
   void InitWhileIOAllowed();
 
   HeadlessBrowserImpl* browser_;  // Not owned.
   std::unique_ptr<HeadlessBrowserContextOptions> context_options_;
   std::unique_ptr<HeadlessResourceContext> resource_context_;
   base::FilePath path_;
 
   std::unordered_map<std::string, std::unique_ptr<HeadlessWebContents>>
       web_contents_map_;
 
+  mutable base::Lock frame_tree_node_map_lock_;  // Guards frame_tree_node_map_

[Charlie Reis, 2017/04/26 19:52:55]

Why does this need to be locked?  We try hard to avoid sharing data structures
across threads when possible.

[alex clarke (OOO till 29th), 2017/04/27 08:49:43]

That's actually why we're adding this :)  We want to call
HeadlessBrowserContextImpl::GetFrameTreeNodeId from the IO thread.  That means
we can't use RenderFrameHost::FromID and have to roll our own thread safe lookup
(HeadlessBrowserContextImpl::SetFrameTreeNodeId gets called on the UI thread).

[Charlie Reis, 2017/04/27 18:58:02]

Hmm, it seems like we should avoid that when possible.  When looking for
alternatives, though, I found that extension_api_frame_id_map.h unfortunately
does something similar.  :(  Maybe we need to provide a better way to do this
association on the IO thread so that features don't add new locks for it.

In the meantime, maybe you can do this within headless if it doesn't affect
normal Chrome.  I'm curious if the need goes away when PlzNavigate ships (see my
other question).

[alex clarke (OOO till 29th), 2017/04/28 09:14:44]

Yes but only for browser side navigation requests. Anything resources fetches
from blink will still require us to do this lookup.

Incidentally I wonder if this could be avoided if the FrameTreeNode Id was
included in URLRequestUserData?

[Charlie Reis, 2017/04/28 20:29:03]

Perhaps.  That's in content/common, so we can't do that until we finish the plan
to expose FrameTreeNode IDs to the renderer process.  Once that's done, I would
imagine we could add it there, and that could give you a way to eliminate this
lock.

Can you add a TODO to that effect, along the lines of this?
"This might be unnecessary if we can add FrameTreeNode ID to URLRequestUserData
in https://crbug.com/715541."

[alex clarke (OOO till 29th), 2017/05/02 08:24:11]

Done.

+  std::map<std::pair<int, int>, int> frame_tree_node_map_;
+
   std::unique_ptr<content::PermissionManager> permission_manager_;
 
   std::string id_;
 
   DISALLOW_COPY_AND_ASSIGN(HeadlessBrowserContextImpl);
 };
 
 }  // namespace headless
 
 #endif  // HEADLESS_LIB_BROWSER_HEADLESS_BROWSER_CONTEXT_IMPL_H_