Adding Navigation Throttles to DevTools

content/browser/devtools/protocol/devtools_protocol_browsertest.cc

Index: content/browser/devtools/protocol/devtools_protocol_browsertest.cc
diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index e6e83576d6626b1eab23df2ea201c6032e936fe0..510779494f9d10bcd29e152b1eb3242d3d8f6ef3 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -180,6 +180,7 @@ class DevToolsProtocolTest : public ContentBrowserTest,
   int last_sent_id_;
   std::vector<int> result_ids_;
   std::vector<std::string> notifications_;
+  std::unique_ptr<base::DictionaryValue> requested_notification_params_;
 
  private:
   void DispatchProtocolMessage(DevToolsAgentHost* agent_host,
@@ -203,6 +204,12 @@ class DevToolsProtocolTest : public ContentBrowserTest,
       EXPECT_TRUE(root->GetString("method", &notification));
       notifications_.push_back(notification);
       if (waiting_for_notification_ == notification) {
+        base::DictionaryValue* params;
+        if (root->GetDictionary("params", &params)) {
+          requested_notification_params_ = params->CreateDeepCopy();
+        } else {
+          requested_notification_params_.reset();
+        }
         waiting_for_notification_ = std::string();
         base::MessageLoop::current()->QuitNow();
       }
@@ -597,4 +604,74 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, BrowserNewPage) {
   EXPECT_EQ(2u, shell()->windows().size());
 }
 
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, NavigationThrottle) {
+  NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1);
+  ASSERT_TRUE(embedded_test_server()->Start());
+  Attach();
+  SendCommand("Page.enable", nullptr);
+
+  std::unique_ptr<base::DictionaryValue> params(new base::DictionaryValue());
+  params->SetBoolean("enabled", true);
+  SendCommand("Page.setControlNavigations", std::move(params), true);
+
+  GURL test_url = embedded_test_server()->GetURL("/devtools/meta_tag.html");
+  shell()->LoadURL(test_url);
+
+  // Wait for and allow navigation to /devtools/meta_tag.html
+  WaitForNotification("Page.navigationRequested");
+  ASSERT_TRUE(requested_notification_params_.get());
+  std::string url;
+  ASSERT_TRUE(requested_notification_params_->GetString("url", &url));
+  EXPECT_TRUE(base::EndsWith(url, "/devtools/meta_tag.html",
+                             base::CompareCase::SENSITIVE));
+  int navigation_id;
+  ASSERT_TRUE(requested_notification_params_->GetInteger("navigationId",
+                                                         &navigation_id));
+  bool renderer_initiated;
+  ASSERT_TRUE(requested_notification_params_->GetBoolean("rendererInitiated",
+                                                         &renderer_initiated));
+  EXPECT_FALSE(renderer_initiated);
+  bool is_in_main_frame;
+  ASSERT_TRUE(requested_notification_params_->GetBoolean("isInMainFrame",
+                                                         &is_in_main_frame));
+  EXPECT_TRUE(is_in_main_frame);
+  bool is_redirect;
+  ASSERT_TRUE(
+      requested_notification_params_->GetBoolean("isRedirect", &is_redirect));
+  EXPECT_FALSE(is_redirect);
+
+  params.reset(new base::DictionaryValue());
+  params->SetString("response", "Proceed");
+  params->SetInteger("navigationId", navigation_id);
+  SendCommand("Page.processNavigation", std::move(params), true);
+
+  // Wait for and cancel navigation to /devtools/navigation.html
+  WaitForNotification("Page.navigationRequested");
+
+  ASSERT_TRUE(requested_notification_params_.get());
+  ASSERT_TRUE(requested_notification_params_->GetString("url", &url));
+  EXPECT_TRUE(base::EndsWith(url, "/devtools/navigation.html",
+                             base::CompareCase::SENSITIVE));
+  ASSERT_TRUE(requested_notification_params_->GetInteger("navigationId",
+                                                         &navigation_id));
+  ASSERT_TRUE(requested_notification_params_->GetBoolean("rendererInitiated",
+                                                         &renderer_initiated));
+  EXPECT_TRUE(renderer_initiated);
+  ASSERT_TRUE(requested_notification_params_->GetBoolean("isInMainFrame",
+                                                         &is_in_main_frame));
+  EXPECT_TRUE(is_in_main_frame);
+  ASSERT_TRUE(
+      requested_notification_params_->GetBoolean("isRedirect", &is_redirect));
+  EXPECT_FALSE(is_redirect);
+
+  params.reset(new base::DictionaryValue());
+  params->SetString("response", "Cancel");
+  params->SetInteger("navigationId", navigation_id);
+  SendCommand("Page.processNavigation", std::move(params), true);
+
+  // Check the cancellation occurred.
+  WaitForNotification("Page.frameClearedScheduledNavigation");
+  ASSERT_TRUE(requested_notification_params_.get());
+}

[nasko, 2016/07/13 22:37:34]

Let's add a test that ensures navigating iframes are also being properly handled
by the new API. You can use content/test/data/cross_site_iframe_factory.html to
create arbitrary tree of iframes (plenty of examples in codesearch) and
IsolateAllSitesForTesting to turn on OOPIF for all cross-site iframes (for bonus
points :) ).

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

It turns out this test is actually very awkward to write with
IsolateAllSitesForTesting enabled.  The Page domain behaves differently because
the commands handled on the renderer only work for the main frame.  Presumably
there's now new Page domains for each of the child processes.

Putting some prints in PageNavigationThrottle shows it does appear to be working
however with IsolateAllSitesForTesting.

If there's some way of detecting when everything has finished loading and
interrogating the iframe URLs without the page domain then I could make this
test work with IsolateAllSitesForTesting.

[dgozman, 2016/07/14 16:40:32]

It's possible to attach to all oopifs separately, as we do in
site_per_process_devtools_browsertest.cc. However, issuing and listening for
protocol events there would be a pain, so I won't insist on doing this. We lack
test infrastructure in this area.

[nasko, 2016/07/14 16:47:19]

Does your CL care about the renderer process at all? If I got it correctly from
dgozman@, the remote debugging protocol shouldn't expose details whether we have
IsolateAllSitesForTesting enabled or not.
There should be at least a new RenderFrameDevToolsAgentHost. The PageHandler
should be the same, but I'm not sure what "Page domain" is.
You can use TestNavigationObserver from
content/public/test/test_navigation_observer.h and the single param constructor
taking a WebContents. It will wait for the load stopped event, which is fired at
the end when all frames have completed loading.

[alex clarke (OOO till 29th), 2016/07/14 17:26:17]

Yes it does :( If IsolateAllSitesForTesting is enabled you only get events for
the main frame sent.
I tried that.  It was only counting 1 or 2 loads.  I'm speculating that's
because the iframes are navigating concurrently.

[nasko, 2016/07/14 17:55:51]

Ok, let's put a TODO to enable OOPIFs later on, once test infrastructure is
caught up.
You should only care about 1 load, as the load of the main frame encloses the
loads of all subframes. You will *not* see 5 loads. Think of the
TestNavigationObserver being the test equivalent of the spinner in the UI -
whenever something is spinning it is waiting. The moment it stops spinning, it
returns from the wait.

[alex clarke (OOO till 29th), 2016/07/15 13:21:21]

I figured out how to get this to work with IsolateAllSitesForTesting.

+
 }  // namespace content