Adding Navigation Throttles to DevTools

content/browser/devtools/render_frame_devtools_agent_host.cc

Index: content/browser/devtools/render_frame_devtools_agent_host.cc
diff --git a/content/browser/devtools/render_frame_devtools_agent_host.cc b/content/browser/devtools/render_frame_devtools_agent_host.cc
index a22158bbfd509b7e5500cdf0b570b68ae76ca881..33f43b6ac6a7278f5c75836571a83ed653b46d4b 100644
--- a/content/browser/devtools/render_frame_devtools_agent_host.cc
+++ b/content/browser/devtools/render_frame_devtools_agent_host.cc
@@ -13,6 +13,7 @@
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/devtools_frame_trace_recorder.h"
 #include "content/browser/devtools/devtools_protocol_handler.h"
+#include "content/browser/devtools/page_navigation_throttle.h"
 #include "content/browser/devtools/protocol/browser_handler.h"
 #include "content/browser/devtools/protocol/dom_handler.h"
 #include "content/browser/devtools/protocol/emulation_handler.h"
@@ -345,6 +346,23 @@ void RenderFrameDevToolsAgentHost::OnBeforeNavigation(
     agent_host->AboutToNavigate(navigation_handle);
 }
 
+// static
+std::unique_ptr<NavigationThrottle>
+RenderFrameDevToolsAgentHost::GetThrottleForNavigation(
+    NavigationHandle* navigation_handle) {
+  FrameTreeNode* frame_tree_node =
+      static_cast<NavigationHandleImpl*>(navigation_handle)->frame_tree_node();
+  while (frame_tree_node && frame_tree_node->parent()) {
+    frame_tree_node = frame_tree_node->parent();

[nasko, 2016/07/12 18:03:11]

Is this code compatible with out-of-process iframes? Why is it always looking at
the root node?

[alex clarke (OOO till 29th), 2016/07/12 20:36:14]

I'm not sure.  I'm not up to speed with OOPIF.
I'm assuming that's where the relevant RenderFrameDevToolsAgentHost is.  The
intended usage is request a throttle at the Page level which will send
Page.navigationRequested events if the main frame or a child frame attempts to
navigate.

[nasko, 2016/07/12 23:08:21]

I'd suggest discussing more with dgozman@, as I'm not very fluent in DevTools,
but my understanding is that RenderFrameDevToolsAgentHost can be attached to any
frame in the tree. Therefore it seems more correct to me that the
FindAgentHost(frame_tree_node) call is made within the while loop and exited
once a DevTools agent is found.

[dgozman, 2016/07/12 23:56:15]

I think this API only makes sense for the page as a whole, so going up to the
root is good.

[alex clarke (OOO till 29th), 2016/07/13 09:18:44]

I agree.

[nasko, 2016/07/13 14:55:55]

Is this feature supposed to allow you to control navigations in subframes or
only the main frame? If only the main frame is of interest, then this code
should work fine. However, if you want to control subframes too, in
out-of-process iframes mode I'm not convinced this code will work correctly.

[alex clarke (OOO till 29th), 2016/07/13 16:14:23]

In both.
What would be needed to fix it for OOPIF?

[nasko, 2016/07/13 16:35:52]

I'm not sure what the design goal of this CL is, so not easy to give direct
advice. Is there a design doc of what this API is supposed to do and how it is
expected to behave? Alternatively, we do a quick chat offline?

+  }
+  RenderFrameDevToolsAgentHost* agent_host = FindAgentHost(frame_tree_node);
+  if (agent_host && agent_host->page_handler_) {
+    return agent_host->page_handler_->GetThrottleForNavigation(
+        navigation_handle);
+  }
+  return nullptr;
+}
+
 RenderFrameDevToolsAgentHost::RenderFrameDevToolsAgentHost(
     RenderFrameHostImpl* host)
     : browser_handler_(new devtools::browser::BrowserHandler()),