PlzNavigate: identify same-page browser-initiated navigation.

third_party/WebKit/Source/core/loader/FrameLoader.cpp

Index: third_party/WebKit/Source/core/loader/FrameLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
index 0610418fd1c887b2c6234a917927ef78991a3ac6..273f5cd576740e0b81b6d92d2384965187127496 100644
--- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -1173,7 +1173,6 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest,
       shouldPerformFragmentNavigation(request.form(),
                                       request.resourceRequest().httpMethod(),
                                       newLoadType, url);
-
   // Perform same document navigation.
   if (sameDocumentHistoryNavigation || sameDocumentNavigation) {
     DCHECK(historyItem || !sameDocumentHistoryNavigation);
@@ -1192,6 +1191,14 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest,
     return;
   }
 
+  // PlzNavigate
+  // If the loader classifies this navigation as a different document navigation
+  // while the browser intended the navigation to be same-document, it means
+  // that a different navigation must have committed while the IPC was sent.
+  // This navigation is no more same-document. The navigation is simply dropped.

[nasko, 2017/01/13 19:36:57]

Looking at the code in RenderFrameImpl::NavigateInternal, it should already
detect the race condition and should have changed the load type to be different
document. Why do we need to special case this case here at all?

[arthursonzogni, 2017/01/17 15:24:48]

It's because the response body stream is empty.
The browser wanted the navigation to be same-document. So no network response
was needed to navigate. The problem is that now, the renderer is not okay to do
a same-document navigation, so the main frame resource data is needed, but we
don't have it.

It's forbidden with PlzNavigate to request the main frame resource from the
renderer, so it will fail.

We can try several things from here:
 * Relaunch the navigation as if it was renderer-initiated  by writing
request.setCheckForBrowserSideNavigation(true). Nevertheless, there is a problem
when it's an history-navigation (they are always browser-initiated). The
PlzNavigate path through RenderFrameImpl::BeginNavigation doesn't allow
history-navigation.
 * Drop the navigation request.

[nasko, 2017/01/19 00:43:35]

How about JS executing "window.history.back()" or similar? Those are
renderer-initiated signals to the browser to do history navigation. Wouldn't it
be more correct to look like one of these?

[arthursonzogni, 2017/01/19 17:49:21]

For renderer-initiated history-navigation. The renderer send a
ViewHostMsg_GoToEntryAtOffset to the browser. Then it uses the browser-initiated
path.

+  if (request.resourceRequest().isSameDocumentNavigation())
+    return;
+
   startLoad(request, newLoadType, policy);
 }