From 89822579003fc4f5128c58f9bbd1a22b347ea229 Mon Sep 17 00:00:00 2001 From: hashimoto Date: Wed, 24 Aug 2016 09:51:38 -0700 Subject: [PATCH] No dbus::FileDescriptor in DebugDaemonClient This is the first step to replace dbus::FileDescriptor with base::ScopedFD. Add a new variant of dbus::MessageWriter::AppendFileDescriptor and dbus::MessageReader::PopFileDescriptor. Use the new variant in DebugDaemonClient. BUG=621841 Review-Url: https://codereview.chromium.org/2081153002 Cr-Commit-Position: refs/heads/master@{#414093} --- .../chromeos/system_logs/debug_log_writer.cc | 36 ++-- .../log_private_apitest_chromeos.cc | 13 +- chrome/browser/metrics/perf/perf_output.cc | 37 +---- chrome/browser/metrics/perf/perf_output.h | 2 - chromeos/dbus/debug_daemon_client.cc | 155 +++++------------- chromeos/dbus/debug_daemon_client.h | 16 +- chromeos/dbus/fake_debug_daemon_client.cc | 10 +- chromeos/dbus/fake_debug_daemon_client.h | 5 +- chromeos/dbus/lorgnette_manager_client.cc | 9 +- chromeos/dbus/pipe_reader.cc | 8 +- chromeos/dbus/pipe_reader.h | 4 +- dbus/message.cc | 17 ++ dbus/message.h | 12 ++ 13 files changed, 123 insertions(+), 201 deletions(-) diff --git a/chrome/browser/chromeos/system_logs/debug_log_writer.cc b/chrome/browser/chromeos/system_logs/debug_log_writer.cc index 3aad0adcb13e50..4ac2f6c2c4a65b 100644 --- a/chrome/browser/chromeos/system_logs/debug_log_writer.cc +++ b/chrome/browser/chromeos/system_logs/debug_log_writer.cc @@ -62,7 +62,7 @@ void WriteDebugLogToFileCompleted( // Stores into |file_path| debug logs in the .tgz format. Calls // |callback| upon completion. -void WriteDebugLogToFile(base::File* file, +void WriteDebugLogToFile(std::unique_ptr file, const std::string& sequence_token_name, const base::FilePath& file_path, bool should_compress, @@ -74,12 +74,15 @@ void WriteDebugLogToFile(base::File* file, << "error: " << file->error_details(); return; } - scoped_refptr task_runner = - GetSequencedTaskRunner(sequence_token_name); chromeos::DBusThreadManager::Get()->GetDebugDaemonClient()->DumpDebugLogs( - should_compress, std::move(*file), task_runner, + should_compress, file->GetPlatformFile(), base::Bind(&WriteDebugLogToFileCompleted, file_path, sequence_token_name, callback)); + + // Close the file on an IO-allowed thread. + GetSequencedTaskRunner(sequence_token_name) + ->PostTask(FROM_HERE, + base::Bind(&base::DeletePointer, file.release())); } // Runs command with its parameters as defined in |argv|. @@ -200,9 +203,9 @@ void OnSystemLogsAdded(const DebugLogWriter::StoreLogsCallback& callback, callback)); } -void IntializeLogFile(base::File* file, - const base::FilePath& file_path, - uint32_t flags) { +void InitializeLogFile(base::File* file, + const base::FilePath& file_path, + uint32_t flags) { base::FilePath dir = file_path.DirName(); if (!base::DirectoryExists(dir)) { if (!base::CreateDirectory(dir)) { @@ -224,16 +227,15 @@ void StartLogRetrieval(const base::FilePath& file_name_template, logging::GenerateTimestampedName(file_name_template, base::Time::Now()); int flags = base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE; - base::File* file = new base::File; - GetSequencedTaskRunner(sequence_token_name)->PostTaskAndReply( - FROM_HERE, - base::Bind(&IntializeLogFile, base::Unretained(file), file_path, flags), - base::Bind(&WriteDebugLogToFile, - base::Owned(file), - sequence_token_name, - file_path, - should_compress, - callback)); + std::unique_ptr file(new base::File); + base::File* file_ptr = file.get(); + GetSequencedTaskRunner(sequence_token_name) + ->PostTaskAndReply( + FROM_HERE, base::Bind(&InitializeLogFile, base::Unretained(file_ptr), + file_path, flags), + base::Bind(&WriteDebugLogToFile, base::Passed(&file), + sequence_token_name, file_path, should_compress, + callback)); } const char kDefaultSequenceName[] = "DebugLogWriter"; diff --git a/chrome/browser/extensions/api/log_private/log_private_apitest_chromeos.cc b/chrome/browser/extensions/api/log_private/log_private_apitest_chromeos.cc index 1cb54030adea11..47f29ab538d65a 100644 --- a/chrome/browser/extensions/api/log_private/log_private_apitest_chromeos.cc +++ b/chrome/browser/extensions/api/log_private/log_private_apitest_chromeos.cc @@ -32,14 +32,13 @@ class TestDebugDaemonClient : public chromeos::FakeDebugDaemonClient { ~TestDebugDaemonClient() override {} void DumpDebugLogs(bool is_compressed, - base::File file, - scoped_refptr task_runner, + int file_descriptor, const GetDebugLogsCallback& callback) override { - base::File* file_param = new base::File(std::move(file)); - task_runner->PostTaskAndReply( - FROM_HERE, - base::Bind( - &GenerateTestLogDumpFile, test_file_, base::Owned(file_param)), + // dup() is needed as the file descriptor will be closed on the client side. + base::File* file_param = new base::File(dup(file_descriptor)); + content::BrowserThread::PostBlockingPoolTaskAndReply( + FROM_HERE, base::Bind(&GenerateTestLogDumpFile, test_file_, + base::Owned(file_param)), base::Bind(callback, true)); } diff --git a/chrome/browser/metrics/perf/perf_output.cc b/chrome/browser/metrics/perf/perf_output.cc index c9c1d37a646ddd..4988d54fafe8f3 100644 --- a/chrome/browser/metrics/perf/perf_output.cc +++ b/chrome/browser/metrics/perf/perf_output.cc @@ -4,26 +4,10 @@ #include "chrome/browser/metrics/perf/perf_output.h" -#include - #include "base/bind.h" -#include "base/location.h" -#include "base/task_runner_util.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/debug_daemon_client.h" -namespace { - -// Create a dbus::FileDescriptor from a base::File. -dbus::ScopedFileDescriptor CreateFileDescriptor(base::File pipe_write_end) { - dbus::ScopedFileDescriptor file_descriptor(new dbus::FileDescriptor); - file_descriptor->PutValue(pipe_write_end.TakePlatformFile()); - file_descriptor->CheckValidity(); - return file_descriptor; -} - -} // namespace - PerfOutputCall::PerfOutputCall( scoped_refptr blocking_task_runner, base::TimeDelta duration, @@ -40,29 +24,16 @@ PerfOutputCall::PerfOutputCall( blocking_task_runner_, base::Bind(&PerfOutputCall::OnIOComplete, weak_factory_.GetWeakPtr()))); - base::File pipe_write_end = perf_data_pipe_reader_->StartIO(); - base::PostTaskAndReplyWithResult( - blocking_task_runner_.get(), FROM_HERE, - base::Bind(&CreateFileDescriptor, base::Passed(&pipe_write_end)), - base::Bind(&PerfOutputCall::OnFileDescriptorCreated, - weak_factory_.GetWeakPtr())); -} - -PerfOutputCall::~PerfOutputCall() {} - -void PerfOutputCall::OnFileDescriptorCreated( - dbus::ScopedFileDescriptor file_descriptor) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(file_descriptor); - + base::ScopedFD pipe_write_end = perf_data_pipe_reader_->StartIO(); chromeos::DebugDaemonClient* client = chromeos::DBusThreadManager::Get()->GetDebugDaemonClient(); - - client->GetPerfOutput(duration_, perf_args_, std::move(file_descriptor), + client->GetPerfOutput(duration_, perf_args_, pipe_write_end.get(), base::Bind(&PerfOutputCall::OnGetPerfOutputError, weak_factory_.GetWeakPtr())); } +PerfOutputCall::~PerfOutputCall() {} + void PerfOutputCall::OnIOComplete() { DCHECK(thread_checker_.CalledOnValidThread()); diff --git a/chrome/browser/metrics/perf/perf_output.h b/chrome/browser/metrics/perf/perf_output.h index a2cd1e3c2002e0..d0f9cde6cb5e3d 100644 --- a/chrome/browser/metrics/perf/perf_output.h +++ b/chrome/browser/metrics/perf/perf_output.h @@ -14,7 +14,6 @@ #include "base/threading/thread_checker.h" #include "base/time/time.h" #include "chromeos/dbus/pipe_reader.h" -#include "dbus/file_descriptor.h" // Class for handling getting output from perf over DBus. Manages the // asynchronous DBus call and retrieving data from quipper over a pipe. @@ -36,7 +35,6 @@ class PerfOutputCall { private: // Internal callbacks. - void OnFileDescriptorCreated(dbus::ScopedFileDescriptor file_descriptor); void OnIOComplete(); void OnGetPerfOutputError(const std::string& error_name, const std::string& error_message); diff --git a/chromeos/dbus/debug_daemon_client.cc b/chromeos/dbus/debug_daemon_client.cc index a101518c25c9f8..dafaed73019958 100644 --- a/chromeos/dbus/debug_daemon_client.cc +++ b/chromeos/dbus/debug_daemon_client.cc @@ -39,33 +39,12 @@ const char kCrOSTraceLabel[] = "systemTraceEvents"; // Because the cheets logs are very huge, we set the D-Bus timeout to 2 minutes. const int kBigLogsDBusTimeoutMS = 120 * 1000; -// The type of the callback to be invoked once the pipe reader has been -// initialized and a D-Bus file descriptor has been created for it. -using OnPipeReaderInitializedCallback = - base::Callback; - // Used in DebugDaemonClient::EmptyStopAgentTracingCallback(). void EmptyStopAgentTracingCallbackBody( const std::string& agent_name, const std::string& events_label, const scoped_refptr& unused_result) {} -// Creates a D-Bus file descriptor from a base::File. -dbus::ScopedFileDescriptor CreateFileDescriptorForPipeWriteEnd( - base::File pipe_write_end) { - if (!pipe_write_end.IsValid()) { - VLOG(1) << "Cannot create pipe reader"; - // NB: continue anyway; toss the data - pipe_write_end.Initialize(base::FilePath(FILE_PATH_LITERAL("/dev/null")), - base::File::FLAG_OPEN | base::File::FLAG_WRITE); - // TODO(afakhry): If this fails AppendFileDescriptor will abort. - } - dbus::ScopedFileDescriptor file_descriptor(new dbus::FileDescriptor); - file_descriptor->PutValue(pipe_write_end.TakePlatformFile()); - file_descriptor->CheckValidity(); - return file_descriptor; -} - // A self-deleting object that wraps the pipe reader operations for reading the // big feedback logs. It will delete itself once the pipe stream has been // terminated. Once the data has been completely read from the pipe, it invokes @@ -80,14 +59,7 @@ class PipeReaderWrapper : public base::SupportsWeakPtr { base::Bind(&PipeReaderWrapper::OnIOComplete, AsWeakPtr())), callback_(callback) {} - void Initialize(const OnPipeReaderInitializedCallback& on_initialized) { - base::File pipe_write_end = pipe_reader_.StartIO(); - base::PostTaskAndReplyWithResult( - task_runner_.get(), FROM_HERE, - base::Bind(&CreateFileDescriptorForPipeWriteEnd, - base::Passed(&pipe_write_end)), - on_initialized); - } + base::ScopedFD Initialize() { return pipe_reader_.StartIO(); } void OnIOComplete() { std::string pipe_data; @@ -137,22 +109,18 @@ class DebugDaemonClientImpl : public DebugDaemonClient { // DebugDaemonClient override. void DumpDebugLogs(bool is_compressed, - base::File file, - scoped_refptr task_runner, + int file_descriptor, const GetDebugLogsCallback& callback) override { - dbus::FileDescriptor* file_descriptor = new dbus::FileDescriptor; - file_descriptor->PutValue(file.TakePlatformFile()); - // Punt descriptor validity check to a worker thread; on return we'll - // issue the D-Bus request to stop tracing and collect results. - task_runner->PostTaskAndReply( - FROM_HERE, - base::Bind(&dbus::FileDescriptor::CheckValidity, - base::Unretained(file_descriptor)), - base::Bind(&DebugDaemonClientImpl::OnCheckValidityGetDebugLogs, - weak_ptr_factory_.GetWeakPtr(), - is_compressed, - base::Owned(file_descriptor), - callback)); + // Issue the dbus request to get debug logs. + dbus::MethodCall method_call(debugd::kDebugdInterface, + debugd::kDumpDebugLogs); + dbus::MessageWriter writer(&method_call); + writer.AppendBool(is_compressed); + writer.AppendFileDescriptor(file_descriptor); + debugdaemon_proxy_->CallMethod( + &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, + base::Bind(&DebugDaemonClientImpl::OnGetDebugLogs, + weak_ptr_factory_.GetWeakPtr(), callback)); } void SetDebugMode(const std::string& subsystem, @@ -242,7 +210,7 @@ class DebugDaemonClientImpl : public DebugDaemonClient { void GetPerfOutput(base::TimeDelta duration, const std::vector& perf_args, - dbus::ScopedFileDescriptor file_descriptor, + int file_descriptor, const DBusMethodErrorCallback& error_callback) override { DCHECK(file_descriptor); dbus::MethodCall method_call(debugd::kDebugdInterface, @@ -250,7 +218,7 @@ class DebugDaemonClientImpl : public DebugDaemonClient { dbus::MessageWriter writer(&method_call); writer.AppendUint32(duration.InSeconds()); writer.AppendArrayOfStrings(perf_args); - writer.AppendFileDescriptor(*file_descriptor); + writer.AppendFileDescriptor(file_descriptor); debugdaemon_proxy_->CallMethodWithErrorCallback( &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, @@ -277,8 +245,17 @@ class DebugDaemonClientImpl : public DebugDaemonClient { // long time to be processed and a new request should never be ignored nor // cancels the on-going one. PipeReaderWrapper* pipe_reader = new PipeReaderWrapper(callback); - pipe_reader->Initialize( - base::Bind(&DebugDaemonClientImpl::OnBigLogsPipeReaderReady, + base::ScopedFD pipe_write_end = pipe_reader->Initialize(); + + dbus::MethodCall method_call(debugd::kDebugdInterface, + debugd::kGetBigFeedbackLogs); + dbus::MessageWriter writer(&method_call); + writer.AppendFileDescriptor(pipe_write_end.get()); + + DVLOG(1) << "Requesting big feedback logs"; + debugdaemon_proxy_->CallMethod( + &method_call, kBigLogsDBusTimeoutMS, + base::Bind(&DebugDaemonClientImpl::OnBigFeedbackLogsResponse, weak_ptr_factory_.GetWeakPtr(), pipe_reader->AsWeakPtr())); } @@ -341,16 +318,21 @@ class DebugDaemonClientImpl : public DebugDaemonClient { base::Bind(&DebugDaemonClientImpl::OnIOComplete, weak_ptr_factory_.GetWeakPtr()))); - base::File pipe_write_end = pipe_reader_->StartIO(); - // Create dbus::FileDescriptor on the worker thread; on return we'll - // issue the D-Bus request to stop tracing and collect results. - base::PostTaskAndReplyWithResult( - stop_agent_tracing_task_runner_.get(), FROM_HERE, - base::Bind(&CreateFileDescriptorForPipeWriteEnd, - base::Passed(&pipe_write_end)), - base::Bind( - &DebugDaemonClientImpl::OnCreateFileDescriptorRequestStopSystem, - weak_ptr_factory_.GetWeakPtr(), callback)); + base::ScopedFD pipe_write_end = pipe_reader_->StartIO(); + DCHECK(pipe_write_end.is_valid()); + // Issue the dbus request to stop system tracing + dbus::MethodCall method_call(debugd::kDebugdInterface, + debugd::kSystraceStop); + dbus::MessageWriter writer(&method_call); + writer.AppendFileDescriptor(pipe_write_end.get()); + + callback_ = callback; + + DVLOG(1) << "Requesting a systrace stop"; + debugdaemon_proxy_->CallMethod( + &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, + base::Bind(&DebugDaemonClientImpl::OnStopAgentTracing, + weak_ptr_factory_.GetWeakPtr())); } void SetStopAgentTracingTaskRunner( @@ -468,25 +450,6 @@ class DebugDaemonClientImpl : public DebugDaemonClient { } private: - // Called when a CheckValidity response is received. - void OnCheckValidityGetDebugLogs(bool is_compressed, - dbus::FileDescriptor* file_descriptor, - const GetDebugLogsCallback& callback) { - // Issue the dbus request to get debug logs. - dbus::MethodCall method_call(debugd::kDebugdInterface, - debugd::kDumpDebugLogs); - dbus::MessageWriter writer(&method_call); - writer.AppendBool(is_compressed); - writer.AppendFileDescriptor(*file_descriptor); - - debugdaemon_proxy_->CallMethod( - &method_call, - dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, - base::Bind(&DebugDaemonClientImpl::OnGetDebugLogs, - weak_ptr_factory_.GetWeakPtr(), - callback)); - } - // Called when a response for GetDebugLogs() is received. void OnGetDebugLogs(const GetDebugLogsCallback& callback, dbus::Response* response) { @@ -589,22 +552,6 @@ class DebugDaemonClientImpl : public DebugDaemonClient { return OnGetAllLogs(callback, response); } - void OnBigLogsPipeReaderReady(base::WeakPtr pipe_reader, - dbus::ScopedFileDescriptor file_descriptor) { - DCHECK(file_descriptor); - - dbus::MethodCall method_call(debugd::kDebugdInterface, - debugd::kGetBigFeedbackLogs); - dbus::MessageWriter writer(&method_call); - writer.AppendFileDescriptor(*file_descriptor); - - DVLOG(1) << "Requesting big feedback logs"; - debugdaemon_proxy_->CallMethod( - &method_call, kBigLogsDBusTimeoutMS, - base::Bind(&DebugDaemonClientImpl::OnBigFeedbackLogsResponse, - weak_ptr_factory_.GetWeakPtr(), pipe_reader)); - } - void OnBigFeedbackLogsResponse(base::WeakPtr pipe_reader, dbus::Response* response) { if (!response && pipe_reader.get()) { @@ -669,28 +616,6 @@ class DebugDaemonClientImpl : public DebugDaemonClient { callback.Run(response != NULL); } - // Called when a CheckValidity response is received. - void OnCreateFileDescriptorRequestStopSystem( - const StopAgentTracingCallback& callback, - dbus::ScopedFileDescriptor file_descriptor) { - DCHECK(file_descriptor); - - // Issue the dbus request to stop system tracing - dbus::MethodCall method_call( - debugd::kDebugdInterface, - debugd::kSystraceStop); - dbus::MessageWriter writer(&method_call); - writer.AppendFileDescriptor(*file_descriptor); - - callback_ = callback; - - DVLOG(1) << "Requesting a systrace stop"; - debugdaemon_proxy_->CallMethod( - &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, - base::Bind(&DebugDaemonClientImpl::OnStopAgentTracing, - weak_ptr_factory_.GetWeakPtr())); - } - // Called when a response for StopAgentTracing() is received. void OnStopAgentTracing(dbus::Response* response) { if (!response) { diff --git a/chromeos/dbus/debug_daemon_client.h b/chromeos/dbus/debug_daemon_client.h index 803500d489e265..20d2ae331443d7 100644 --- a/chromeos/dbus/debug_daemon_client.h +++ b/chromeos/dbus/debug_daemon_client.h @@ -17,7 +17,6 @@ #include "base/trace_event/tracing_agent.h" #include "chromeos/chromeos_export.h" #include "chromeos/dbus/dbus_client.h" -#include "dbus/file_descriptor.h" #include "third_party/cros_system_api/dbus/service_constants.h" namespace chromeos { @@ -33,12 +32,13 @@ class CHROMEOS_EXPORT DebugDaemonClient // - succeeded: was the logs stored successfully. typedef base::Callback GetDebugLogsCallback; - // Requests to store debug logs into |file| and calls |callback| + // Requests to store debug logs into |file_descriptor| and calls |callback| // when completed. Debug logs will be stored in the .tgz if // |is_compressed| is true, otherwise in logs will be stored in .tar format. + // This method duplicates |file_descriptor| so it's OK to close the FD without + // waiting for the result. virtual void DumpDebugLogs(bool is_compressed, - base::File file, - scoped_refptr task_runner, + int file_descriptor, const GetDebugLogsCallback& callback) = 0; // Called once SetDebugMode() is complete. Takes one parameter: @@ -98,11 +98,13 @@ class CHROMEOS_EXPORT DebugDaemonClient // seconds) and returns data collected over the passed |file_descriptor|. // |error_callback| is called if there is an error with the DBus call. // Note that quipper failures may occur after successfully running the DBus - // method. Such errors can be detected by |file_descriptor| being closed with - // no data written. + // method. Such errors can be detected by |file_descriptor| and all its + // duplicates being closed with no data written. + // This method duplicates |file_descriptor| so it's OK to close the FD without + // waiting for the result. virtual void GetPerfOutput(base::TimeDelta duration, const std::vector& perf_args, - dbus::ScopedFileDescriptor file_descriptor, + int file_descriptor, const DBusMethodErrorCallback& error_callback) = 0; // Callback type for GetScrubbedLogs(), GetAllLogs() or GetUserLogFiles(). diff --git a/chromeos/dbus/fake_debug_daemon_client.cc b/chromeos/dbus/fake_debug_daemon_client.cc index 3a7d4035fc0ffb..57680af6dc974a 100644 --- a/chromeos/dbus/fake_debug_daemon_client.cc +++ b/chromeos/dbus/fake_debug_daemon_client.cc @@ -17,7 +17,6 @@ #include "base/single_thread_task_runner.h" #include "base/threading/thread_task_runner_handle.h" #include "chromeos/chromeos_switches.h" -#include "dbus/file_descriptor.h" namespace { @@ -39,8 +38,7 @@ void FakeDebugDaemonClient::Init(dbus::Bus* bus) {} void FakeDebugDaemonClient::DumpDebugLogs( bool is_compressed, - base::File file, - scoped_refptr task_runner, + int file_descriptor, const GetDebugLogsCallback& callback) { callback.Run(true); } @@ -111,10 +109,8 @@ void FakeDebugDaemonClient::GetNetworkInterfaces( void FakeDebugDaemonClient::GetPerfOutput( base::TimeDelta duration, const std::vector& perf_args, - dbus::ScopedFileDescriptor file_descriptor, - const DBusMethodErrorCallback& error_callback) { - // Nothing to do but close the file descriptor, which its dtor will do. -} + int file_descriptor, + const DBusMethodErrorCallback& error_callback) {} void FakeDebugDaemonClient::GetScrubbedLogs(const GetLogsCallback& callback) { std::map sample; diff --git a/chromeos/dbus/fake_debug_daemon_client.h b/chromeos/dbus/fake_debug_daemon_client.h index 01738d15c25258..1099b5f86873ff 100644 --- a/chromeos/dbus/fake_debug_daemon_client.h +++ b/chromeos/dbus/fake_debug_daemon_client.h @@ -24,8 +24,7 @@ class CHROMEOS_EXPORT FakeDebugDaemonClient : public DebugDaemonClient { void Init(dbus::Bus* bus) override; void DumpDebugLogs(bool is_compressed, - base::File file, - scoped_refptr task_runner, + int file_descriptor, const GetDebugLogsCallback& callback) override; void SetDebugMode(const std::string& subsystem, const SetDebugModeCallback& callback) override; @@ -46,7 +45,7 @@ class CHROMEOS_EXPORT FakeDebugDaemonClient : public DebugDaemonClient { const GetNetworkInterfacesCallback& callback) override; void GetPerfOutput(base::TimeDelta duration, const std::vector& perf_args, - dbus::ScopedFileDescriptor file_descriptor, + int file_descriptor, const DBusMethodErrorCallback& error_callback) override; void GetScrubbedLogs(const GetLogsCallback& callback) override; void GetScrubbedBigLogs(const GetLogsCallback& callback) override; diff --git a/chromeos/dbus/lorgnette_manager_client.cc b/chromeos/dbus/lorgnette_manager_client.cc index c7feacdc745bf6..f3fd54130a73d8 100644 --- a/chromeos/dbus/lorgnette_manager_client.cc +++ b/chromeos/dbus/lorgnette_manager_client.cc @@ -72,9 +72,10 @@ class LorgnetteManagerClientImpl : public LorgnetteManagerClient { // Owned by the callback created in scan_to_string_completion->Start(). ScanToStringCompletion* scan_to_string_completion = new ScanToStringCompletion(); - base::File file; + base::ScopedFD fd; ScanImageToFileCallback file_callback = - scan_to_string_completion->Start(callback, &file); + scan_to_string_completion->Start(callback, &fd); + base::File file(fd.release()); ScanImageToFile(device_name, properties, file_callback, &file); } @@ -96,7 +97,7 @@ class LorgnetteManagerClientImpl : public LorgnetteManagerClient { // of |this| to a returned callback that can be handed to a // ScanImageToFile invocation. ScanImageToFileCallback Start(const ScanImageToStringCallback& callback, - base::File *file) { + base::ScopedFD* fd) { CHECK(!pipe_reader_.get()); const bool kTasksAreSlow = true; scoped_refptr task_runner = @@ -106,7 +107,7 @@ class LorgnetteManagerClientImpl : public LorgnetteManagerClient { task_runner, base::Bind(&ScanToStringCompletion::OnScanToStringDataCompleted, base::Unretained(this)))); - *file = pipe_reader_->StartIO(); + *fd = pipe_reader_->StartIO(); return base::Bind(&ScanToStringCompletion::OnScanToStringCompleted, base::Owned(this), callback); diff --git a/chromeos/dbus/pipe_reader.cc b/chromeos/dbus/pipe_reader.cc index 70b44b35d68802..b1a096fd062013 100644 --- a/chromeos/dbus/pipe_reader.cc +++ b/chromeos/dbus/pipe_reader.cc @@ -23,15 +23,15 @@ PipeReader::PipeReader(const scoped_refptr& task_runner, PipeReader::~PipeReader() { } -base::File PipeReader::StartIO() { +base::ScopedFD PipeReader::StartIO() { // Use a pipe to collect data int pipe_fds[2]; const int status = HANDLE_EINTR(pipe(pipe_fds)); if (status < 0) { PLOG(ERROR) << "pipe"; - return base::File(); + return base::ScopedFD(); } - base::File pipe_write_end(pipe_fds[1]); + base::ScopedFD pipe_write_end(pipe_fds[1]); // Pass ownership of pipe_fds[0] to data_stream_, which will close it. data_stream_.reset(new net::FileStream( base::File(pipe_fds[0]), task_runner_)); @@ -42,7 +42,7 @@ base::File PipeReader::StartIO() { base::Bind(&PipeReader::OnDataReady, weak_ptr_factory_.GetWeakPtr())); if (rv != net::ERR_IO_PENDING) { LOG(ERROR) << "Unable to post initial read"; - return base::File(); + return base::ScopedFD(); } return pipe_write_end; } diff --git a/chromeos/dbus/pipe_reader.h b/chromeos/dbus/pipe_reader.h index eb19382ea989df..3df735538602e1 100644 --- a/chromeos/dbus/pipe_reader.h +++ b/chromeos/dbus/pipe_reader.h @@ -9,7 +9,7 @@ #include #include "base/callback.h" -#include "base/files/file.h" +#include "base/files/scoped_file.h" #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" @@ -47,7 +47,7 @@ class CHROMEOS_EXPORT PipeReader { // On success data will automatically be accumulated into a string that // can be retrieved with PipeReader::data(). To shutdown collection delete // the instance and/or use PipeReader::OnDataReady(-1). - base::File StartIO(); + base::ScopedFD StartIO(); // Called when pipe data are available. Can also be used to shutdown // data collection by passing -1 for |byte_count|. diff --git a/dbus/message.cc b/dbus/message.cc index 4a84756c41fcb6..43e88e2286ad49 100644 --- a/dbus/message.cc +++ b/dbus/message.cc @@ -714,6 +714,11 @@ void MessageWriter::AppendVariantOfBasic(int dbus_type, const void* value) { CloseContainer(&variant_writer); } +void MessageWriter::AppendFileDescriptor(int value) { + CHECK(IsDBusTypeUnixFdSupported()); + AppendBasic(DBUS_TYPE_UNIX_FD, &value); // This duplicates the FD. +} + void MessageWriter::AppendFileDescriptor(const FileDescriptor& value) { CHECK(IsDBusTypeUnixFdSupported()); @@ -1016,6 +1021,18 @@ bool MessageReader::PopVariantOfBasic(int dbus_type, void* value) { return variant_reader.PopBasic(dbus_type, value); } +bool MessageReader::PopFileDescriptor(base::ScopedFD* value) { + CHECK(IsDBusTypeUnixFdSupported()); + + int fd = -1; + const bool success = PopBasic(DBUS_TYPE_UNIX_FD, &fd); + if (!success) + return false; + + *value = base::ScopedFD(fd); + return true; +} + bool MessageReader::PopFileDescriptor(FileDescriptor* value) { CHECK(IsDBusTypeUnixFdSupported()); diff --git a/dbus/message.h b/dbus/message.h index 0aa010ccde0b74..272a740e4430b9 100644 --- a/dbus/message.h +++ b/dbus/message.h @@ -13,6 +13,7 @@ #include #include +#include "base/files/scoped_file.h" #include "base/macros.h" #include "dbus/dbus_export.h" #include "dbus/file_descriptor.h" @@ -285,6 +286,13 @@ class CHROME_DBUS_EXPORT MessageWriter { void AppendDouble(double value); void AppendString(const std::string& value); void AppendObjectPath(const ObjectPath& value); + + // Appends a file descriptor to the message. + // The FD will be duplicated so you still have to close the original FD. + void AppendFileDescriptor(int value); + + // DEPRECATED: Use the method with the same name above instead. + // TODO(hashimoto): Remove this. crbug.com/621841 void AppendFileDescriptor(const FileDescriptor& value); // Opens an array. The array contents can be added to the array with @@ -398,6 +406,10 @@ class CHROME_DBUS_EXPORT MessageReader { bool PopDouble(double* value); bool PopString(std::string* value); bool PopObjectPath(ObjectPath* value); + bool PopFileDescriptor(base::ScopedFD* value); + + // DEPRECATED: Use the method with the same name above. + // TODO(hashimoto): Remove this. crbug.com/621841 bool PopFileDescriptor(FileDescriptor* value); // Sets up the given message reader to read an array at the current