PlzNavigate: inform the WebFrameClient that a form will be submitted

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

Index: third_party/WebKit/Source/core/loader/ProgressTracker.cpp
diff --git a/third_party/WebKit/Source/core/loader/ProgressTracker.cpp b/third_party/WebKit/Source/core/loader/ProgressTracker.cpp
index d36261a18510f718c7e9223801dbab0c23e7886a..f8523408baea53cc1bae2b47a433622b3c11d07d 100644
--- a/third_party/WebKit/Source/core/loader/ProgressTracker.cpp
+++ b/third_party/WebKit/Source/core/loader/ProgressTracker.cpp
@@ -116,12 +116,13 @@ void ProgressTracker::reset()
     m_finalProgressChangedSent = false;
 }
 
-void ProgressTracker::progressStarted()
+void ProgressTracker::progressStarted(bool informClient)
 {
     if (!m_frame->isLoading()) {
         reset();
         m_progressValue = initialProgressValue;
-        m_frame->loader().client()->didStartLoading(NavigationToDifferentDocument);
+        if (informClient)
+            m_frame->loader().client()->didStartLoading(NavigationToDifferentDocument);

[dcheng, 2015/12/11 17:14:06]

We absolutely can not special-case mandoline here. We already have trouble with
varying the order of loading-related callbacks. Allowing callbacks to optionally
occur just makes it even worse.

creis@ added some history navigation code that takes advantage of
NavigationPolicyHandledByClient recently, and as far as I know, it Just Worked.
How does //content handle this so that it doesn't need a special case here?

[clamy, 2015/12/14 15:47:18]

The issue here is the combination of what content clients expect + PlzNavigate
loading decisions + the fact that we allow concurrent navigations in a FTN
(something that Mandoline people seemed not to bother about when they landed
this). Roughly, the browser can decide (for valid reasons) not to proceed with
the renderer-initiated navigations. So in the browser, we should not consider
that our frame is loading during that time (or the page may end up in a
perpetually loading state). But if we inform the client here, then the renderer
sends a DidStartLoading message to the browser. The simple fix would seem to be
to ignore that message, which means ignoring all DidStartLoading IPCs from the
renderer since we can't properly distinguish that case.

But, doing that prevents us from reproducing the exact behavior with which we
call WCO::DidStartLoading when PlzNavigate is enabled. This is because the
firing of WCO::DidStartLoading is heavily dependent on the notion of RFH, and it
also depends on the presence of a pending RFh in the FTN (something a bit hard
to match in PlzNavigate before we actually tell the frame to commit the
navigation). We got away until then by reproducing part of this logic when the
frame is about to commit the navigation/the error page. However, us ignoring the
signals from the renderer effectively prevent us from firing
WCO::DidStartLoading based on what the renderer about to commit is telling us.

To sum it up, ideally:
1) with PlzNavigate enabled we should ignore all DidStartLoading IPCs from the
renderer except for in-page navigations (since for all other navigations, the
browser decides whether it should proceed or not).
2) we should have a simpler model for deciding when to fire WCO::DidStartLoading
so that it can work with PlzNavigate as well.

I will be looking at 2, but this requires a rewrite of several tests that rely
on the current behavior for WCO::DidStartLoading. If that goes through, I can
then have 1 work, and we won't need to special case here.

     }
     m_frame->setIsLoading(true);
     InspectorInstrumentation::frameStartedLoading(m_frame);