Pipe the devTools FrameId from blink into the browser for headless

headless/lib/browser/headless_browser_context_impl.h

Index: headless/lib/browser/headless_browser_context_impl.h
diff --git a/headless/lib/browser/headless_browser_context_impl.h b/headless/lib/browser/headless_browser_context_impl.h
index 688244917936123d7ac9e369501f85f4135758bb..e2820ed3d7e5264b3ef923aba100f3123590afff 100644
--- a/headless/lib/browser/headless_browser_context_impl.h
+++ b/headless/lib/browser/headless_browser_context_impl.h
@@ -45,6 +45,12 @@ class HeadlessBrowserContextImpl : public HeadlessBrowserContext,
   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;
@@ -81,6 +87,10 @@ class HeadlessBrowserContextImpl : public HeadlessBrowserContext,
   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,
@@ -98,6 +108,9 @@ class HeadlessBrowserContextImpl : public HeadlessBrowserContext,
   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_;