[DeviceService] Update device::SendBuffer to avoid unnecessary vector…

… copy - Makes device::SendBuffer ctor take 'const std::vector<uint8_t>&' rather than 'const std::vector<char>&' - Changes type of its member |data_| from 'const std::vector<char>' to 'const std::vector<uint8_t>'. BUG=749515 TBR=charliea@chromium.org Change-Id: I27424642d866174be84f7916f260b1f5e6b769a8 Reviewed-on: https://chromium-review.googlesource.com/620207 Commit-Queue: Han Leon <leon.han@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#496571}
Denger-Network · Aug 23, 2017 · 4c0e4cb · 4c0e4cb
1 parent a40ace7
commit 4c0e4cb
Show file tree

Hide file tree

Showing 12 changed files with 39 additions and 41 deletions.
diff --git a/device/serial/buffer.cc b/device/serial/buffer.cc
@@ -14,13 +14,13 @@ ReadOnlyBuffer::~ReadOnlyBuffer() {
 WritableBuffer::~WritableBuffer() {
 }
 
-SendBuffer::SendBuffer(const std::vector<char>& data,
+SendBuffer::SendBuffer(const std::vector<uint8_t>& data,
                        SendCompleteCallback callback)
     : data_(data), callback_(std::move(callback)) {}
 
 SendBuffer::~SendBuffer() {}
 
-const char* SendBuffer::GetData() {
+const uint8_t* SendBuffer::GetData() {
   return data_.data();
 }
 

diff --git a/device/serial/buffer.h b/device/serial/buffer.h
@@ -20,7 +20,7 @@ namespace device {
 class ReadOnlyBuffer {
  public:
   virtual ~ReadOnlyBuffer();
-  virtual const char* GetData() = 0;
+  virtual const uint8_t* GetData() = 0;
   virtual uint32_t GetSize() = 0;
   virtual void Done(uint32_t bytes_read) = 0;
   virtual void DoneWithError(uint32_t bytes_read, int32_t error) = 0;
@@ -45,16 +45,16 @@ class SendBuffer : public device::ReadOnlyBuffer {
  public:
   using SendCompleteCallback =
       base::OnceCallback<void(int, device::mojom::SerialSendError)>;
-  SendBuffer(const std::vector<char>& data, SendCompleteCallback callback);
+  SendBuffer(const std::vector<uint8_t>& data, SendCompleteCallback callback);
   ~SendBuffer() override;
 
-  const char* GetData() override;
+  const uint8_t* GetData() override;
   uint32_t GetSize() override;
   void Done(uint32_t bytes_read) override;
   void DoneWithError(uint32_t bytes_read, int32_t error) override;
 
  private:
-  const std::vector<char> data_;
+  const std::vector<uint8_t> data_;
   SendCompleteCallback callback_;
 };
 

diff --git a/device/serial/serial_io_handler.h b/device/serial/serial_io_handler.h
@@ -176,7 +176,7 @@ class SerialIoHandler : public base::RefCountedThreadSafe<SerialIoHandler> {
 
   bool read_canceled() const { return read_canceled_; }
 
-  const char* pending_write_buffer() const {
+  const uint8_t* pending_write_buffer() const {
     return pending_write_buffer_ ? pending_write_buffer_->GetData() : NULL;
   }
 

diff --git a/device/serial/test_serial_io_handler.cc b/device/serial/test_serial_io_handler.cc
@@ -42,8 +42,8 @@ void TestSerialIoHandler::ReadImpl() {
 
   size_t num_bytes =
       std::min(buffer_.size(), static_cast<size_t>(pending_read_buffer_len()));
-  memcpy(pending_read_buffer(), buffer_.c_str(), num_bytes);
-  buffer_ = buffer_.substr(num_bytes);
+  memcpy(pending_read_buffer(), buffer_.data(), num_bytes);
+  buffer_.erase(buffer_.begin(), buffer_.begin() + num_bytes);
   ReadCompleted(static_cast<uint32_t>(num_bytes),
                 mojom::SerialReceiveError::NONE);
 }
@@ -57,7 +57,8 @@ void TestSerialIoHandler::WriteImpl() {
     std::move(send_callback_).Run();
     return;
   }
-  buffer_ += std::string(pending_write_buffer(), pending_write_buffer_len());
+  buffer_.insert(buffer_.end(), pending_write_buffer(),
+                 pending_write_buffer() + pending_write_buffer_len());
   WriteCompleted(pending_write_buffer_len(), mojom::SerialSendError::NONE);
   if (pending_read_buffer())
     ReadImpl();

diff --git a/device/serial/test_serial_io_handler.h b/device/serial/test_serial_io_handler.h
@@ -60,7 +60,7 @@ class TestSerialIoHandler : public SerialIoHandler {
   bool dtr_;
   bool rts_;
   mutable int flushes_;
-  std::string buffer_;
+  std::vector<uint8_t> buffer_;
   base::OnceClosure send_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(TestSerialIoHandler);

diff --git a/extensions/browser/api/serial/serial_apitest.cc b/extensions/browser/api/serial/serial_apitest.cc
@@ -88,13 +88,12 @@ class FakeSerialIoHandler : public device::mojom::SerialIoHandler {
   void Write(const std::vector<uint8_t>& data,
              WriteCallback callback) override {
     test_io_handler_->Write(base::MakeUnique<device::SendBuffer>(
-        std::vector<char>(data.data(), data.data() + data.size()),
-        base::BindOnce(
-            [](WriteCallback callback, int bytes_sent,
-               device::mojom::SerialSendError error) {
-              std::move(callback).Run(bytes_sent, error);
-            },
-            std::move(callback))));
+        data, base::BindOnce(
+                  [](WriteCallback callback, int bytes_sent,
+                     device::mojom::SerialSendError error) {
+                    std::move(callback).Run(bytes_sent, error);
+                  },
+                  std::move(callback))));
   }
   void CancelRead(device::mojom::SerialReceiveError reason) override {
     test_io_handler_->CancelRead(reason);

diff --git a/services/device/serial/serial_io_handler_impl.cc b/services/device/serial/serial_io_handler_impl.cc
@@ -50,13 +50,12 @@ void SerialIoHandlerImpl::Read(uint32_t bytes, ReadCallback callback) {
 void SerialIoHandlerImpl::Write(const std::vector<uint8_t>& data,
                                 WriteCallback callback) {
   io_handler_->Write(base::MakeUnique<SendBuffer>(
-      std::vector<char>(data.data(), data.data() + data.size()),
-      base::BindOnce(
-          [](WriteCallback callback, int bytes_sent,
-             mojom::SerialSendError error) {
-            std::move(callback).Run(bytes_sent, error);
-          },
-          std::move(callback))));
+      data, base::BindOnce(
+                [](WriteCallback callback, int bytes_sent,
+                   mojom::SerialSendError error) {
+                  std::move(callback).Run(bytes_sent, error);
+                },
+                std::move(callback))));
 }
 
 void SerialIoHandlerImpl::CancelRead(mojom::SerialReceiveError reason) {

diff --git a/tools/battor_agent/battor_connection_impl.cc b/tools/battor_agent/battor_connection_impl.cc
@@ -122,11 +122,11 @@ void BattOrConnectionImpl::Close() {
 void BattOrConnectionImpl::SendBytes(BattOrMessageType type,
                                      const void* buffer,
                                      size_t bytes_to_send) {
-  const char* bytes = reinterpret_cast<const char*>(buffer);
+  const uint8_t* bytes = reinterpret_cast<const uint8_t*>(buffer);
 
   // Reserve a send buffer with enough extra bytes for the start, type, end, and
   // escape bytes.
-  vector<char> data;
+  vector<uint8_t> data;
   data.reserve(2 * bytes_to_send + 3);
 
   data.push_back(BATTOR_CONTROL_BYTE_START);
@@ -143,7 +143,7 @@ void BattOrConnectionImpl::SendBytes(BattOrMessageType type,
 
   data.push_back(BATTOR_CONTROL_BYTE_END);
 
-  LogSerial(StringPrintf("Bytes sent: %s.", CharVectorToString(data).c_str()));
+  LogSerial(StringPrintf("Bytes sent: %s.", ByteVectorToString(data).c_str()));
 
   pending_write_length_ = data.size();
   io_handler_->Write(base::MakeUnique<device::SendBuffer>(

diff --git a/tools/battor_agent/battor_connection_impl_unittest.cc b/tools/battor_agent/battor_connection_impl_unittest.cc
@@ -96,9 +96,9 @@ class BattOrConnectionImplTest : public testing::Test,
 
   // Writes the specified bytes directly to the serial connection.
   void SendBytesRaw(const char* data, uint16_t bytes_to_send) {
-    std::vector<char> data_vector(data, data + bytes_to_send);
     connection_->GetIoHandler()->Write(base::MakeUnique<device::SendBuffer>(
-        data_vector, base::BindOnce(&NullWriteCallback)));
+        std::vector<uint8_t>(data, data + bytes_to_send),
+        base::BindOnce(&NullWriteCallback)));
     task_runner_->RunUntilIdle();
   }
 

diff --git a/tools/battor_agent/serial_utils.cc b/tools/battor_agent/serial_utils.cc
@@ -6,13 +6,13 @@
 
 namespace battor {
 
-std::string CharVectorToString(const std::vector<char> data) {
+std::string ByteVectorToString(const std::vector<uint8_t>& data) {
   std::string s;
 
   // Reserve enough bytes for '0x', the two data characters, a space, and a null
   // terminating byte.
   char num_buff[6];
-  for (char d : data) {
+  for (uint8_t d : data) {
     // We use sprintf because stringstream's hex support wants to print our
     // characters as signed.
     sprintf(num_buff, "0x%02hhx ", d);
@@ -23,7 +23,7 @@ std::string CharVectorToString(const std::vector<char> data) {
 }
 
 std::string CharArrayToString(const char* bytes, size_t len) {
-  return CharVectorToString(std::vector<char>(bytes, bytes + len));
+  return ByteVectorToString(std::vector<uint8_t>(bytes, bytes + len));
 }
 
 }  // namespace battor
diff --git a/tools/battor_agent/serial_utils.h b/tools/battor_agent/serial_utils.h
@@ -13,7 +13,7 @@ namespace battor {
 // Prints |bytes| as a space separated list of hex numbers (e.g. {'A', 'J'}
 // would return "0x41 0x4A").
 std::string CharArrayToString(const char* bytes, size_t len);
-std::string CharVectorToString(const std::vector<char> bytes);
+std::string ByteVectorToString(const std::vector<uint8_t>& bytes);
 
 }  // namespace battor
 

diff --git a/tools/battor_agent/serial_utils_unittest.cc b/tools/battor_agent/serial_utils_unittest.cc
@@ -10,17 +10,16 @@ using namespace testing;
 
 namespace battor {
 
-TEST(SerialUtilsTest, CharVectorToStringLengthZero) {
-  EXPECT_EQ("", CharVectorToString(std::vector<char>()));
+TEST(SerialUtilsTest, ByteVectorToStringLengthZero) {
+  EXPECT_EQ("", ByteVectorToString(std::vector<uint8_t>()));
 }
 
-TEST(SerialUtilsTest, CharVectorToStringLengthOne) {
-  EXPECT_EQ("0x41", CharVectorToString(std::vector<char>({'A'})));
+TEST(SerialUtilsTest, ByteVectorToStringLengthOne) {
+  EXPECT_EQ("0x41", ByteVectorToString(std::vector<uint8_t>({'A'})));
 }
 
-TEST(SerialUtilsTest, CharVectorToStringLengthTwo) {
-  EXPECT_EQ("0x41 0x4a",
-            CharVectorToString(std::vector<char>({'A', 'J'})));
+TEST(SerialUtilsTest, ByteVectorToStringLengthTwo) {
+  EXPECT_EQ("0x41 0x4a", ByteVectorToString(std::vector<uint8_t>({'A', 'J'})));
 }
 
 TEST(SerialUtilsTest, CharArrayToStringLengthOne) {