Clean up NavigationSimulator::FailWithResponseHeaders

This change removes all calls from NavigationSimulatorImpl to
NavigationRequest::set_response_headers_from_testing. Those calls are
not needed anymore, since https://crrev.com/c/2294801 added a method
NavigationSimulator::SetResponseHeaders which allows to mock response
headers going through the URLLoader.

The function NavigationSimulator::FailWithResponseHeaders, which was
relying on NavigationRequest::set_response_headers_from_testing, turn
out not to be needed anymore and has been removed in this CL.

Change-Id: I7ab18e0c2306aab97f4d04ad655fec5aa050f43c
Bug: 1094909
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310416
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Reviewed-by: John Delaney <johnidel@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Tim Schumann <tschumann@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804701}

chrome/browser/sync/sync_encryption_keys_tab_helper_unittest.cc

@@ -68,9 +68,8 @@ TEST_F(SyncEncryptionKeysTabHelperTest, ShouldNotExposeMojoApiIfNavigatedAway) {
 
 TEST_F(SyncEncryptionKeysTabHelperTest,
        ShouldNotExposeMojoApiIfNavigationFailed) {
-  web_contents_tester()->NavigateAndFail(
-      GaiaUrls::GetInstance()->gaia_url(), net::ERR_ABORTED,
-      base::MakeRefCounted<net::HttpResponseHeaders>("some_headers"));
+  web_contents_tester()->NavigateAndFail(GaiaUrls::GetInstance()->gaia_url(),
+                                         net::ERR_ABORTED);
   EXPECT_THAT(frame_receiver_set(), IsNull());
 }
 
@@ -78,9 +77,8 @@ TEST_F(SyncEncryptionKeysTabHelperTest,
        ShouldNotExposeMojoApiIfNavigatedAwayToErrorPage) {
   web_contents_tester()->NavigateAndCommit(GaiaUrls::GetInstance()->gaia_url());
   ASSERT_THAT(frame_receiver_set(), NotNull());
-  web_contents_tester()->NavigateAndFail(
-      GURL("http://page.com"), net::ERR_ABORTED,
-      base::MakeRefCounted<net::HttpResponseHeaders>("some_headers"));
+  web_contents_tester()->NavigateAndFail(GURL("http://page.com"),
+                                         net::ERR_ABORTED);
   EXPECT_THAT(frame_receiver_set(), IsNull());
 }
 

components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc

@@ -279,9 +279,7 @@ TEST_F(MetricsWebContentsObserverTest,
        DISABLED_MainFrameNavigationInternalAbort) {
   auto navigation = content::NavigationSimulator::CreateBrowserInitiated(
       GURL(kDefaultTestUrl), web_contents());
-  navigation->FailWithResponseHeaders(
-      net::ERR_ABORTED,
-      base::MakeRefCounted<net::HttpResponseHeaders>("some_headers"));
+  navigation->Fail(net::ERR_ABORTED);
   ASSERT_EQ(1u, observed_aborted_urls().size());
   ASSERT_EQ(kDefaultTestUrl, observed_aborted_urls().front().spec());
 }

content/public/test/navigation_simulator.h

@@ -203,12 +203,6 @@ class NavigationSimulator {
   // Note: this is only valid for renderer-initiated navigations.
   virtual void AbortFromRenderer() = 0;
 
-  // Simulates the navigation failing with the error code |error_code| and
-  // response headers |response_headers|.
-  virtual void FailWithResponseHeaders(
-      int error_code,
-      scoped_refptr<net::HttpResponseHeaders> response_headers) = 0;
-
   // Simulates the navigation failing with the error code |error_code|.
   // IMPORTANT NOTE: This is simulating a network connection error and implies
   // we do not get a response. Error codes like 204 are not properly managed.

content/public/test/web_contents_tester.h

@@ -92,11 +92,8 @@ class WebContentsTester {
       ui::PageTransition transition = ui::PAGE_TRANSITION_LINK) = 0;
 
   // Creates a pending navigation to the given URL with the default parameters
-  // and then aborts it with the given |error_code| and |response_headers|.
-  virtual void NavigateAndFail(
-      const GURL& url,
-      int error_code,
-      scoped_refptr<net::HttpResponseHeaders> response_headers) = 0;
+  // and then aborts it with the given |error_code|.
+  virtual void NavigateAndFail(const GURL& url, int error_code) = 0;
 
   // Sets the loading state to the given value.
   virtual void TestSetIsLoading(bool value) = 0;

content/test/navigation_simulator_impl.cc

@@ -524,6 +524,11 @@ void NavigationSimulatorImpl::ReadyToCommit() {
     }
   }
 
+  if (!response_headers_) {
+    response_headers_ =
+        base::MakeRefCounted<net::HttpResponseHeaders>(std::string());
+  }
+  response_headers_->SetHeader("Content-Type", contents_mime_type_);
   PrepareCompleteCallbackOnRequest();
   if (frame_tree_node_->navigation_request()) {
     static_cast<TestRenderFrameHost*>(frame_tree_node_->current_frame_host())
@@ -603,13 +608,6 @@ void NavigationSimulatorImpl::Commit() {
     browser_interface_broker_receiver_.reset();
   }
 
-  if (request_) {
-    scoped_refptr<net::HttpResponseHeaders> response_headers =
-        new net::HttpResponseHeaders(std::string());
-    response_headers->SetHeader("Content-Type", contents_mime_type_);
-    request_->set_response_headers_for_testing(response_headers);
-  }
-
   auto params = BuildDidCommitProvisionalLoadParams(
       same_document_ /* same_document */, false /* failed_navigation */);
   render_frame_host_->SimulateCommitProcessed(
@@ -666,9 +664,7 @@ void NavigationSimulatorImpl::AbortFromRenderer() {
   CHECK_EQ(1, num_did_finish_navigation_called_);
 }
 
-void NavigationSimulatorImpl::FailWithResponseHeaders(
-    int error_code,
-    scoped_refptr<net::HttpResponseHeaders> response_headers) {
+void NavigationSimulatorImpl::Fail(int error_code) {
   CHECK_LE(state_, STARTED) << "NavigationSimulatorImpl::Fail can only be "
                                "called once, and cannot be called after "
                                "NavigationSimulatorImpl::ReadyToCommit";
@@ -680,9 +676,6 @@ void NavigationSimulatorImpl::FailWithResponseHeaders(
   if (state_ == INITIALIZATION)
     Start();
 
-  CHECK(!request_->GetResponseHeaders());
-  request_->set_response_headers_for_testing(response_headers);
-
   state_ = FAILED;
 
   PrepareCompleteCallbackOnRequest();
@@ -705,10 +698,6 @@ void NavigationSimulatorImpl::FailWithResponseHeaders(
   std::move(complete_closure).Run();
 }
 
-void NavigationSimulatorImpl::Fail(int error_code) {
-  FailWithResponseHeaders(error_code, nullptr);
-}
-
 void NavigationSimulatorImpl::FailComplete(int error_code) {
   bool should_result_in_error_page = error_code != net::ERR_ABORTED;
   if (error_code != net::ERR_ABORTED) {

content/test/navigation_simulator_impl.h

@@ -71,9 +71,6 @@ class NavigationSimulatorImpl : public NavigationSimulator,
   void Commit() override;
   void AbortCommit() override;
   void AbortFromRenderer() override;
-  void FailWithResponseHeaders(
-      int error_code,
-      scoped_refptr<net::HttpResponseHeaders> response_headers) override;
   void Fail(int error_code) override;
   void CommitErrorPage() override;
   void CommitSameDocument() override;

content/test/navigation_simulator_unittest.cc

@@ -117,9 +117,6 @@ class CancellingNavigationSimulatorTest
 
   void DidFinishNavigation(content::NavigationHandle* handle) override {
     did_finish_navigation_ = true;
-    if (handle->GetResponseHeaders()) {
-      response_headers_ = handle->GetResponseHeaders()->raw_headers();
-    }
   }
 
   void OnWillFailRequestCalled() { will_fail_request_called_ = true; }
@@ -129,7 +126,6 @@ class CancellingNavigationSimulatorTest
   std::unique_ptr<NavigationSimulator> simulator_;
   bool did_finish_navigation_ = false;
   bool will_fail_request_called_ = false;
-  std::string response_headers_;
   base::WeakPtrFactory<CancellingNavigationSimulatorTest> weak_ptr_factory_{
       this};
 
@@ -167,6 +163,28 @@ class MethodCheckingNavigationSimulatorTest : public NavigationSimulatorTest,
   DISALLOW_COPY_AND_ASSIGN(MethodCheckingNavigationSimulatorTest);
 };
 
+class ResponseHeadersCheckingNavigationSimulatorTest
+    : public NavigationSimulatorTest,
+      public WebContentsObserver {
+ public:
+  ResponseHeadersCheckingNavigationSimulatorTest() = default;
+  ~ResponseHeadersCheckingNavigationSimulatorTest() override = default;
+
+  void SetUp() override {
+    RenderViewHostImplTestHarness::SetUp();
+    contents()->GetMainFrame()->InitializeRenderFrameIfNeeded();
+    Observe(RenderViewHostImplTestHarness::web_contents());
+  }
+
+  void DidFinishNavigation(content::NavigationHandle* handle) override {
+    if (handle->GetResponseHeaders()) {
+      response_headers_ = handle->GetResponseHeaders();
+    }
+  }
+
+  const net::HttpResponseHeaders* response_headers_;
+};
+
 TEST_F(NavigationSimulatorTest, AutoAdvanceOff) {
   std::unique_ptr<NavigationSimulator> simulator =
       NavigationSimulator::CreateRendererInitiated(
@@ -225,6 +243,22 @@ TEST_F(MethodCheckingNavigationSimulatorTest, SetMethodPost) {
   EXPECT_TRUE(is_post());
 }
 
+TEST_F(ResponseHeadersCheckingNavigationSimulatorTest, CheckResponseHeaders) {
+  std::unique_ptr<NavigationSimulator> simulator =
+      NavigationSimulator::CreateRendererInitiated(
+          GURL("https://example.test/"), main_rfh());
+  simulator->Start();
+
+  auto response_headers =
+      base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 200 OK");
+  response_headers->SetHeader("My-Test-Header", "my-test-value");
+  simulator->SetResponseHeaders(response_headers);
+  simulator->ReadyToCommit();
+  simulator->Commit();
+  EXPECT_TRUE(
+      response_headers_->HasHeaderValue("My-Test-Header", "my-test-value"));
+}
+
 // Stress test the navigation simulator by having a navigation throttle cancel
 // the navigation at various points in the flow, both synchronously and
 // asynchronously.
@@ -289,21 +323,6 @@ TEST_P(NavigationSimulatorTestCancelFail, Fail) {
             simulator_->GetLastThrottleCheckResult());
 }
 
-// Test canceling the simulated navigation with response headers.
-TEST_P(NavigationSimulatorTestCancelFail, FailWithResponseHeaders) {
-  simulator_->Start();
-
-  using std::string_literals::operator""s;
-  std::string header =
-      "HTTP/1.1 404 Not Found\0"
-      "content-encoding: gzip\0\0"s;
-
-  simulator_->FailWithResponseHeaders(
-      net::ERR_CERT_DATE_INVALID,
-      base::MakeRefCounted<net::HttpResponseHeaders>(header));
-  EXPECT_EQ(response_headers_, header);
-}
-
 INSTANTIATE_TEST_SUITE_P(
     Fail,
     NavigationSimulatorTestCancelFail,

content/test/test_web_contents.cc

@@ -306,13 +306,10 @@ void TestWebContents::NavigateAndCommit(const GURL& url,
   navigation->Commit();
 }
 
-void TestWebContents::NavigateAndFail(
-    const GURL& url,
-    int error_code,
-    scoped_refptr<net::HttpResponseHeaders> response_headers) {
+void TestWebContents::NavigateAndFail(const GURL& url, int error_code) {
   std::unique_ptr<NavigationSimulator> navigation =
       NavigationSimulator::CreateBrowserInitiated(url, this);
-  navigation->FailWithResponseHeaders(error_code, std::move(response_headers));
+  navigation->Fail(error_code);
 }
 
 void TestWebContents::TestSetIsLoading(bool value) {

content/test/test_web_contents.h

@@ -77,10 +77,7 @@ class TestWebContents : public WebContentsImpl, public WebContentsTester {
       const GURL& url,
       ui::PageTransition transition = ui::PAGE_TRANSITION_LINK) override;
 
-  void NavigateAndFail(
-      const GURL& url,
-      int error_code,
-      scoped_refptr<net::HttpResponseHeaders> response_headers) override;
+  void NavigateAndFail(const GURL& url, int error_code) override;
   void TestSetIsLoading(bool value) override;
   void TestDidNavigate(RenderFrameHost* render_frame_host,
                        int nav_entry_id,