Instantiate ScopedTaskEnvironment in net unittests.

net/url_request/url_fetcher_impl_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 "net/url_request/url_fetcher_impl.h"
 
 #include <stdint.h>
 #include <string.h>
 
 #include <algorithm>
 #include <limits>
 #include <memory>
 #include <string>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task_scheduler/post_task.h"
-#include "base/test/scoped_task_scheduler.h"
+#include "base/test/scoped_task_environment.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "crypto/nss_util.h"
 #include "net/base/elements_upload_data_stream.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/base/upload_element_reader.h"
 #include "net/base/upload_file_element_reader.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/http/http_response_headers.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/gtest_util.h"
+#include "net/test/net_test_suite.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_test_util.h"
 #include "net/url_request/url_request_throttler_manager.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(USE_NSS_CERTS)
 #include "net/cert_net/nss_ocsp.h"
@@ skipping 49 matching lines @@
   URLFetcher* fetcher() const { return fetcher_.get(); }
 
   // Wait until the request has completed or been canceled.
   void StartFetcherAndWait() {
     fetcher_->Start();
     WaitForComplete();
   }
 
   // Wait until the request has completed or been canceled. Does not start the
   // request.
-  void WaitForComplete() { run_loop_.Run(); }
+  void WaitForComplete() {
+    EXPECT_TRUE(task_runner_->BelongsToCurrentThread());
+    run_loop_.Run();
+  }
 
   // Cancels the fetch by deleting the fetcher.
   void CancelFetch() {
     EXPECT_TRUE(fetcher_);
     fetcher_.reset();
-    run_loop_.Quit();
+    task_runner_->PostTask(FROM_HERE, run_loop_.QuitClosure());
   }
 
   // URLFetcherDelegate:
   void OnURLFetchComplete(const URLFetcher* source) override {
     EXPECT_FALSE(did_complete_);
     EXPECT_TRUE(fetcher_);
     EXPECT_EQ(fetcher_.get(), source);
     did_complete_ = true;
-    run_loop_.Quit();
+    task_runner_->PostTask(FROM_HERE, run_loop_.QuitClosure());

[fdoray, 2017/05/09 21:43:47]

TaskScheduler tasks run on dedicated thread when they are posted within the
scope of a ScopedTaskEnvironment while they run on the main thread when posted
within the scope of a ScopedTaskScheduler. Switching from ScopedTaskEnvironment
to ScopedTaskScheduler means that OnURLFetchComplete is not called on the main
thread and a post task to |task_runner_| is required to quit the run loop on the
main thread.

[mmenke, 2017/05/10 15:43:44]

I'm not really following, nor do I understand why that affects this test and no
others.  I'll try to work through it later today, but not sure I'll get to it.
Have a number of reviews on my plate today.

[fdoray, 2017/05/10 16:16:12]

- run_loop_.Quit() and run_loop_.Run() must be called on the same thread.
- This test calls run_loop_.Run() on the main thread.
- OnURLFetchComplete() is called as part of a TaskScheduler task.
- Within the scope of a ScopedTaskScheduler (before this CL), TaskScheduler
tasks are scheduled on the main thread. Therefore, before this CL,
OnURLFetchComplete() ran on the same thread as run_loop_.Run() and it was valid
to call run_loop_.Quit() inline.
- Within the scope of a ScopedTaskEnvironment (with this CL), TaskScheduler
tasks are scheduled on a dedicated thread. Therefore, it is no longer valid to
call run_loop_.Quit() inline. Instead, we must post a QuitClosure() to the main
thread.

[mmenke, 2017/05/11 15:30:39]

URLFetchers are cross-thread objects.  You start them on one thread, it fetches
on another thread (Which may be the starting thread), and then they notify of
completion on the original thread.

The URLFetcher is being started on the main thread, so it should also be
notified of completion on the main thread.  So if this isn't being called on the
main thread, we may have a problem.

[fdoray, 2017/05/11 16:15:08]

URLFetcherTest.SequencedTaskTest creates a URLFetcher in  TaskScheduler.
OnURLFetchComplete() will therefore be invoked in TaskScheduler.

URLFetcherTest.SequencedTaskTest also calls StartFetcherAndWait(), which ends up
calling run_loop_.Run(), on the main thread.

The call to run_loop_.Quit() must happen on the main thread (same thread as
run_loop_.Run()). Therefore, it cannot happen synchronously in
OnURLFetchComplete(). A post task back to the main thread is required.

   }
 
   void OnURLFetchDownloadProgress(const URLFetcher* source,
                                   int64_t current,
                                   int64_t total,
                                   int64_t current_network_bytes) override {
     // Note that the current progress may be greater than the previous progress,
     // in the case of retrying the request.
     EXPECT_FALSE(did_complete_);
     EXPECT_TRUE(fetcher_);
@@ skipping 19 matching lines @@
     if (total >= 0)
       EXPECT_LE(current, total);
   }
 
   bool did_complete() const { return did_complete_; }
 
  private:
   bool did_complete_;
 
   std::unique_ptr<URLFetcherImpl> fetcher_;
+  const scoped_refptr<base::SingleThreadTaskRunner> task_runner_ =
+      base::ThreadTaskRunnerHandle::Get();
   base::RunLoop run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(WaitingURLFetcherDelegate);
 };
 
 // A TestURLRequestContext with a ThrottleManager and a MockHostResolver.
 class FetcherTestURLRequestContext : public TestURLRequestContext {
  public:
   // All requests for |hanging_domain| will hang on host resolution until the
   // mock_resolver()->ResolveAllPending() is called.
@@ skipping 336 matching lines @@
   EXPECT_TRUE(delegate.fetcher()->GetStatus().is_success());
   EXPECT_EQ(200, delegate.fetcher()->GetResponseCode());
   std::string data;
   ASSERT_TRUE(delegate.fetcher()->GetResponseAsString(&data));
   EXPECT_EQ(kDefaultResponseBody, data);
 }
 
 // Create the fetcher from a sequenced (not single-threaded) task. Verify that
 // the expected response is received.
 TEST_F(URLFetcherTest, SequencedTaskTest) {
-  base::test::ScopedTaskScheduler scoped_task_scheduler(
-      base::MessageLoop::current());
   auto sequenced_task_runner = base::CreateSequencedTaskRunnerWithTraits({});
-
   auto delegate = base::MakeUnique<WaitingURLFetcherDelegate>();
   sequenced_task_runner->PostTask(
       FROM_HERE, base::Bind(&WaitingURLFetcherDelegate::CreateFetcher,
                             base::Unretained(delegate.get()),
                             test_server_->GetURL(kDefaultResponsePath),
                             URLFetcher::GET, CreateCrossThreadContextGetter()));
-  base::RunLoop().RunUntilIdle();
+  NetTestSuite::GetScopedTaskEnvironment()->RunUntilIdle();
   delegate->StartFetcherAndWait();
 
   EXPECT_TRUE(delegate->fetcher()->GetStatus().is_success());
   EXPECT_EQ(200, delegate->fetcher()->GetResponseCode());
   std::string data;
   ASSERT_TRUE(delegate->fetcher()->GetResponseAsString(&data));
   EXPECT_EQ(kDefaultResponseBody, data);
 
   sequenced_task_runner->DeleteSoon(FROM_HERE, delegate.release());
-  base::RunLoop().RunUntilIdle();
+  NetTestSuite::GetScopedTaskEnvironment()->RunUntilIdle();
 }
 
 // Tests to make sure CancelAll() will successfully cancel existing URLFetchers.
 TEST_F(URLFetcherTest, CancelAll) {
   EXPECT_EQ(0, GetNumFetcherCores());
 
   scoped_refptr<FetcherTestURLRequestContextGetter> context_getter(
       CreateSameThreadContextGetter());
   // Force context creation.
   context_getter->GetURLRequestContext();
@@ skipping 1013 matching lines @@
   EXPECT_EQ(-1, delegate.fetcher()->GetResponseCode());
   EXPECT_FALSE(delegate.fetcher()->GetResponseHeaders());
   std::string data;
   EXPECT_TRUE(delegate.fetcher()->GetResponseAsString(&data));
   EXPECT_TRUE(data.empty());
 }
 
 }  // namespace
 
 }  // namespace net