libbrillo: remove all uses of dbus::FileDescriptor

We don't support this anymore and we have to remove it for the
libchrome uprev anyway.

dbus::FileDescriptor is now dead.

CQ-DEPEND=CL:1000494
BUG=b:37434548
TEST=unit tests, build_packages

Change-Id: I1ca743a2b6f2875eb97c6c95faf4b2717a21e9d1
Reviewed-on: https://chromium-review.googlesource.com/1000725
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

5 files changed, 4 insertions, 120 deletions

diff --git a/brillo/dbus/data_serialization.cc b/brillo/dbus/data_serialization.cc
index 35ab00a..4cae471 100644
--- a/brillo/dbus/data_serialization.cc
+++ b/brillo/dbus/data_serialization.cc
@@ -62,11 +62,6 @@ void AppendValueToWriter(dbus::MessageWriter* writer,
 }
 
 void AppendValueToWriter(dbus::MessageWriter* writer,
-                         const dbus::FileDescriptor& value) {
-  writer->AppendFileDescriptor(value);
-}
-
-void AppendValueToWriter(dbus::MessageWriter* writer,
                          const FileDescriptor& value) {
   writer->AppendFileDescriptor(value.get());
 }
@@ -145,16 +140,6 @@ bool PopValueFromReader(dbus::MessageReader* reader, dbus::ObjectPath* value) {
 }
 
 bool PopValueFromReader(dbus::MessageReader* reader,
-                        dbus::FileDescriptor* value) {
-  dbus::MessageReader variant_reader(nullptr);
-  bool ok = details::DescendIntoVariantIfPresent(&reader, &variant_reader) &&
-            reader->PopFileDescriptor(value);
-  if (ok)
-    value->CheckValidity();
-  return ok;
-}
-
-bool PopValueFromReader(dbus::MessageReader* reader,
                         base::ScopedFD* value) {
   dbus::MessageReader variant_reader(nullptr);
   return details::DescendIntoVariantIfPresent(&reader, &variant_reader) &&
@@ -321,7 +306,7 @@ bool PopValueFromReader(dbus::MessageReader* reader, brillo::Any* value) {
       return false;
     case dbus::Message::UNIX_FD:
       CHECK(dbus::IsDBusTypeUnixFdSupported()) << "UNIX_FD data not supported";
-      // dbus::FileDescriptor is not a copyable type. Cannot be returned via
+      // File descriptors don't use copyable types. Cannot be returned via
       // brillo::Any. Fail here.
       LOG(ERROR) << "Cannot return FileDescriptor via Any";
       return false;
diff --git a/brillo/dbus/data_serialization.h b/brillo/dbus/data_serialization.h
index e9265af..bad1f6f 100644
--- a/brillo/dbus/data_serialization.h
+++ b/brillo/dbus/data_serialization.h
@@ -36,8 +36,7 @@
 //               |     (UVW...)    |  std::tuple<U,V,W,...>
 //   DICT        |       a{KV}     |  std::map<K,V>
 //   VARIANT     |        v        |  brillo::Any
-//   UNIX_FD     |        h        |  dbus::FileDescriptor (deprecated)
-//               |                 |  brillo::dbus_utils::FileDescriptor (write)
+//   UNIX_FD     |        h        |  brillo::dbus_utils::FileDescriptor (write)
 //               |                 |  base::ScopedFD (read)
 //   SIGNATURE   |        g        |  (unsupported)
 //
@@ -415,27 +414,6 @@ struct DBusType<dbus::ObjectPath> {
   }
 };
 
-// dbus::FileDescriptor -------------------------------------------------------
-BRILLO_EXPORT void AppendValueToWriter(dbus::MessageWriter* writer,
-                                         const dbus::FileDescriptor& value);
-BRILLO_EXPORT bool PopValueFromReader(dbus::MessageReader* reader,
-                                        dbus::FileDescriptor* value);
-
-template<>
-struct DBusType<dbus::FileDescriptor> {
-  inline static std::string GetSignature() {
-    return DBUS_TYPE_UNIX_FD_AS_STRING;
-  }
-  inline static void Write(dbus::MessageWriter* writer,
-                           const dbus::FileDescriptor& value) {
-    AppendValueToWriter(writer, value);
-  }
-  inline static bool Read(dbus::MessageReader* reader,
-                          dbus::FileDescriptor* value) {
-    return PopValueFromReader(reader, value);
-  }
-};
-
 // brillo::dbus_utils::FileDescriptor/base::ScopedFD --------------------------
 BRILLO_EXPORT void AppendValueToWriter(dbus::MessageWriter* writer,
                                          const FileDescriptor& value);
diff --git a/brillo/dbus/data_serialization_unittest.cc b/brillo/dbus/data_serialization_unittest.cc
index 915195b..1b6975d 100644
--- a/brillo/dbus/data_serialization_unittest.cc
+++ b/brillo/dbus/data_serialization_unittest.cc
@@ -32,7 +32,6 @@ TEST(DBusUtils, Supported_BasicTypes) {
   EXPECT_TRUE(IsTypeSupported<double>::value);
   EXPECT_TRUE(IsTypeSupported<std::string>::value);
   EXPECT_TRUE(IsTypeSupported<ObjectPath>::value);
-  EXPECT_TRUE(IsTypeSupported<dbus::FileDescriptor>::value);
   EXPECT_TRUE(IsTypeSupported<FileDescriptor>::value);
   EXPECT_TRUE(IsTypeSupported<base::ScopedFD>::value);
   EXPECT_TRUE(IsTypeSupported<Any>::value);
@@ -90,7 +89,6 @@ TEST(DBusUtils, Signatures_BasicTypes) {
   EXPECT_EQ("d", GetDBusSignature<double>());
   EXPECT_EQ("s", GetDBusSignature<std::string>());
   EXPECT_EQ("o", GetDBusSignature<ObjectPath>());
-  EXPECT_EQ("h", GetDBusSignature<dbus::FileDescriptor>());
   EXPECT_EQ("h", GetDBusSignature<FileDescriptor>());
   EXPECT_EQ("h", GetDBusSignature<base::ScopedFD>());
   EXPECT_EQ("v", GetDBusSignature<Any>());
@@ -108,7 +106,6 @@ TEST(DBusUtils, Signatures_Arrays) {
   EXPECT_EQ("ad", GetDBusSignature<std::vector<double>>());
   EXPECT_EQ("as", GetDBusSignature<std::vector<std::string>>());
   EXPECT_EQ("ao", GetDBusSignature<std::vector<ObjectPath>>());
-  EXPECT_EQ("ah", GetDBusSignature<std::vector<dbus::FileDescriptor>>());
   EXPECT_EQ("ah", GetDBusSignature<std::vector<FileDescriptor>>());
   EXPECT_EQ("ah", GetDBusSignature<std::vector<base::ScopedFD>>());
   EXPECT_EQ("av", GetDBusSignature<std::vector<Any>>());
@@ -233,38 +230,6 @@ TEST(DBusUtils, AppendAndPopBasicDataTypes) {
 }
 
 // Check all basic types can be properly written and read.
-TEST(DBusUtils, AppendAndPopFileDescriptor_Deprecated) {
-  if (!dbus::IsDBusTypeUnixFdSupported()) {
-    LOG(WARNING) << "FD passing is not supported";
-    return;
-  }
-
-  std::unique_ptr<Response> message = Response::CreateEmpty();
-  MessageWriter writer(message.get());
-
-  // Append stdout.
-  dbus::FileDescriptor temp(1);
-  // Descriptor should not be valid until checked.
-  EXPECT_FALSE(temp.is_valid());
-  // NB: thread IO requirements not relevant for unit tests.
-  temp.CheckValidity();
-  EXPECT_TRUE(temp.is_valid());
-  AppendValueToWriter(&writer, temp);
-
-  EXPECT_EQ("h", message->GetSignature());
-
-  dbus::FileDescriptor fd_value;
-
-  MessageReader reader(message.get());
-  EXPECT_TRUE(reader.HasMoreData());
-  EXPECT_TRUE(PopValueFromReader(&reader, &fd_value));
-  EXPECT_FALSE(reader.HasMoreData());
-  // Descriptor is automatically checked for validity as part of
-  // PopValueFromReader() call.
-  EXPECT_TRUE(fd_value.is_valid());
-}
-
-// Check all basic types can be properly written and read.
 TEST(DBusUtils, AppendAndPopFileDescriptor) {
   if (!dbus::IsDBusTypeUnixFdSupported()) {
     LOG(WARNING) << "FD passing is not supported";
diff --git a/brillo/dbus/dbus_method_invoker.h b/brillo/dbus/dbus_method_invoker.h
index ff6b79e..19a58e6 100644
--- a/brillo/dbus/dbus_method_invoker.h
+++ b/brillo/dbus/dbus_method_invoker.h
@@ -144,7 +144,7 @@ inline std::unique_ptr<dbus::Response> CallMethodAndBlock(
 }
 
 namespace internal {
-// In order to support non-copyable dbus::FileDescriptor, we have this
+// In order to support non-copyable file descriptor types, we have this
 // internal::HackMove() helper function that does really nothing for normal
 // types but uses Pass() for file descriptors so we can move them out from
 // the temporaries created inside DBusParamReader<...>::Invoke().
@@ -159,9 +159,6 @@ inline const T& HackMove(const T& val) {
 // inside DBusParamReader<...>::Invoke() and is temporary in nature, so it is
 // safe to move the file descriptor out of |val|. That's why we are doing
 // const_cast here. It is a bit hacky, but there is no negative side effects.
-inline dbus::FileDescriptor HackMove(const dbus::FileDescriptor& val) {
-  return std::move(const_cast<dbus::FileDescriptor&>(val));
-}
 inline base::ScopedFD HackMove(const base::ScopedFD& val) {
   return std::move(const_cast<base::ScopedFD&>(val));
 }
diff --git a/brillo/dbus/dbus_method_invoker_unittest.cc b/brillo/dbus/dbus_method_invoker_unittest.cc
index ef20cc7..08d8a22 100644
--- a/brillo/dbus/dbus_method_invoker_unittest.cc
+++ b/brillo/dbus/dbus_method_invoker_unittest.cc
@@ -35,7 +35,6 @@ const char kTestMethod1[] = "TestMethod1";
 const char kTestMethod2[] = "TestMethod2";
 const char kTestMethod3[] = "TestMethod3";
 const char kTestMethod4[] = "TestMethod4";
-const char kTestMethod5[] = "TestMethod5";
 
 class DBusMethodInvokerTest : public testing::Test {
  public:
@@ -89,18 +88,6 @@ class DBusMethodInvokerTest : public testing::Test {
           return response.release();
         }
       } else if (method_call->GetMember() == kTestMethod4) {
-        // DEPRECATED. will be removed after libchrome uprev b/37434548
-        method_call->SetSerial(123);
-        MessageReader reader(method_call);
-        dbus::FileDescriptor fd;
-        if (reader.PopFileDescriptor(&fd)) {
-          auto response = Response::CreateEmpty();
-          MessageWriter writer(response.get());
-          fd.CheckValidity();
-          writer.AppendFileDescriptor(fd);
-          return response.release();
-        }
-      } else if (method_call->GetMember() == kTestMethod5) {
         method_call->SetSerial(123);
         MessageReader reader(method_call);
         base::ScopedFD fd;
@@ -142,25 +129,12 @@ class DBusMethodInvokerTest : public testing::Test {
     return result;
   }
 
-  // Sends a file descriptor received over D-Bus back to the caller.
-  dbus::FileDescriptor DeprecatedEchoFD(const dbus::FileDescriptor& fd_in) {
-    std::unique_ptr<dbus::Response> response =
-        brillo::dbus_utils::CallMethodAndBlock(mock_object_proxy_.get(),
-                                               kTestInterface, kTestMethod4,
-                                               nullptr, fd_in);
-    EXPECT_NE(nullptr, response.get());
-    dbus::FileDescriptor fd_out;
-    using brillo::dbus_utils::ExtractMethodCallResults;
-    EXPECT_TRUE(ExtractMethodCallResults(response.get(), nullptr, &fd_out));
-    return fd_out;
-  }
-
   // Sends a file descriptor received over D-Bus back to the caller using the
   // new types.
   base::ScopedFD EchoFD(int fd_in) {
     std::unique_ptr<dbus::Response> response =
         brillo::dbus_utils::CallMethodAndBlock(
-            mock_object_proxy_.get(), kTestInterface, kTestMethod5,
+            mock_object_proxy_.get(), kTestInterface, kTestMethod4,
             nullptr, brillo::dbus_utils::FileDescriptor{fd_in});
     EXPECT_NE(nullptr, response.get());
     base::ScopedFD fd_out;
@@ -200,21 +174,6 @@ TEST_F(DBusMethodInvokerTest, TestProtobuf) {
   EXPECT_EQ("bar", resp.bar());
 }
 
-TEST_F(DBusMethodInvokerTest, TestFileDescriptors_Deprecated) {
-  // Passing a file descriptor over D-Bus would effectively duplicate the fd.
-  // So the resulting file descriptor value would be different but it still
-  // should be valid.
-  dbus::FileDescriptor fd_stdin(0);
-  fd_stdin.CheckValidity();
-  EXPECT_NE(fd_stdin.value(), DeprecatedEchoFD(fd_stdin).value());
-  dbus::FileDescriptor fd_stdout(1);
-  fd_stdout.CheckValidity();
-  EXPECT_NE(fd_stdout.value(), DeprecatedEchoFD(fd_stdout).value());
-  dbus::FileDescriptor fd_stderr(2);
-  fd_stderr.CheckValidity();
-  EXPECT_NE(fd_stderr.value(), DeprecatedEchoFD(fd_stderr).value());
-}
-
 TEST_F(DBusMethodInvokerTest, TestFileDescriptors) {
   // Passing a file descriptor over D-Bus would effectively duplicate the fd.
   // So the resulting file descriptor value would be different but it still