Removing file_thread_task_runner parameter from SerialIoHandler

device/serial/serial_io_handler.cc

 // Copyright 2014 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 "device/serial/serial_io_handler.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 
 #if defined(OS_CHROMEOS)
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/permission_broker_client.h"
 #endif  // defined(OS_CHROMEOS)
 
 namespace device {
 
 SerialIoHandler::SerialIoHandler(
-    scoped_refptr<base::SingleThreadTaskRunner> file_thread_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> ui_thread_task_runner)
-    : file_thread_task_runner_(file_thread_task_runner),
-      ui_thread_task_runner_(ui_thread_task_runner) {
+    : ui_thread_task_runner_(ui_thread_task_runner) {
   options_.bitrate = 9600;
   options_.data_bits = serial::DataBits::EIGHT;
   options_.parity_bit = serial::ParityBit::NO_PARITY;
   options_.stop_bits = serial::StopBits::ONE;
   options_.cts_flow_control = false;
   options_.has_cts_flow_control = true;
 }
 
 SerialIoHandler::~SerialIoHandler() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  Close();
 }
 
 void SerialIoHandler::Open(const std::string& port,
                            const serial::ConnectionOptions& options,
                            const OpenCompleteCallback& callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(open_complete_.is_null());
   open_complete_ = callback;
-  DCHECK(file_thread_task_runner_.get());
   DCHECK(ui_thread_task_runner_.get());
   MergeConnectionOptions(options);
   port_ = port;
 
 #if defined(OS_CHROMEOS)
   chromeos::PermissionBrokerClient* client =
       chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
   DCHECK(client) << "Could not get permission_broker client.";
   // PermissionBrokerClient should be called on the UI thread.
   scoped_refptr<base::SingleThreadTaskRunner> task_runner =
       base::ThreadTaskRunnerHandle::Get();
   ui_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(
           &chromeos::PermissionBrokerClient::OpenPath, base::Unretained(client),
           port, base::Bind(&SerialIoHandler::OnPathOpened, this, task_runner),
           base::Bind(&SerialIoHandler::OnPathOpenError, this, task_runner)));
-#else
-  file_thread_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&SerialIoHandler::StartOpen, this, port,
-                            base::ThreadTaskRunnerHandle::Get()));

[Reilly Grant (use Gerrit), 2017/06/01 13:48:21]

file_thread_task_runner_ is clearly used here so the original issue was wrong to
say it was completely unused. However we can replace this statement with,

base::PostTaskWithTraits(
    FROM_HERE,
    {base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
    base::Bind(&SerialIoHandler::StartOpen, this, port,	
               base::ThreadTaskRunnerHandle::Get()));

[sujith, 2017/06/01 14:57:31]

Done.

 #endif  // defined(OS_CHROMEOS)
 }
 
 #if defined(OS_CHROMEOS)
 
 void SerialIoHandler::OnPathOpened(
     scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner,
     base::ScopedFD fd) {
   base::File file(fd.release());
   io_thread_task_runner->PostTask(
@@ skipping 40 matching lines @@
   if (options.has_cts_flow_control) {
     DCHECK(options_.has_cts_flow_control);
     options_.cts_flow_control = options.cts_flow_control;
   }
 }
 
 void SerialIoHandler::StartOpen(
     const std::string& port,
     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) {
   DCHECK(!open_complete_.is_null());
-  DCHECK(file_thread_task_runner_->RunsTasksInCurrentSequence());
   DCHECK(!file_.IsValid());
   // It's the responsibility of the API wrapper around SerialIoHandler to
   // validate the supplied path against the set of valid port names, and
   // it is a reasonable assumption that serial port names are ASCII.
   DCHECK(base::IsStringASCII(port));
   base::FilePath path(base::FilePath::FromUTF8Unsafe(MaybeFixUpPortName(port)));
   int flags = base::File::FLAG_OPEN | base::File::FLAG_READ |
               base::File::FLAG_EXCLUSIVE_READ | base::File::FLAG_WRITE |
               base::File::FLAG_EXCLUSIVE_WRITE | base::File::FLAG_ASYNC |
               base::File::FLAG_TERMINAL_DEVICE;
@@ skipping 11 matching lines @@
   if (!file.IsValid()) {
     LOG(ERROR) << "Failed to open serial port: "
                << base::File::ErrorToString(file.error_details());
     callback.Run(false);
     return;
   }
 
   file_ = std::move(file);
 
   bool success = PostOpen() && ConfigurePortImpl();
-  if (!success)
-    Close();
 
   callback.Run(success);
 }
 
 bool SerialIoHandler::PostOpen() {
   return true;
 }
 
-void SerialIoHandler::Close() {
-  if (file_.IsValid()) {
-    DCHECK(file_thread_task_runner_.get());
-    file_thread_task_runner_->PostTask(

[Reilly Grant (use Gerrit), 2017/06/01 13:48:21]

Same here. We can't just remove the Close method. Please keep this method and
use base::PostTaskWithTraits instead.

[sujith, 2017/06/01 14:57:31]

Done.

-        FROM_HERE,
-        base::Bind(&SerialIoHandler::DoClose, Passed(std::move(file_))));
-  }
-}
-
 // static
 void SerialIoHandler::DoClose(base::File port) {
   // port closed by destructor.
 }
 
 void SerialIoHandler::Read(std::unique_ptr<WritableBuffer> buffer) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!IsReadPending());
   pending_read_buffer_ = std::move(buffer);
   read_canceled_ = false;
@@ skipping 80 matching lines @@
 }
 
 void SerialIoHandler::QueueWriteCompleted(int bytes_written,
                                           serial::SendError error) {
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(&SerialIoHandler::WriteCompleted, this, bytes_written, error));
 }
 
 }  // namespace device