[Task Migration][Extensions] Update FileReader

Update the FileReader class to use the new task scheduling API, as well
as the dedicated Extensions-related file task runner.

Bug: 689520
Bug: 750122
Change-Id: I3dd119e9c6bc368ab0186e599a4a67df02b872d6
Reviewed-on: https://chromium-review.googlesource.com/594010
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491566}

extensions/browser/api/execute_code_function.cc

@@ -237,7 +237,7 @@ bool ExecuteCodeFunction::LoadFile(const std::string& file) {
                        resource_.relative_path().AsUTF8Unsafe(),
                        true /* We assume this call always succeeds */));
   } else {
-    FileReader::OptionalFileThreadTaskCallback get_file_and_l10n_callback =
+    FileReader::OptionalFileSequenceTask get_file_and_l10n_callback =
         base::Bind(&ExecuteCodeFunction::GetFileURLAndMaybeLocalizeInBackground,
                    this, extension_id, extension_path, extension_default_locale,
                    might_require_localization);

extensions/browser/file_reader.cc

@@ -8,37 +8,36 @@
 #include "base/callback_helpers.h"
 #include "base/files/file_util.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "content/public/browser/browser_thread.h"
-
-using content::BrowserThread;
+#include "extensions/browser/extension_file_task_runner.h"
 
 FileReader::FileReader(
     const extensions::ExtensionResource& resource,
-    const OptionalFileThreadTaskCallback& optional_file_thread_task_callback,
+    const OptionalFileSequenceTask& optional_file_sequence_task,
     const DoneCallback& done_callback)
     : resource_(resource),
-      optional_file_thread_task_callback_(optional_file_thread_task_callback),
+      optional_file_sequence_task_(optional_file_sequence_task),
       done_callback_(done_callback),
       origin_task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
 
 void FileReader::Start() {
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&FileReader::ReadFileOnBackgroundThread, this));
+  extensions::GetExtensionFileTaskRunner()->PostTask(
+      FROM_HERE, base::Bind(&FileReader::ReadFileOnFileSequence, this));
 }
 
 FileReader::~FileReader() {}
 
-void FileReader::ReadFileOnBackgroundThread() {
+void FileReader::ReadFileOnFileSequence() {
+  DCHECK(
+      extensions::GetExtensionFileTaskRunner()->RunsTasksInCurrentSequence());
+
   std::unique_ptr<std::string> data(new std::string());
   bool success = base::ReadFileToString(resource_.GetFilePath(), data.get());
 
-  if (!optional_file_thread_task_callback_.is_null()) {
+  if (!optional_file_sequence_task_.is_null()) {
     if (success) {
-      base::ResetAndReturn(&optional_file_thread_task_callback_)
-          .Run(data.get());
+      base::ResetAndReturn(&optional_file_sequence_task_).Run(data.get());
     } else {
-      optional_file_thread_task_callback_.Reset();
+      optional_file_sequence_task_.Reset();
     }
   }
 

extensions/browser/file_reader.h

@@ -13,37 +13,40 @@
 #include "extensions/common/extension_resource.h"
 
 // This file defines an interface for reading a file asynchronously on a
-// background thread.
+// background sequence.
 // Consider abstracting out a FilePathProvider (ExtensionResource) and moving
 // back to chrome/browser/net if other subsystems want to use it.
 class FileReader : public base::RefCountedThreadSafe<FileReader> {
  public:
+  // TODO(devlin): Use base::OnceCallback here.
   // Reports success or failure and the data of the file upon success.
   using DoneCallback = base::Callback<void(bool, std::unique_ptr<std::string>)>;
   // Lets the caller accomplish tasks on the file data, after the file content
   // has been read.
   // If the file reading doesn't succeed, this will be ignored.
-  using OptionalFileThreadTaskCallback = base::Callback<void(std::string*)>;
+  using OptionalFileSequenceTask = base::Callback<void(std::string*)>;
 
   FileReader(const extensions::ExtensionResource& resource,
-             const OptionalFileThreadTaskCallback& file_thread_task_callback,
+             const OptionalFileSequenceTask& file_sequence_task,
              const DoneCallback& done_callback);
 
-  // Called to start reading the file on a background thread.  Upon completion,
+  // Called to start reading the file on a background sequence. Upon completion,
   // the callback will be notified of the results.
   void Start();
 
  private:
   friend class base::RefCountedThreadSafe<FileReader>;
 
-  virtual ~FileReader();
+  ~FileReader();
 
-  void ReadFileOnBackgroundThread();
+  void ReadFileOnFileSequence();
 
   extensions::ExtensionResource resource_;
-  OptionalFileThreadTaskCallback optional_file_thread_task_callback_;
+  OptionalFileSequenceTask optional_file_sequence_task_;
   DoneCallback done_callback_;
   const scoped_refptr<base::SingleThreadTaskRunner> origin_task_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileReader);
 };
 
 #endif  // EXTENSIONS_BROWSER_FILE_READER_H_

extensions/browser/file_reader_unittest.cc

@@ -2,41 +2,44 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "extensions/browser/file_reader.h"
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
-#include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
 #include "components/crx_file/id_util.h"
-#include "content/public/test/test_browser_thread.h"
-#include "extensions/browser/file_reader.h"
 #include "extensions/common/extension_paths.h"
 #include "extensions/common/extension_resource.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using content::BrowserThread;
-
 namespace extensions {
 
 class FileReaderTest : public testing::Test {
  public:
-  FileReaderTest() : file_thread_(BrowserThread::FILE) {
-    file_thread_.Start();
-  }
+  FileReaderTest() {}
+
  private:
-  base::MessageLoop message_loop_;
-  content::TestBrowserThread file_thread_;
+  base::test::ScopedTaskEnvironment task_environment_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileReaderTest);
 };
 
 class Receiver {
  public:
-  Receiver() : succeeded_(false) {
-  }
-
-  FileReader::DoneCallback NewCallback() {
-    return base::Bind(&Receiver::DidReadFile, base::Unretained(this));
+  Receiver(const ExtensionResource& resource)
+      : succeeded_(false),
+        file_reader_(new FileReader(
+            resource,
+            FileReader::OptionalFileSequenceTask(),
+            base::Bind(&Receiver::DidReadFile, base::Unretained(this)))) {}
+
+  void Run() {
+    file_reader_->Start();
+    run_loop_.Run();
   }
 
   bool succeeded() const { return succeeded_; }
@@ -46,11 +49,15 @@ class Receiver {
   void DidReadFile(bool success, std::unique_ptr<std::string> data) {
     succeeded_ = success;
     data_ = std::move(data);
-    base::RunLoop::QuitCurrentWhenIdleDeprecated();
+    run_loop_.QuitWhenIdle();
   }
 
   bool succeeded_;
   std::unique_ptr<std::string> data_;
+  scoped_refptr<FileReader> file_reader_;
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(Receiver);
 };
 
 void RunBasicTest(const char* filename) {
@@ -64,14 +71,8 @@ void RunBasicTest(const char* filename) {
   std::string file_contents;
   ASSERT_TRUE(base::ReadFileToString(path, &file_contents));
 
-  Receiver receiver;
-
-  scoped_refptr<FileReader> file_reader(
-      new FileReader(resource, FileReader::OptionalFileThreadTaskCallback(),
-                     receiver.NewCallback()));
-  file_reader->Start();
-
-  base::RunLoop().Run();
+  Receiver receiver(resource);
+  receiver.Run();
 
   EXPECT_TRUE(receiver.succeeded());
   EXPECT_EQ(file_contents, receiver.data());
@@ -93,14 +94,8 @@ TEST_F(FileReaderTest, NonExistantFile) {
       FILE_PATH_LITERAL("file_that_does_not_exist")));
   path = path.AppendASCII("file_that_does_not_exist");
 
-  Receiver receiver;
-
-  scoped_refptr<FileReader> file_reader(
-      new FileReader(resource, FileReader::OptionalFileThreadTaskCallback(),
-                     receiver.NewCallback()));
-  file_reader->Start();
-
-  base::RunLoop().Run();
+  Receiver receiver(resource);
+  receiver.Run();
 
   EXPECT_FALSE(receiver.succeeded());
 }