ThrottlingURLLoader: Stop invoking OnReceiveRedirect() Synchronously

Invoke OnReceiveRedirect() method asynchronously,
because there is way to use this code and get
uninitialized variable:
We want to create our |loader| variable from static function
CreateLoaderAndStart(). But in this function there is way to
invoke OnReciveRedirect() from our client code. And in our client
code we do not expect, that |loader| variable is not
costructed (uninitialized).

Bug: 1043752
Change-Id: I5c0722d89ed9f849c0b82281ad6ceb5febfc467b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2011221
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735785}

third_party/blink/common/loader/throttling_url_loader.cc

@@ -446,7 +446,11 @@ void ThrottlingURLLoader::StartNow() {
     response_head->headers = base::MakeRefCounted<net::HttpResponseHeaders>(
         net::HttpUtil::AssembleRawHeaders(header_string));
     response_head->encoded_data_length = header_string.size();
-    OnReceiveRedirect(redirect_info, std::move(response_head));
+    start_info_->task_runner->PostTask(
+        FROM_HERE,
+        base::BindOnce(&ThrottlingURLLoader::OnReceiveRedirect,
+                       base::Unretained(this), std::move(redirect_info),
+                       std::move(response_head)));
     return;
   }
 

third_party/blink/common/loader/throttling_url_loader_unittest.cc

@@ -513,10 +513,14 @@ TEST_F(ThrottlingURLLoaderTest, ModifyURLAndDeferRedirect) {
   throttle_->set_will_start_request_callback(
       base::BindRepeating([](blink::URLLoaderThrottle::Delegate* /* delegate */,
                              bool* defer) { *defer = true; }));
+  base::RunLoop run_loop;
   throttle_->set_will_redirect_request_callback(base::BindLambdaForTesting(
-      [](blink::URLLoaderThrottle::Delegate* /* delegate */, bool* defer,
-         std::vector<std::string>* /* removed_headers */,
-         net::HttpRequestHeaders* /* modified_headers */) { *defer = true; }));
+      [&](blink::URLLoaderThrottle::Delegate* /* delegate */, bool* defer,
+          std::vector<std::string>* /* removed_headers */,
+          net::HttpRequestHeaders* /* modified_headers */) {
+        *defer = true;
+        run_loop.Quit();
+      }));
 
   CreateLoaderAndStart();
 
@@ -524,6 +528,7 @@ TEST_F(ThrottlingURLLoaderTest, ModifyURLAndDeferRedirect) {
   EXPECT_EQ(0u, throttle_->will_redirect_request_called());
 
   throttle_->delegate()->Resume();
+  run_loop.Run();
 
   EXPECT_EQ(1u, throttle_->will_start_request_called());
   EXPECT_EQ(1u, throttle_->will_redirect_request_called());
@@ -2194,6 +2199,7 @@ TEST_F(ThrottlingURLLoaderTest,
   base::RunLoop run_loop2;
   base::RunLoop run_loop3;
   base::RunLoop run_loop4;
+  base::RunLoop run_loop_for_redirect;
 
   // URL for internal redirect.
   GURL modified_url = GURL("www.example.uk.com");
@@ -2214,6 +2220,7 @@ TEST_F(ThrottlingURLLoaderTest,
     net::HttpRequestHeaders modified_headers;
     loader_->FollowRedirect({} /* removed_headers */,
                             std::move(modified_headers));
+    run_loop_for_redirect.Quit();
   }));
 
   // Have two of the throttles defer, and one call restart
@@ -2275,11 +2282,18 @@ TEST_F(ThrottlingURLLoaderTest,
       run_loop3.QuitClosure()));
 
   int next_load_flag = 1;
-  for (auto* throttle : throttles) {
-    throttle->delegate()->RestartWithURLResetAndFlags(next_load_flag);
-    throttle->delegate()->Resume();
-    next_load_flag <<= 1;
-  }
+  throttles[0]->delegate()->RestartWithURLResetAndFlags(next_load_flag);
+  throttles[0]->delegate()->Resume();
+  next_load_flag <<= 1;
+
+  throttles[1]->delegate()->RestartWithURLResetAndFlags(next_load_flag);
+  throttles[1]->delegate()->Resume();
+  next_load_flag <<= 1;
+  run_loop_for_redirect.Run();
+
+  throttles[2]->delegate()->RestartWithURLResetAndFlags(next_load_flag);
+  throttles[2]->delegate()->Resume();
+  next_load_flag <<= 1;
 
   run_loop3.Run();
 
@@ -2584,6 +2598,7 @@ TEST_F(ThrottlingURLLoaderTest, MultipleRestartOfMultipleTypesDeferAndSync) {
   base::RunLoop run_loop2;
   base::RunLoop run_loop3;
   base::RunLoop run_loop4;
+  base::RunLoop run_loop_for_redirect;
 
   // URL for internal redirect.
   GURL modified_url = GURL("www.example.uk.com");
@@ -2604,6 +2619,7 @@ TEST_F(ThrottlingURLLoaderTest, MultipleRestartOfMultipleTypesDeferAndSync) {
     net::HttpRequestHeaders modified_headers;
     loader_->FollowRedirect({} /* removed_headers */,
                             std::move(modified_headers));
+    run_loop_for_redirect.Quit();
   }));
 
   // Have two of the throttles defer, and one call restart
@@ -2665,11 +2681,18 @@ TEST_F(ThrottlingURLLoaderTest, MultipleRestartOfMultipleTypesDeferAndSync) {
       run_loop3.QuitClosure()));
 
   int next_load_flag = 1;
-  for (auto* throttle : throttles) {
-    throttle->delegate()->RestartWithURLResetAndFlags(next_load_flag);
-    throttle->delegate()->Resume();
-    next_load_flag <<= 1;
-  }
+  throttles[0]->delegate()->RestartWithURLResetAndFlags(next_load_flag);
+  throttles[0]->delegate()->Resume();
+  next_load_flag <<= 1;
+
+  throttles[1]->delegate()->RestartWithURLResetAndFlags(next_load_flag);
+  throttles[1]->delegate()->Resume();
+  next_load_flag <<= 1;
+  run_loop_for_redirect.Run();
+
+  throttles[2]->delegate()->RestartWithURLResetAndFlags(next_load_flag);
+  throttles[2]->delegate()->Resume();
+  next_load_flag <<= 1;
 
   run_loop3.Run();