No dbus::FileDescriptor in DebugDaemonClient

chromeos/dbus/debug_daemon_client.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 "chromeos/dbus/debug_daemon_client.h"
 
 #include <fcntl.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <unistd.h>
@@ skipping 21 matching lines @@
 namespace chromeos {
 
 namespace {
 
 const char kCrOSTracingAgentName[] = "cros";
 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<void(dbus::ScopedFileDescriptor)>;
-
 // Used in DebugDaemonClient::EmptyStopAgentTracingCallback().
 void EmptyStopAgentTracingCallbackBody(
     const std::string& agent_name,
     const std::string& events_label,
     const scoped_refptr<base::RefCountedString>& 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
 // the GetLogsCallback |callback| passing the deserialized logs data back to
 // the requester.
 class PipeReaderWrapper : public base::SupportsWeakPtr<PipeReaderWrapper> {
  public:
   explicit PipeReaderWrapper(const DebugDaemonClient::GetLogsCallback& callback)
       : task_runner_(
             base::WorkerPool::GetTaskRunner(true /** tasks_are_slow */)),
         pipe_reader_(task_runner_,
                      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;
     pipe_reader_.GetData(&pipe_data);
     JSONStringValueDeserializer json_reader(pipe_data);
 
     std::map<std::string, std::string> data;
     const base::DictionaryValue* dictionary = nullptr;
     std::unique_ptr<base::Value> logs_value =
         json_reader.Deserialize(nullptr, nullptr);
@@ skipping 29 matching lines @@
 
 // The DebugDaemonClient implementation used in production.
 class DebugDaemonClientImpl : public DebugDaemonClient {
  public:
   DebugDaemonClientImpl() : debugdaemon_proxy_(NULL), weak_ptr_factory_(this) {}
 
   ~DebugDaemonClientImpl() override {}
 
   // DebugDaemonClient override.
   void DumpDebugLogs(bool is_compressed,
-                     base::File file,
-                     scoped_refptr<base::TaskRunner> 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,
                     const SetDebugModeCallback& callback) override {
     dbus::MethodCall method_call(debugd::kDebugdInterface,
                                  debugd::kSetDebugMode);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(subsystem);
     debugdaemon_proxy_->CallMethod(
         &method_call,
@@ skipping 69 matching lines @@
     debugdaemon_proxy_->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(&DebugDaemonClientImpl::OnGetNetworkInterfaces,
                    weak_ptr_factory_.GetWeakPtr(),
                    callback));
   }
 
   void GetPerfOutput(base::TimeDelta duration,
                      const std::vector<std::string>& perf_args,
-                     dbus::ScopedFileDescriptor file_descriptor,
+                     int file_descriptor,
                      const DBusMethodErrorCallback& error_callback) override {
     DCHECK(file_descriptor);
     dbus::MethodCall method_call(debugd::kDebugdInterface,
                                  debugd::kGetPerfOutputFd);
     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,
         dbus::ObjectProxy::EmptyResponseCallback(),
         base::Bind(&DebugDaemonClientImpl::OnDBusMethodError,
                    weak_ptr_factory_.GetWeakPtr(), error_callback));
   }
 
   void GetScrubbedLogs(const GetLogsCallback& callback) override {
     dbus::MethodCall method_call(debugd::kDebugdInterface,
                                  debugd::kGetFeedbackLogs);
     debugdaemon_proxy_->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(&DebugDaemonClientImpl::OnGetAllLogs,
                    weak_ptr_factory_.GetWeakPtr(),
                    callback));
   }
 
   void GetScrubbedBigLogs(const GetLogsCallback& callback) override {
     // The PipeReaderWrapper is a self-deleting object; we don't have to worry
     // about ownership or lifetime. We need to create a new one for each Big
     // Logs requests in order to queue these requests. One request can take a
     // 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()));
   }
 
   void GetAllLogs(const GetLogsCallback& callback) override {
     dbus::MethodCall method_call(debugd::kDebugdInterface,
                                  debugd::kGetAllLogs);
     debugdaemon_proxy_->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(&DebugDaemonClientImpl::OnGetAllLogs,
@@ skipping 42 matching lines @@
     if (pipe_reader_ != NULL) {
       LOG(ERROR) << "Busy doing StopSystemTracing";
       return;
     }
 
     pipe_reader_.reset(
         new PipeReaderForString(stop_agent_tracing_task_runner_,
                                 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(
       scoped_refptr<base::TaskRunner> task_runner) override {
     stop_agent_tracing_task_runner_ = task_runner;
   }
 
   void TestICMP(const std::string& ip_address,
                 const TestICMPCallback& callback) override {
     dbus::MethodCall method_call(debugd::kDebugdInterface,
@@ skipping 97 matching lines @@
   }
 
  protected:
   void Init(dbus::Bus* bus) override {
     debugdaemon_proxy_ =
         bus->GetObjectProxy(debugd::kDebugdServiceName,
                             dbus::ObjectPath(debugd::kDebugdServicePath));
   }
 
  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) {
     if (!response) {
       LOG(ERROR) << "Failed to get debug logs";
       callback.Run(false);
       return;
     }
     callback.Run(true);
   }
@@ skipping 82 matching lines @@
       logs[key] = value;
     }
     callback.Run(!sub_reader.HasMoreData() && !broken, logs);
   }
 
   void OnGetUserLogFiles(const GetLogsCallback& callback,
                          dbus::Response* response) {
     return OnGetAllLogs(callback, response);
   }
 
-  void OnBigLogsPipeReaderReady(base::WeakPtr<PipeReaderWrapper> 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<PipeReaderWrapper> pipe_reader,
                                  dbus::Response* response) {
     if (!response && pipe_reader.get()) {
       // We need to terminate the data stream if an error occurred while the
       // pipe reader is still waiting on read.
       pipe_reader->TerminateStream();
     }
   }
 
   // Called when a response for a simple start is received.
@@ skipping 44 matching lines @@
 
   void OnRemoveRootfsVerification(
       const EnableDebuggingCallback& callback,
       dbus::Response* response) {
     if (callback.is_null())
       return;
 
     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) {
       LOG(ERROR) << "Failed to request systrace stop";
       // If debugd crashes or completes I/O before this message is processed
       // then pipe_reader_ can be NULL, see OnIOComplete().
       if (pipe_reader_.get())
         pipe_reader_->OnDataReady(-1); // terminate data stream
     }
     // NB: requester is signaled when i/o completes
@@ skipping 36 matching lines @@
 DebugDaemonClient::EmptyStopAgentTracingCallback() {
   return base::Bind(&EmptyStopAgentTracingCallbackBody);
 }
 
 // static
 DebugDaemonClient* DebugDaemonClient::Create() {
   return new DebugDaemonClientImpl();
 }
 
 }  // namespace chromeos