Fix NetErrorTabHelper with PlzNavigate.

content/browser/frame_host/navigation_handle_impl.cc

 // Copyright 2015 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/navigation_handle_impl.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "content/browser/browsing_data/clear_site_data_throttle.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/navigator_delegate.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/common/frame_messages.h"
 #include "content/common/resource_request_body_impl.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/content_client.h"
+#include "content/public/common/url_constants.h"
 #include "net/url_request/redirect_info.h"
 #include "url/gurl.h"
 #include "url/url_constants.h"
 
 namespace content {
 
 namespace {
 
 void UpdateThrottleCheckResult(
     NavigationThrottle::ThrottleCheckResult* to_update,
@@ skipping 423 matching lines @@
   DCHECK(!render_frame_host_ || render_frame_host_ == render_frame_host);
   DCHECK_EQ(frame_tree_node_, render_frame_host->frame_tree_node());
   CHECK_EQ(url_, params.url);
 
   method_ = params.method;
   has_user_gesture_ = (params.gesture == NavigationGestureUser);
   transition_ = params.transition;
   render_frame_host_ = render_frame_host;
   is_same_page_ = same_page;
 
-  state_ = net_error_code_ == net::OK ? DID_COMMIT : DID_COMMIT_ERROR_PAGE;
+  // If an error page reloads, net_error_code might be 200 but we still want to

[clamy, 2016/09/19 11:46:44]

nit: no "we" in comments?

[jam, 2016/09/19 14:19:40]

I realize some people think it's unambiguous in some situations, but I think in
this case it's not.

[clamy, 2016/09/19 15:27:11]

Acknowledged.

+  // count it as an error page.
+  if (params.base_url.spec() == kUnreachableWebDataURL ||

[clamy, 2016/09/19 11:46:44]

When we reload the error page, do we know at the start of the navigation that
we're loading kUnreachableWebDataURL, or just at commit time? If at the start,
should we modify IsErrorPage so that it returns true also when we're trying to
load kUnreachableWebDataURL?

[jam, 2016/09/19 14:19:40]

It's not known at the start (because the url is the what the user typed). Only
when the commit IPC comes does it have the base url to identify it as an error
page. 

[clamy, 2016/09/19 15:27:11]

Acknowledged.

+      net_error_code_ != net::OK) {
+    state_ = DID_COMMIT_ERROR_PAGE;
+  } else {
+    state_ = DID_COMMIT;
+  }
 }
 
 NavigationThrottle::ThrottleCheckResult
 NavigationHandleImpl::CheckWillStartRequest() {
   DCHECK(state_ == WILL_SEND_REQUEST || state_ == DEFERRING_START);
   DCHECK(state_ != WILL_SEND_REQUEST || next_index_ == 0);
   DCHECK(state_ != DEFERRING_START || next_index_ != 0);
   for (size_t i = next_index_; i < throttles_.size(); ++i) {
     NavigationThrottle::ThrottleCheckResult result =
         throttles_[i]->WillStartRequest();
@@ skipping 117 matching lines @@
     throttles_to_register.push_back(std::move(clear_site_data_throttle));
 
   if (throttles_to_register.size() > 0) {
     throttles_.insert(throttles_.begin(), throttles_to_register.begin(),
                       throttles_to_register.end());
     throttles_to_register.weak_clear();
   }
 }
 
 }  // namespace content