Cleanup VpnServices interface with shill client

This CL replaces an argument from brittle pointer and length
to a string.

TBR=stevenjb@chromium.org

BUG=407541

Review URL: https://codereview.chromium.org/768723003

Cr-Commit-Position: refs/heads/master@{#306106}

5 files changed, 22 insertions, 20 deletions

diff --git a/chromeos/dbus/fake_shill_third_party_vpn_driver_client.cc b/chromeos/dbus/fake_shill_third_party_vpn_driver_client.cc
index e8d370b..8be3db3 100644
--- a/chromeos/dbus/fake_shill_third_party_vpn_driver_client.cc
+++ b/chromeos/dbus/fake_shill_third_party_vpn_driver_client.cc
@@ -72,8 +72,7 @@ void FakeShillThirdPartyVpnDriverClient::OnPacketReceived(
     return;
   }
 
-  it->second->OnPacketReceived(reinterpret_cast<const uint8_t*>(packet.data()),
-                               packet.length());
+  it->second->OnPacketReceived(packet);
 }
 
 void FakeShillThirdPartyVpnDriverClient::OnPlatformMessage(
diff --git a/chromeos/dbus/shill_third_party_vpn_driver_client.cc b/chromeos/dbus/shill_third_party_vpn_driver_client.cc
index b1f3b73..c3392cd 100644
--- a/chromeos/dbus/shill_third_party_vpn_driver_client.cc
+++ b/chromeos/dbus/shill_third_party_vpn_driver_client.cc
@@ -4,6 +4,8 @@
 
 #include "chromeos/dbus/shill_third_party_vpn_driver_client.h"
 
+#include <string>
+
 #include "base/bind.h"
 #include "chromeos/dbus/shill_third_party_vpn_observer.h"
 #include "dbus/bus.h"
@@ -265,8 +267,10 @@ void ShillThirdPartyVpnDriverClientImpl::OnPacketReceived(
   dbus::MessageReader reader(signal);
   const uint8_t* data = nullptr;
   size_t length = 0;
-  if (reader.PopArrayOfBytes(&data, &length))
-    helper_info->observer()->OnPacketReceived(data, length);
+  if (reader.PopArrayOfBytes(&data, &length)) {
+    helper_info->observer()->OnPacketReceived(
+        std::string(reinterpret_cast<const char*>(data), length));
+  }
 }
 
 // static
diff --git a/chromeos/dbus/shill_third_party_vpn_driver_client_unittest.cc b/chromeos/dbus/shill_third_party_vpn_driver_client_unittest.cc
index b74e153..2501f53 100644
--- a/chromeos/dbus/shill_third_party_vpn_driver_client_unittest.cc
+++ b/chromeos/dbus/shill_third_party_vpn_driver_client_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <string>
+
 #include "base/bind.h"
 #include "chromeos/dbus/shill_client_unittest_base.h"
 #include "chromeos/dbus/shill_third_party_vpn_driver_client.h"
@@ -20,7 +22,7 @@ class MockShillThirdPartyVpnObserver : public ShillThirdPartyVpnObserver {
  public:
   MockShillThirdPartyVpnObserver() {}
   ~MockShillThirdPartyVpnObserver() override {}
-  MOCK_METHOD2(OnPacketReceived, void(const uint8_t* data, size_t length));
+  MOCK_METHOD1(OnPacketReceived, void(const std::string& data));
   MOCK_METHOD1(OnPlatformMessage, void(uint32_t message));
 };
 
@@ -57,7 +59,7 @@ class ShillThirdPartyVpnDriverClientTest : public ShillClientUnittestBase {
 TEST_F(ShillThirdPartyVpnDriverClientTest, PlatformSignal) {
   uint32_t connected_state = 123456;
   const int kPacketSize = 5;
-  uint8_t data[kPacketSize] = {};
+  std::string data_packet(1, kPacketSize);
   dbus::Signal pmessage_signal(shill::kFlimflamThirdPartyVpnInterface,
                                shill::kOnPlatformMessageFunction);
   {
@@ -69,13 +71,15 @@ TEST_F(ShillThirdPartyVpnDriverClientTest, PlatformSignal) {
                                 shill::kOnPacketReceivedFunction);
   {
     dbus::MessageWriter writer(&preceived_signal);
-    writer.AppendArrayOfBytes(data, kPacketSize);
+    writer.AppendArrayOfBytes(
+        reinterpret_cast<const unsigned char*>(data_packet.data()),
+        data_packet.size());
   }
 
   // Expect each signal to be triggered once.
   MockShillThirdPartyVpnObserver observer;
   EXPECT_CALL(observer, OnPlatformMessage(connected_state)).Times(1);
-  EXPECT_CALL(observer, OnPacketReceived(_, kPacketSize)).Times(1);
+  EXPECT_CALL(observer, OnPacketReceived(data_packet)).Times(1);
 
   client_->AddShillThirdPartyVpnObserver(kExampleIPConfigPath, &observer);
 
@@ -106,7 +110,7 @@ TEST_F(ShillThirdPartyVpnDriverClientTest, PlatformSignal) {
 
   // Check after removing the observer that there is no further signals.
   EXPECT_CALL(observer, OnPlatformMessage(connected_state)).Times(0);
-  EXPECT_CALL(observer, OnPacketReceived(_, kPacketSize)).Times(0);
+  EXPECT_CALL(observer, OnPacketReceived(data_packet)).Times(0);
 
   // Run the signal callback.
   SendPlatformMessageSignal(&pmessage_signal);
diff --git a/chromeos/dbus/shill_third_party_vpn_observer.h b/chromeos/dbus/shill_third_party_vpn_observer.h
index 5b0e013..a01eb97 100644
--- a/chromeos/dbus/shill_third_party_vpn_observer.h
+++ b/chromeos/dbus/shill_third_party_vpn_observer.h
@@ -6,6 +6,7 @@
 #define CHROMEOS_DBUS_SHILL_THIRD_PARTY_VPN_OBSERVER_H_
 
 #include <stdint.h>
+#include <string>
 
 namespace chromeos {
 
@@ -13,11 +14,7 @@ namespace chromeos {
 // ThirdPartyVpnAdaptor in Shill.
 class ShillThirdPartyVpnObserver {
  public:
-  // Ownership of |data| belongs to the caller, hence the contents should be
-  // consumed before the call returns, i.e., pointer should not be dereferenced
-  // after the function returns. The method takes raw data pointer and length
-  // instead of a type like vector to avoid additional memcpys.
-  virtual void OnPacketReceived(const uint8_t* data, size_t length) = 0;
+  virtual void OnPacketReceived(const std::string& data) = 0;
   virtual void OnPlatformMessage(uint32_t message) = 0;
 
  protected:
diff --git a/extensions/browser/api/vpn_provider/vpn_service.cc b/extensions/browser/api/vpn_provider/vpn_service.cc
index 65be2e3..1a618d6 100644
--- a/extensions/browser/api/vpn_provider/vpn_service.cc
+++ b/extensions/browser/api/vpn_provider/vpn_service.cc
@@ -71,8 +71,7 @@ class VpnService::VpnConfiguration : public ShillThirdPartyVpnObserver {
   const std::string& object_path() const { return object_path_; }
 
   // ShillThirdPartyVpnObserver:
-  // TODO(kaliamoorthi): Replace |data| and |length| with a string value.
-  void OnPacketReceived(const uint8_t* data, size_t length) override;
+  void OnPacketReceived(const std::string& data) override;
   void OnPlatformMessage(uint32_t message) override;
 
  private:
@@ -103,13 +102,12 @@ VpnService::VpnConfiguration::VpnConfiguration(
 VpnService::VpnConfiguration::~VpnConfiguration() {
 }
 
-void VpnService::VpnConfiguration::OnPacketReceived(const uint8_t* data,
-                                                    size_t length) {
+void VpnService::VpnConfiguration::OnPacketReceived(const std::string& data) {
   if (!vpn_service_) {
     return;
   }
-  scoped_ptr<base::ListValue> event_args = api_vpn::OnPacketReceived::Create(
-      std::string(reinterpret_cast<const char*>(data), length));
+  scoped_ptr<base::ListValue> event_args =
+      api_vpn::OnPacketReceived::Create(data);
   vpn_service_->SendSignalToExtension(
       extension_id_, api_vpn::OnPacketReceived::kEventName, event_args.Pass());
 }