PlzNavigate: properly update NavigationHandle for 204/205 and download responses

content/browser/frame_host/navigation_request.cc

 // Copyright 2014 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_request.h"
 
 #include <utility>
 
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
@@ skipping 357 matching lines @@
       common_params_.url, common_params_.method, common_params_.referrer.url,
       false, response->head.headers, response->head.connection_info,
       base::Bind(&NavigationRequest::OnRedirectChecksComplete,
                  base::Unretained(this)));
 }
 
 void NavigationRequest::OnResponseStarted(
     const scoped_refptr<ResourceResponse>& response,
     std::unique_ptr<StreamHandle> body,
     const SSLStatus& ssl_status,
-    std::unique_ptr<NavigationData> navigation_data) {
+    std::unique_ptr<NavigationData> navigation_data,
+    bool is_download) {
   DCHECK(state_ == STARTED);
   state_ = RESPONSE_STARTED;
 
-  // HTTP 204 (No Content) and HTTP 205 (Reset Content) responses should not
-  // commit; they leave the frame showing the previous page.
   DCHECK(response);
-  if (response->head.headers.get() &&
-      (response->head.headers->response_code() == 204 ||
-       response->head.headers->response_code() == 205)) {
-    frame_tree_node_->navigator()->DiscardPendingEntryIfNeeded(
-        navigation_handle_.get());
-    frame_tree_node_->ResetNavigationRequest(false);
-    return;
-  }
 
   // Update the service worker params of the request params.
   bool did_create_service_worker_host =
       navigation_handle_->service_worker_handle() &&
       navigation_handle_->service_worker_handle()
               ->service_worker_provider_host_id() !=
           kInvalidServiceWorkerProviderId;
   request_params_.service_worker_provider_id =
       did_create_service_worker_host
           ? navigation_handle_->service_worker_handle()
@@ skipping 23 matching lines @@
   }
 
   if (navigation_data)
     navigation_handle_->set_navigation_data(std::move(navigation_data));
 
   // Store the response and the StreamHandle until checks have been processed.
   response_ = response;
   body_ = std::move(body);
 
   // Check if the navigation should be allowed to proceed.
-  // TODO(clamy): pass the right values for request_id, is_download and
-  // is_stream.
+  // TODO(clamy): pass the right values for request_id and is_stream.
   navigation_handle_->WillProcessResponse(
       render_frame_host, response->head.headers.get(),
       response->head.connection_info, ssl_status, GlobalRequestID(),
-      common_params_.should_replace_current_entry, false, false,
+      common_params_.should_replace_current_entry, is_download, false,

[clamy, 2016/11/25 10:46:00]

Could we pass the values for is_stream and GlobalRequestID() while we're at it?
The files to modify are the same.

[yzshen1, 2016/11/28 22:03:59]

Done.

       base::Closure(),
       base::Bind(&NavigationRequest::OnWillProcessResponseChecksComplete,
                  base::Unretained(this)));
 }
 
 void NavigationRequest::OnRequestFailed(bool has_stale_copy_in_cache,
                                         int net_error) {
   DCHECK(state_ == STARTED);
   state_ = FAILED;
   navigation_handle_->set_net_error_code(static_cast<net::Error>(net_error));
@@ skipping 109 matching lines @@
   }
 
   loader_->FollowRedirect();
 }
 
 void NavigationRequest::OnWillProcessResponseChecksComplete(
     NavigationThrottle::ThrottleCheckResult result) {
   CHECK(result != NavigationThrottle::DEFER);
 
   // Abort the request if needed. This will destroy the NavigationRequest.
-  if (result == NavigationThrottle::CANCEL_AND_IGNORE ||
-      result == NavigationThrottle::CANCEL) {
-    // TODO(clamy): distinguish between CANCEL and CANCEL_AND_IGNORE.
+  if (result == NavigationThrottle::CANCEL_AND_IGNORE) {
+    frame_tree_node_->navigator()->DiscardPendingEntryIfNeeded(
+        navigation_handle_.get());
     frame_tree_node_->ResetNavigationRequest(false);
     return;
   }
+  if (result == NavigationThrottle::CANCEL) {
+    frame_tree_node_->ResetNavigationRequest(false);
+    return;
+  }
 
   // Have the processing of the response resume in the network stack.
   loader_->ProceedWithResponse();
 
   CommitNavigation();
 
   // DO NOT ADD CODE after this. The previous call to CommitNavigation caused
   // the destruction of the NavigationRequest.
 }
 
@@ skipping 14 matching lines @@
   DCHECK_EQ(request_params_.has_user_gesture, begin_params_.has_user_gesture);
 
   render_frame_host->CommitNavigation(response_.get(), std::move(body_),
                                       common_params_, request_params_,
                                       is_view_source_);
 
   frame_tree_node_->ResetNavigationRequest(true);
 }
 
 }  // namespace content