PlzNavigate: inform the WebFrameClient that a form will be submitted

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

 /*
  * Copyright (C) 2007 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 98 matching lines @@
     m_progressItems.clear();
 
     m_totalPageAndResourceBytesToLoad = 0;
     m_totalBytesReceived = 0;
     m_progressValue = 0;
     m_lastNotifiedProgressValue = 0;
     m_lastNotifiedProgressTime = 0;
     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);
 }
 
 void ProgressTracker::progressCompleted()
 {
     ASSERT(m_frame->isLoading());
     m_frame->setIsLoading(false);
     sendFinalProgress();
@@ skipping 138 matching lines @@
         return;
 
     // Adjust the total expected bytes to account for any overage/underage.
     long long delta = item->bytesReceived - item->estimatedLength;
     m_totalPageAndResourceBytesToLoad += delta;
 
     m_progressItems.remove(identifier);
 }
 
 }