PlzNavigate: don't discard pending entry in DidFailProvisionalLoad

content/browser/frame_host/render_frame_host_impl.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/render_frame_host_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 882 matching lines @@
     const base::TimeTicks& navigation_start) {
   frame_tree_node_->navigator()->DidStartProvisionalLoad(this, url,
                                                          navigation_start);
 }
 
 void RenderFrameHostImpl::OnDidFailProvisionalLoadWithError(
     const FrameHostMsg_DidFailProvisionalLoadWithError_Params& params) {
   if (!IsBrowserSideNavigationEnabled() && navigation_handle_) {
     navigation_handle_->set_net_error_code(
         static_cast<net::Error>(params.error_code));
+  } else {
+    // The renderer should not be sending this message unless asked to commit
+    // an error page.
+    // TODO(clamy): Stop sending DidFailProvisionalLoad IPCs at all when enough
+    // observers have moved to DidFinishNavigation.
+    if (!navigation_handle_ ||
+        navigation_handle_->GetNetErrorCode() == net::OK) {
+      bad_message::ReceivedBadMessage(
+          GetProcess(), bad_message::RFH_UNEXPECTED_FAIL_PROVISIONAL_LOAD);

[Charlie Reis, 2016/04/11 21:10:57]

The conditions surrounding this will make it hard to diagnose if we see it fail
in practice.  We'll get reports if (1) there's no handle (whether we're in
PlzNavigate or not) or (2) we're in PlzNavigate and the handle says net::OK. 
Those would be very different problems, so maybe we should have separate codes
for them.

That would also let us restructure this method to make it easier to read. 
Something like:

if (!navigation_handle_) {
  ReceivedBadMessage(RFH_FAIL_PROVISIONAL_LOAD_NO_HANDLE)
  return;
}

if (PlzNavigate && net::OK) {
  ReceivedBadMessage(RFH_FAIL_PROVISIONAL_LOAD_NO_ERROR)
  return;
}

if (!PlzNavigate)
  set_net_error_code...
...DidFailProvisionalLoadWithError...

[clamy, 2016/04/12 14:08:01]

Done. It was actually hard enough to read that I though it would only trigger
when PlzNavigate was enabled.

+      return;
+    }
   }
   frame_tree_node_->navigator()->DidFailProvisionalLoadWithError(this, params);
 }
 
 void RenderFrameHostImpl::OnDidFailLoadWithError(
     const GURL& url,
     int error_code,
     const base::string16& error_description,
     bool was_ignored_by_handler) {
   GURL validated_url(url);
@@ skipping 1828 matching lines @@
 }
 
 void RenderFrameHostImpl::CreateWebBluetoothService(
     blink::mojom::WebBluetoothServiceRequest request) {
   DCHECK(!web_bluetooth_service_);
   web_bluetooth_service_.reset(
       new WebBluetoothServiceImpl(this, std::move(request)));
 }
 
 }  // namespace content