PlzNavigate: make all ResourceDispatcherHostTests pass

content/browser/loader/resource_dispatcher_host_unittest.cc

 // Copyright (c) 2012 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 <stddef.h>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/scoped_vector.h"
 #include "base/memory/shared_memory.h"
 #include "base/pickle.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/thread_task_runner_handle.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/cert_store_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
+#include "content/browser/frame_host/navigation_request_info.h"
 #include "content/browser/loader/cross_site_resource_handler.h"
 #include "content/browser/loader/detachable_resource_handler.h"
+#include "content/browser/loader/navigation_url_loader.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_loader.h"
 #include "content/browser/loader/resource_message_filter.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/common/appcache_interfaces.h"
 #include "content/common/child_process_host_impl.h"
+#include "content/common/navigation_params.h"
 #include "content/common/resource_messages.h"
 #include "content/common/ssl_status_serialization.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/resource_dispatcher_host_delegate.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/resource_throttle.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/process_type.h"
 #include "content/public/common/resource_response.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_renderer_host.h"
 #include "content/test/test_content_browser_client.h"
+#include "content/test/test_navigation_url_loader_delegate.h"
 #include "net/base/elements_upload_data_stream.h"
+#include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/request_priority.h"
 #include "net/base/test_data_directory.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/http/http_util.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/url_request/url_request_failed_job.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_job.h"
@@ skipping 794 matching lines @@
   }
 
   void set_default_cert_id(int default_cert_id) {
     default_cert_id_ = default_cert_id;
   }
 
  private:
   int default_cert_id_;
 };
 
+void CheckRequestCompleteErrorCode(const IPC::Message& message,
+                                   int expected_error_code) {
+  // Verify the expected error code was received.
+  int request_id;
+  int error_code;
+
+  ASSERT_EQ(ResourceMsg_RequestComplete::ID, message.type());
+
+  base::PickleIterator iter(message);
+  ASSERT_TRUE(IPC::ReadParam(&message, &iter, &request_id));
+  ASSERT_TRUE(IPC::ReadParam(&message, &iter, &error_code));
+  ASSERT_EQ(expected_error_code, error_code);
+}
+
 class ResourceDispatcherHostTest : public testing::TestWithParam<TestConfig>,
                                    public IPC::Sender {
  public:
   typedef ResourceDispatcherHostImpl::LoadInfo LoadInfo;
   typedef ResourceDispatcherHostImpl::LoadInfoMap LoadInfoMap;
 
   ResourceDispatcherHostTest()
       : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
         use_test_ssl_certificate_(false),
         old_factory_(NULL),
-        send_data_received_acks_(false) {
+        send_data_received_acks_(false),
+        auto_advance_(false) {
     browser_context_.reset(new TestBrowserContext());
     BrowserContext::EnsureResourceContextInitialized(browser_context_.get());
     base::RunLoop().RunUntilIdle();
     filter_ = MakeForwardingFilter();
     // TODO(cbentzel): Better way to get URLRequestContext?
     net::URLRequestContext* request_context =
         browser_context_->GetResourceContext()->GetRequestContext();
     job_factory_.reset(new TestURLRequestJobFactory(this));
     request_context->set_job_factory(job_factory_.get());
     request_context->set_network_delegate(&network_delegate_);
@@ skipping 209 matching lines @@
     DCHECK(!wait_for_request_complete_loop_);
     wait_for_request_complete_loop_.reset(new base::RunLoop);
     wait_for_request_complete_loop_->Run();
     wait_for_request_complete_loop_.reset();
   }
 
   void DeleteRenderFrame(const GlobalFrameRoutingId& global_routing_id) {
     host_.OnRenderFrameDeleted(global_routing_id);
   }
 
+  // Creates and drive a main resource request until completion. Then asserts

[mmenke, 2016/04/08 15:19:12]

nit:  "Create and drive" or "creates and drives".  Whichever you use should
agree in tense with the next sentence as well.

[clamy, 2016/04/08 16:03:17]

Done.

+  // that the expected_error_code has been emitted for the request.
+  void CompleteFailingMainResourceRequest(const GURL& url,
+                                          int expected_error_code) {
+    if (IsBrowserSideNavigationEnabled()) {
+      auto_advance_ = true;
+
+      // Make a navigation request.
+      TestNavigationURLLoaderDelegate delegate;
+      BeginNavigationParams begin_params(std::string(), net::LOAD_NORMAL, false,
+                                         false, REQUEST_CONTEXT_TYPE_LOCATION);

[mmenke, 2016/04/08 15:19:12]

I assume "REQUEST_CONTEXT_TYPE_LOCATION" is the general case?  None of the
RequestContextTypes are documented.  :(

[clamy, 2016/04/08 16:03:17]

I think so. It's the clicking on a link case.

+      CommonNavigationParams common_params;
+      common_params.url = url;
+      scoped_ptr<NavigationRequestInfo> request_info(new NavigationRequestInfo(
+          common_params, begin_params, url, url::Origin(url), true, false, -1,
+          scoped_refptr<ResourceRequestBody>()));
+      scoped_ptr<NavigationURLLoader> test_loader = NavigationURLLoader::Create(
+          browser_context_.get(), std::move(request_info), nullptr, &delegate);
+
+      // The navigation should fail with the error code of ERR_INVALID_RESPONSE.

[carlosk, 2016/04/05 15:28:46]

As the expected error code is an input parameter this comment seems incorrect.

[clamy, 2016/04/08 16:03:17]

Done. Indeed.

+      delegate.WaitForRequestFailed();
+      ASSERT_EQ(expected_error_code, delegate.net_error());
+      return;
+    }
+
+    MakeTestRequestWithResourceType(filter_.get(), 0, 1, url,
+                                    RESOURCE_TYPE_MAIN_FRAME);
+
+    // Flush all pending requests.
+    while (net::URLRequestTestJob::ProcessOnePendingMessage()) {
+    }
+    base::MessageLoop::current()->RunUntilIdle();
+
+    // Sorts out all the messages we saw by request.
+    ResourceIPCAccumulator::ClassifiedMessages msgs;
+    accum_.GetClassifiedMessages(&msgs);
+
+    // We should have gotten one RequestComplete message.
+    ASSERT_EQ(1U, msgs.size());
+    ASSERT_EQ(1U, msgs[0].size());
+    EXPECT_EQ(ResourceMsg_RequestComplete::ID, msgs[0][0].type());
+
+    // The RequestComplete message should have had the expected error code.
+    CheckRequestCompleteErrorCode(msgs[0][0], expected_error_code);
+  }
+
   scoped_ptr<LoadInfoTestRequestInfo> loader_test_request_info_;
   scoped_ptr<base::RunLoop> wait_for_request_create_loop_;
 
   content::TestBrowserThreadBundle thread_bundle_;
   scoped_ptr<TestBrowserContext> browser_context_;
   scoped_ptr<TestURLRequestJobFactory> job_factory_;
   scoped_ptr<WebContents> web_contents_;
   scoped_ptr<TestWebContentsObserver> web_contents_observer_;
   scoped_refptr<ForwardingFilter> filter_;
   scoped_refptr<TestFilterSpecifyingChild> web_contents_filter_;
   net::TestNetworkDelegate network_delegate_;
   ResourceDispatcherHostImpl host_;
   ResourceIPCAccumulator accum_;
   std::string response_headers_;
   std::string response_data_;
   bool use_test_ssl_certificate_;
   std::string scheme_;
   net::URLRequest::ProtocolFactory* old_factory_;
   bool send_data_received_acks_;
   std::set<int> child_ids_;
   scoped_ptr<base::RunLoop> wait_for_request_complete_loop_;
   RenderViewHostTestEnabler render_view_host_test_enabler_;
   MockCertStore mock_cert_store_;
+  bool auto_advance_;
 };
 
 void ResourceDispatcherHostTest::MakeTestRequest(int render_view_id,
                                                  int request_id,
                                                  const GURL& url) {
   MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
                                   url, RESOURCE_TYPE_SUB_RESOURCE);
 }
 
 void ResourceDispatcherHostTest::MakeTestRequestWithRenderFrame(
@@ skipping 89 matching lines @@
 void ResourceDispatcherHostTest::CompleteStartRequest(
     ResourceMessageFilter* filter,
     int request_id) {
   GlobalRequestID gid(filter->child_id(), request_id);
   net::URLRequest* req = host_.GetURLRequest(gid);
   EXPECT_TRUE(req);
   if (req)
     URLRequestTestDelayedStartJob::CompleteStart(req);
 }
 
-void CheckRequestCompleteErrorCode(const IPC::Message& message,
-                                   int expected_error_code) {
-  // Verify the expected error code was received.
-  int request_id;
-  int error_code;
-
-  ASSERT_EQ(ResourceMsg_RequestComplete::ID, message.type());
-
-  base::PickleIterator iter(message);
-  ASSERT_TRUE(IPC::ReadParam(&message, &iter, &request_id));
-  ASSERT_TRUE(IPC::ReadParam(&message, &iter, &error_code));
-  ASSERT_EQ(expected_error_code, error_code);
-}
-
 testing::AssertionResult ExtractInlinedChunkData(
     const IPC::Message& message,
     std::string* leading_chunk_data) {
   base::PickleIterator iter(message);
   int request_id;
   if (!IPC::ReadParam(&message, &iter, &request_id))
     return testing::AssertionFailure() << "Could not read request_id";
 
   std::vector<char> data;
   if (!IPC::ReadParam(&message, &iter, &data))
@@ skipping 1195 matching lines @@
 // Tests for crbug.com/31266 (Non-2xx + application/octet-stream).
 TEST_P(ResourceDispatcherHostTest, ForbiddenDownload) {
   std::string raw_headers("HTTP/1.1 403 Forbidden\n"
                           "Content-disposition: attachment; filename=blah\n"
                           "Content-type: application/octet-stream\n\n");
   std::string response_data("<html><title>Test One</title></html>");
   SetResponse(raw_headers, response_data);
 
   HandleScheme("http");
 
-  // Only MAIN_FRAMEs can trigger a download.
-  MakeTestRequestWithResourceType(filter_.get(), 0, 1, GURL("http:bla"),
-                                  RESOURCE_TYPE_MAIN_FRAME);
+  const int expected_error_code = net::ERR_INVALID_RESPONSE;
+  const GURL forbidden_download_url = GURL("http:bla");

[mmenke, 2016/04/08 15:19:12]

consts are pretty rare, unless you use kConstNamingStyle.  I'd just make thes
enon-const, to match the style of the std::strings above, but I'm happy, either
way.

[clamy, 2016/04/08 16:03:17]

Done.

 
-  // Flush all pending requests.
-  while (net::URLRequestTestJob::ProcessOnePendingMessage()) {}
-  base::MessageLoop::current()->RunUntilIdle();
-
-  // Sorts out all the messages we saw by request.
-  ResourceIPCAccumulator::ClassifiedMessages msgs;
-  accum_.GetClassifiedMessages(&msgs);
-
-  // We should have gotten one RequestComplete message.
-  ASSERT_EQ(1U, msgs.size());
-  ASSERT_EQ(1U, msgs[0].size());
-  EXPECT_EQ(ResourceMsg_RequestComplete::ID, msgs[0][0].type());
-
-  // The RequestComplete message should have had the error code of
-  // ERR_INVALID_RESPONSE.
-  CheckRequestCompleteErrorCode(msgs[0][0], net::ERR_INVALID_RESPONSE);
+  CompleteFailingMainResourceRequest(forbidden_download_url,
+                                     expected_error_code);
 }
 
 // Test for http://crbug.com/76202 .  We don't want to destroy a
 // download request prematurely when processing a cancellation from
 // the renderer.
 TEST_P(ResourceDispatcherHostTest, IgnoreCancelForDownloads) {
+  // PlzNavigate: A request that ends up being a download is a main resource
+  // request. Hence, it has been initiated by the browser and is not associated
+  // with a renderer. Therefore, it cannot be canceled by a renderer IPC.
+  if (IsBrowserSideNavigationEnabled())

[carlosk, 2016/04/05 15:28:46]

nit: use the SUCCEED() << "Message" operator (which is still a nop BTW).

[clamy, 2016/04/08 16:03:17]

Done.

+    return;
+
   EXPECT_EQ(0, host_.pending_requests());
 
   int render_view_id = 0;
   int request_id = 1;
 
   std::string raw_headers("HTTP\n"
                           "Content-disposition: attachment; filename=foo\n\n");
   std::string response_data("01234567890123456789\x01foobar");
 
   // Get past sniffing metrics in the MimeTypeResourceHandler.  Note that
@@ skipping 35 matching lines @@
   std::string raw_headers("HTTP\n"
                           "Content-disposition: attachment; filename=foo\n\n");
   std::string response_data("01234567890123456789\x01foobar");
   // Get past sniffing metrics.
   response_data.resize(1025, ' ');
 
   SetResponse(raw_headers, response_data);
   job_factory_->SetDelayedCompleteJobGeneration(true);
   HandleScheme("http");
 
-  MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
-                                  GURL("http://example.com/blah"),
-                                  RESOURCE_TYPE_MAIN_FRAME);
-  // Return some data so that the request is identified as a download
-  // and the proper resource handlers are created.
-  EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage());
+  const GURL download_url = GURL("http://example.com/blah");
 
-  // And now simulate a cancellation coming from the renderer.
-  ResourceHostMsg_CancelRequest msg(request_id);
-  host_.OnMessageReceived(msg, filter_.get());
+  if (IsBrowserSideNavigationEnabled()) {
+    // Create a NavigationRequest.
+    TestNavigationURLLoaderDelegate delegate;
+    BeginNavigationParams begin_params(std::string(), net::LOAD_NORMAL, false,
+                                       false, REQUEST_CONTEXT_TYPE_LOCATION);
+    CommonNavigationParams common_params;
+    common_params.url = download_url;
+    scoped_ptr<NavigationRequestInfo> request_info(new NavigationRequestInfo(
+        common_params, begin_params, download_url, url::Origin(download_url),
+        true, false, -1, scoped_refptr<ResourceRequestBody>()));
+    scoped_ptr<NavigationURLLoader> loader = NavigationURLLoader::Create(
+        browser_context_.get(), std::move(request_info), nullptr, &delegate);
 
-  // Since the request had already started processing as a download,
-  // the cancellation above should have been ignored and the request
-  // should still be alive.
-  EXPECT_EQ(1, host_.pending_requests());
+    // Wait until a response has been received and proceed with the response.
+    KickOffRequest();
 
-  // Cancelling by other methods shouldn't work either.
-  host_.CancelRequestsForProcess(render_view_id);
-  EXPECT_EQ(1, host_.pending_requests());
+    // Return some data so that the request is identified as a download
+    // and the proper resource handlers are created.
+    EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage());
+    base::MessageLoop::current()->RunUntilIdle();
 
-  // Cancelling by context should work.
-  host_.CancelRequestsForContext(filter_->resource_context());
-  EXPECT_EQ(0, host_.pending_requests());
+    EXPECT_EQ(delegate.net_error(), net::ERR_ABORTED);

[mmenke, 2016/04/08 15:19:12]

Think this requires a comment.  I assume this is because the navigation should
be aborted once the request is identified as a download (but the request itself
should continue).

[clamy, 2016/04/08 16:03:17]

Done.

+    EXPECT_EQ(1, host_.pending_requests());
+
+    // In PlzNavigate, the renderer cannot cancel the request directly.
+    // However, cancelling by context should work.
+    host_.CancelRequestsForContext(browser_context_->GetResourceContext());
+    EXPECT_EQ(0, host_.pending_requests());
+
+    base::MessageLoop::current()->RunUntilIdle();
+  } else {
+    MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
+                                    download_url, RESOURCE_TYPE_MAIN_FRAME);
+
+    // Return some data so that the request is identified as a download
+    // and the proper resource handlers are created.
+    EXPECT_TRUE(net::URLRequestTestJob::ProcessOnePendingMessage());
+
+    // And now simulate a cancellation coming from the renderer.
+    ResourceHostMsg_CancelRequest msg(request_id);
+    host_.OnMessageReceived(msg, filter_.get());
+
+    // Since the request had already started processing as a download,
+    // the cancellation above should have been ignored and the request
+    // should still be alive.
+    EXPECT_EQ(1, host_.pending_requests());
+
+    // Cancelling by other methods shouldn't work either.
+    host_.CancelRequestsForProcess(render_view_id);
+    EXPECT_EQ(1, host_.pending_requests());
+
+    // Cancelling by context should work.
+    host_.CancelRequestsForContext(filter_->resource_context());
+    EXPECT_EQ(0, host_.pending_requests());
+  }
 }
 
 TEST_P(ResourceDispatcherHostTest, CancelRequestsForContextDetached) {
   EXPECT_EQ(0, host_.pending_requests());
 
   int render_view_id = 0;
   int request_id = 1;
 
   MakeTestRequestWithResourceType(filter_.get(), render_view_id, request_id,
                                   net::URLRequestTestJob::test_url_4(),
@@ skipping 572 matching lines @@
   ASSERT_EQ(2U, msgs.size());
   EXPECT_EQ(ResourceMsg_ReceivedRedirect::ID, msgs[0][0].type());
   CheckSuccessfulRequest(msgs[1], kResponseBody);
 }
 
 TEST_P(ResourceDispatcherHostTest, UnknownURLScheme) {
   EXPECT_EQ(0, host_.pending_requests());
 
   HandleScheme("http");
 
-  MakeTestRequestWithResourceType(filter_.get(), 0, 1, GURL("foo://bar"),
-                                  RESOURCE_TYPE_MAIN_FRAME);
+  const GURL invalid_sheme_url = GURL("foo://bar");
+  const int expected_error_code = net::ERR_UNKNOWN_URL_SCHEME;
 
-  // Flush all pending requests.
-  while (net::URLRequestTestJob::ProcessOnePendingMessage()) {}
-
-  // Sort all the messages we saw by request.
-  ResourceIPCAccumulator::ClassifiedMessages msgs;
-  accum_.GetClassifiedMessages(&msgs);
-
-  // We should have gotten one RequestComplete message.
-  ASSERT_EQ(1U, msgs[0].size());
-  EXPECT_EQ(ResourceMsg_RequestComplete::ID, msgs[0][0].type());
-
-  // The RequestComplete message should have the error code of
-  // ERR_UNKNOWN_URL_SCHEME.
-  CheckRequestCompleteErrorCode(msgs[0][0], net::ERR_UNKNOWN_URL_SCHEME);
+  CompleteFailingMainResourceRequest(invalid_sheme_url, expected_error_code);
 }
 
 TEST_P(ResourceDispatcherHostTest, DataReceivedACKs) {
   EXPECT_EQ(0, host_.pending_requests());
 
   SendDataReceivedACKs(true);
 
   HandleScheme("big-job");
   MakeTestRequest(0, 1, GURL("big-job:0123456789,1000000"));
 
@@ skipping 590 matching lines @@
     if (delay_start_) {
       return new URLRequestTestDelayedStartJob(request, network_delegate);
     } else if (delay_complete_) {
       return new URLRequestTestDelayedCompletionJob(request,
                                                     network_delegate);
     } else if (network_start_notification_) {
       return new URLRequestTestDelayedNetworkJob(request, network_delegate);
     } else if (scheme == "big-job") {
       return new URLRequestBigJob(request, network_delegate);
     } else {
-      return new net::URLRequestTestJob(request, network_delegate);
+      return new net::URLRequestTestJob(request, network_delegate,
+                                        test_fixture_->auto_advance_);
     }
   } else {
     if (delay_start_) {
       return new URLRequestTestDelayedStartJob(
           request, network_delegate,
           test_fixture_->response_headers_, test_fixture_->response_data_,
           false);
     } else if (delay_complete_) {
       return new URLRequestTestDelayedCompletionJob(
           request, network_delegate,
           test_fixture_->response_headers_, test_fixture_->response_data_,
           false);
     } else if (test_fixture_->use_test_ssl_certificate_) {
       return new TestHTTPSURLRequestJob(request, network_delegate,
                                         test_fixture_->response_headers_,
                                         test_fixture_->response_data_, false);
     } else {
       return new net::URLRequestTestJob(
-          request, network_delegate,
-          test_fixture_->response_headers_, test_fixture_->response_data_,
-          false);
+          request, network_delegate, test_fixture_->response_headers_,
+          test_fixture_->response_data_, test_fixture_->auto_advance_);
     }
   }
 }
 
 net::URLRequestJob* TestURLRequestJobFactory::MaybeInterceptRedirect(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate,
     const GURL& location) const {
   return nullptr;
 }
 
 net::URLRequestJob* TestURLRequestJobFactory::MaybeInterceptResponse(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate) const {
   return nullptr;
 }
 
 INSTANTIATE_TEST_CASE_P(
     ResourceDispatcherHostTests,
     ResourceDispatcherHostTest,
     testing::Values(TestConfig::kDefault,
                     TestConfig::kOptimizeIPCForSmallResourceEnabled));
 
 }  // namespace content