Cleanup blink-side PlzNavigate logic

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 3ed4a1794a994fdc9aa92333b023463f9440782b..8dc555901d9481c85e935585825ce849d44c6dc1 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -3306,11 +3306,6 @@ void RenderFrameImpl::willSendSubmitEvent(const blink::WebFormElement& form) {
 }
 
 void RenderFrameImpl::willSubmitForm(const blink::WebFormElement& form) {
-  // With PlzNavigate-enabled, this will be called before a DataSource has been

[Nate Chapin, 2017/02/24 23:42:05]

willSubmitForm() is always called with a live provisionalDataSource() now.

-  // set-up.
-  // TODO(clamy): make sure the internal state is properly updated at some
-  // point in the navigation.
-  if (!IsBrowserSideNavigationEnabled() && !!frame_->provisionalDataSource()) {
     DocumentState* document_state =
         DocumentState::FromDataSource(frame_->provisionalDataSource());
     NavigationStateImpl* navigation_state =
@@ -3328,7 +3323,6 @@ void RenderFrameImpl::willSubmitForm(const blink::WebFormElement& form) {
     internal_data->set_searchable_form_url(web_searchable_form_data.url());
     internal_data->set_searchable_form_encoding(
         web_searchable_form_data.encoding().utf8());
-  }
 
   for (auto& observer : observers_)
     observer.WillSubmitForm(form);
@@ -5214,9 +5208,20 @@ void RenderFrameImpl::OnFailedNavigation(
 
   // For renderer initiated navigations, we send out a didFailProvisionalLoad()
   // notification.
-  if (request_params.nav_entry_id == 0)
-    didFailProvisionalLoad(frame_, error, blink::WebStandardCommit);
-  LoadNavigationErrorPage(failed_request, error, replace, history_entry.get());
+  bool had_provisional_data_source = frame_->provisionalDataSource();
+  if (request_params.nav_entry_id == 0) {
+    didFailProvisionalLoad(
+        frame_, error,
+        replace ? blink::WebHistoryInertCommit : blink::WebStandardCommit);
+  }
+  // If we didn't call didFailProvisionalLoad or there wasn't a
+  // provisionalDataSource(), LoadNavigationErrorPage wasn't called, so do it
+  // now.
+  if (request_params.nav_entry_id != 0 || !had_provisional_data_source) {

[Nate Chapin, 2017/02/24 23:42:05]

This block guarantees that LoadNavigationErrorPage() is called exactly once,
whether directly here, or via didFailProvisionalLoad().

+    LoadNavigationErrorPage(failed_request, error, replace,
+                            history_entry.get());
+  }
+
   browser_side_navigation_pending_ = false;
 }
 
@@ -5301,7 +5306,7 @@ WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation(
                 GetWebURLRequestHeaders(info.urlRequest), referrer,
                 info.defaultPolicy, info.replacesCurrentHistoryItem, true);
         // Suppress the load in Blink but mark the frame as loading.
-        return blink::WebNavigationPolicyHandledByClient;
+        return blink::WebNavigationPolicyHandledByClientForInitialHistory;

[Nate Chapin, 2017/02/24 23:42:05]

The plumbing for this case and the behavior it needs from blink is sufficiently
different from PlzNavigate that I split the enum values. I'd like to merge them
once I understand this path better (see TODO in FrameLoader).

[clamy, 2017/03/02 14:37:18]

Acknowledged.

       } else {
         // Client redirects during an initial history load should attempt to
         // cancel the history navigation.  They will create a provisional