PlzNavigate: Support data urls

content/browser/frame_host/navigation_request.cc

Index: content/browser/frame_host/navigation_request.cc
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
index 8a599b199427ffc84e9990ea1897e95279082bc7..b254658f7ad9e174bf40b3407d727d004cf42878 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -85,12 +85,16 @@ scoped_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated(
   ui_timestamp = entry.intent_received_timestamp();
 #endif
 
+  const GURL history_url_for_data_url =
+    entry.GetBaseURLForDataURL().is_empty()? GURL() : entry.GetVirtualURL();

[Charlie Reis, 2015/02/18 00:43:25]

nit: Space before ?

[clamy, 2015/02/20 17:05:58]

Done.

+
   scoped_ptr<NavigationRequest> navigation_request(new NavigationRequest(
       frame_tree_node,
       CommonNavigationParams(entry.GetURL(), entry.GetReferrer(),
                              entry.GetTransitionType(), navigation_type,
                              !entry.IsViewSourceMode(),ui_timestamp,
-                             report_type),
+                             report_type, entry.GetBaseURLForDataURL(),
+                             history_url_for_data_url),
       BeginNavigationParams(method, headers.ToString(),
                             LoadFlagFromNavigationType(navigation_type),
                             false),
@@ -158,13 +162,27 @@ NavigationRequest::NavigationRequest(
 NavigationRequest::~NavigationRequest() {
 }
 
-void NavigationRequest::BeginNavigation() {
+bool NavigationRequest::BeginNavigation() {
   DCHECK(!loader_);
   DCHECK(state_ == NOT_STARTED || state_ == WAITING_FOR_RENDERER_RESPONSE);
   state_ = STARTED;
-  loader_ = NavigationURLLoader::Create(
-      frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
-      frame_tree_node_->frame_tree_node_id(), info_.Pass(), this);
+
+  if (ShouldMakeNetworkRequestForNavigation(common_params_.url)) {
+    loader_ = NavigationURLLoader::Create(
+        frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
+        frame_tree_node_->frame_tree_node_id(), info_.Pass(), this);
+    return true;
+  }
+
+  state_ = RESPONSE_STARTED;
+  // There is no need to make a network request for this navigation, so commit
+  // it immediately if it was browser initiated. If it was renderer initiated,
+  // the renderer should already be trying to commit it.

[Charlie Reis, 2015/02/18 00:43:25]

I'm not sure I agree with the second sentence.  Data URLs don't commit
synchronously, and it's not clear to me whether the renderer process should
directly commit them.  Why would they not go through the browser process?

[clamy, 2015/02/20 17:05:58]

Changed the behavior so that now we always go through the commit step. I guess
the only renderer-initiated navigations that the renderer should pursue by
itself are the same-document ones.

+  if (browser_initiated_) {
+    frame_tree_node_->navigator()->CommitNavigation(

[carlosk, 2015/02/17 12:35:09]

I find it weird that we're calling into Navigator from a class named
NavitationRequest (it sounds like a static thing) but as we're already doing so
in 2 different instances this doesn't change anything. It's maybe something we'd
want to figure out later...

[clamy, 2015/02/17 12:54:28]

I am not quite sure what you mean there. There are several instances of
Navigator, so we need the right one to commit the navigation.

[carlosk, 2015/02/17 14:11:23]

Yes, one navigator per node. I's just an architectural concern that maybe a new
delegate would solve.

[Charlie Reis, 2015/02/18 00:43:25]

To be clear, there is not one Navigator per FrameTreeNode.  In most cases, all
FrameTreeNodes of a WebContents share the same Navigator.  We made it possible
for a FTN to have a different Navigator to support things like the <webview> tag
in apps.

[carlosk, 2015/02/18 15:45:08]

Acknowledged.

+        frame_tree_node_, nullptr, scoped_ptr<StreamHandle>());
+  }
+  return false;
 
   // TODO(davidben): Fire (and add as necessary) observer methods such as
   // DidStartProvisionalLoadForFrame for the navigation.