Classify navigations without page id in parallel to the existing classifier.

content/browser/frame_host/navigator_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/navigator_impl.h"
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ skipping 155 matching lines @@
 
 void NavigatorImpl::DidFailProvisionalLoadWithError(
     RenderFrameHostImpl* render_frame_host,
     const FrameHostMsg_DidFailProvisionalLoadWithError_Params& params) {
   VLOG(1) << "Failed Provisional Load: " << params.url.possibly_invalid_spec()
           << ", error_code: " << params.error_code
           << ", error_description: " << params.error_description
           << ", showing_repost_interstitial: " <<
             params.showing_repost_interstitial
           << ", frame_id: " << render_frame_host->GetRoutingID();
+  if (controller_->GetPendingEntry() && params.nav_entry_id != 0 &&
+      controller_->GetPendingEntry()->GetUniqueID() != params.nav_entry_id) {
+    // If this failure is browser-initiated but isn't that of the pending entry,
+    // ignore it. This can happen if there is an outstanding request that never
+    // committed, and then a user makes a new request. In that case the new
+    // request will be the pending entry, and the attempt to load the new
+    // request will cause a failure message from the old request.
+    return;

[Charlie Reis, 2015/04/10 23:54:21]

I'm a little surprised this didn't cause issues with the Web Navigation API,
which has pretty specific expectations for hearing about failures.  Maybe it has
already given up on the outstanding request at the time the new pending entry
was created?

(This strikes me as another place that might cause regressions, so it might
benefit from landing in a separate CL.)

[Avi (use Gerrit), 2015/04/13 22:42:48]

This was full of surprise to me, and I was kinda shocked we didn't already
handle this case.
I was thinking of doing that, but the problem is we can't with the current
system.

- This particular fix relies on nav_entry_id, which of course only exists in
this CL.
- A general fix can't be done with page_id. Because page_id is assigned by the
renderer, the browser has no general cookie value it can use to match up the
failures from the renderer with the pending entry. The fact that we round-trip a
cookie is the only reason we _can_ now catch this.

But yeah, if you can think of a good way to fix this independently, I would
definitely like to land this separately.

+  }
+
   GURL validated_url(params.url);
   RenderProcessHost* render_process_host = render_frame_host->GetProcess();
   render_process_host->FilterURL(false, &validated_url);
 
   if (net::ERR_ABORTED == params.error_code) {
     // EVIL HACK ALERT! Ignore failed loads when we're showing interstitials.
     // This means that the interstitial won't be torn down properly, which is
     // bad. But if we have an interstitial, go back to another tab type, and
     // then load the same interstitial again, we could end up getting the first
     // interstitial's "failed" message (as a result of the cancel) when we're on
@@ skipping 705 matching lines @@
         "Navigation.TimeToCommit_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_commit);
     UMA_HISTOGRAM_TIMES(
         "Navigation.TimeToURLJobStart_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_network);
   }
   navigation_data_.reset();
 }
 
 }  // namespace content