Ensure that the safe browsing NetworkContext is torn down before its owning URLRequestContext.

This wasn't the case when I added the SafeBrowsingNetworkContext  in r547291, as the NetworkContext destruction task would run after the destruction of the net::URLRequestContext in SafeBrowsingURLRequestContextGetter::ServiceShuttingDown().

Bug: 830538
Change-Id: Ie98300dea00b2af967edec10197edb4cb2ced8d8
Reviewed-on: https://chromium-review.googlesource.com/1002733
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549343}

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -188,6 +188,12 @@ void SafeBrowsingService::ShutDown() {
 
   services_delegate_->ShutdownServices();
 
+  // Make sure to destruct SafeBrowsingNetworkContext first before
+  // |url_request_context_getter_|, as they both post tasks to the IO thread. We
+  // want the underlying NetworkContext C++ class to be torn down first so that
+  // it destroys any URLLoaders in flight.
+  network_context_->ServiceShuttingDown();
+
   // Since URLRequestContextGetters are refcounted, can't count on clearing
   // |url_request_context_getter_| to delete it, so need to shut it down first,
   // which will cancel any requests that are currently using it, and prevent
@@ -196,7 +202,6 @@ void SafeBrowsingService::ShutDown() {
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&SafeBrowsingURLRequestContextGetter::ServiceShuttingDown,
                      url_request_context_getter_));
-  network_context_->ServiceShuttingDown();
 
   // Release the URLRequestContextGetter after passing it to the IOThread.  It
   // has to be released now rather than in the destructor because it can only

chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc

@@ -90,8 +90,10 @@
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
+#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/websockets/websocket_handshake_constants.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 #include "sql/connection.h"
 #include "sql/statement.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -1782,6 +1784,32 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, StartAndStop) {
   EXPECT_FALSE(csd_service->enabled());
 }
 
+// This test should not end in an AssertNoURLLRequests CHECK.
+IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, ShutdownWithLiveRequest) {
+  std::unique_ptr<network::ResourceRequest> request =
+      std::make_unique<network::ResourceRequest>();
+  request->url = embedded_test_server()->GetURL("/hung-after-headers");
+  std::unique_ptr<network::SimpleURLLoader> loader =
+      network::SimpleURLLoader::Create(std::move(request),
+                                       TRAFFIC_ANNOTATION_FOR_TESTS);
+
+  base::RunLoop run_loop;
+  loader->SetOnResponseStartedCallback(base::BindOnce(
+      [](const base::Closure& quit_closure, const GURL& final_url,
+         const network::ResourceResponseHead& response_head) {
+        quit_closure.Run();
+      },
+      run_loop.QuitClosure()));
+  loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      g_browser_process->safe_browsing_service()->GetURLLoaderFactory().get(),
+      base::BindOnce([](std::unique_ptr<std::string> response_body) {}));
+
+  // Ensure that the request has already reached the URLLoader responsible for
+  // making it, or otherwise this test might pass if we have a regression.
+  run_loop.Run();
+  loader.release();
+}
+
 // Parameterised fixture to permit running the same test for Window and Worker
 // scopes.
 class SafeBrowsingServiceJsRequestTest