PlzNavigate: send Javascript urls synchronously to the renderer

content/browser/frame_host/frame_tree_node.cc

 // Copyright 2013 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.
 
 #include "content/browser/frame_host/frame_tree_node.h"
 
 #include <queue>
 
 #include "base/command_line.h"
 #include "base/profiler/scoped_tracker.h"
@@ skipping 168 matching lines @@
 bool FrameTreeNode::IsLoading() const {
   RenderFrameHostImpl* current_frame_host =
       render_manager_.current_frame_host();
   RenderFrameHostImpl* pending_frame_host =
       render_manager_.pending_frame_host();
 
   DCHECK(current_frame_host);
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableBrowserSideNavigation)) {
-    if (navigation_request_)
+    if (navigation_request_ &&
+        !navigation_request_->common_params().url.SchemeIs(
+            url::kJavaScriptScheme))

[nasko, 2015/05/29 14:17:46]

Why not keep it closer to existing functionality? The loading state is
explicitly set in RFH::Navigate -
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr....

[clamy, 2015/05/29 16:02:06]

We cannot use a RFH to set the loading state as we don't have one until the
navigation is ready to commit, and we need to be aware we are loading before
that. So we're using the NavigationRequest until the navigation has committed. I
have added a comment as to why this specific case about JavaScript urls is
needed.

[nasko, 2015/06/01 20:38:22]

Do we even create NavigationRequest objects for JS URLs? Purely on code flow, if
there is navigation_request_ for JS URL, we will fall back to
current_frame_host->is_loading(). What happens if we don't have a current host -
new tab that is going to load a JS URL?

[clamy, 2015/06/02 12:28:16]

After further thinking, I removed this part, and now reset the NavigationRequest
in NavigatorImpl::CommitNavigation, since we are not expecting a commit from a
Javascript URL. This seems better and solve the loading state issue if the
Javascript URL triggers a new navigation.

       return true;
   } else {
     if (pending_frame_host && pending_frame_host->is_loading())
       return true;
   }
   return current_frame_host->is_loading();
 }
 
 bool FrameTreeNode::CommitPendingSandboxFlags() {
   bool did_change_flags =
@@ skipping 100 matching lines @@
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "465796 FrameTreeNode::DidStopLoading::End"));
 }
 
 void FrameTreeNode::DidChangeLoadProgress(double load_progress) {
   loading_progress_ = load_progress;
   frame_tree_->UpdateLoadProgress();
 }
 
 }  // namespace content