PlzNavigate: Call NavigationHandle::WillProcessResponse for 204/205s

content/browser/loader/navigation_resource_handler.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/loader/navigation_resource_handler.h"
 
 #include <memory>
 
 #include "base/logging.h"
 #include "content/browser/loader/navigation_url_loader_impl_core.h"
@@ skipping 76 matching lines @@
   *defer = true;
   return true;
 }
 
 bool NavigationResourceHandler::OnResponseStarted(ResourceResponse* response,
                                                   bool* defer) {
   DCHECK(core_);
 
   ResourceRequestInfoImpl* info = GetRequestInfo();
 
-  // If the MimeTypeResourceHandler intercepted this request and converted it
-  // into a download, it will still call OnResponseStarted and immediately
-  // cancel. Ignore the call; OnReadCompleted will happen shortly.
-  //
-  // TODO(davidben): Move the dispatch out of MimeTypeResourceHandler. Perhaps
-  // all the way to the UI thread. Downloads, user certificates, etc., should be
-  // dispatched at the navigation layer.
-  if (info->IsDownload())
-    return true;
-
   StreamContext* stream_context =
       GetStreamContextForResourceContext(info->GetContext());
   writer_.InitializeStream(stream_context->registry(),
                            request()->url().GetOrigin());
 
   NetLogObserver::PopulateResponseInfo(request(), response);
 
   std::unique_ptr<NavigationData> cloned_data;
   if (resource_dispatcher_host_delegate_) {
     // Ask the embedder for a NavigationData instance.
@@ skipping 14 matching lines @@
   core_->NotifyResponseStarted(response, writer_.stream()->CreateHandle(),
                                ssl_status, std::move(cloned_data),
                                info->GetGlobalRequestID(), info->IsDownload(),
                                info->is_stream());
   // Don't defer stream based requests. This includes requests initiated via
   // mime type sniffing, etc.
   // TODO(ananta)
   // Make sure that the requests go through the throttle checks. Currently this
   // does not work as the InterceptingResourceHandler is above us and hence it
   // does not expect the old handler to defer the request.
-  if (!info->is_stream())
+  // TODO(clamy): check if we should also make the downloads wait on the
+  // NavigationThrottle checks be performed.

[nasko, 2016/12/22 18:16:09]

Why wouldn't we want downloads to go through the throttle checks? Explicit
downloads won't hit them, but a navigation that turns into a download probably
should. Does anything break if we defer downloads here?

[clamy, 2016/12/23 16:54:35]

That doesn't work because of the InterceptingResourceHandler as mentionned
above. This will be fixed when I split this in two, with the part doing the
checks moved before the InterceptingResourceHandler. Clarified the TODO.

+  if (!info->is_stream() && !info->IsDownload())
     *defer = true;
+
   return true;
 }
 
 bool NavigationResourceHandler::OnWillStart(const GURL& url, bool* defer) {
   return true;
 }
 
 bool NavigationResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
                                            int* buf_size,
                                            int min_size) {
@@ skipping 27 matching lines @@
   NOTREACHED();
 }
 
 void NavigationResourceHandler::DetachFromCore() {
   DCHECK(core_);
   core_->set_resource_handler(nullptr);
   core_ = nullptr;
 }
 
 }  // namespace content