ppapi: PPB_VpnProvider: Bind VpnService to the PPAPI Resource Host

extensions/browser/api/vpn_provider/vpn_service.cc

Index: extensions/browser/api/vpn_provider/vpn_service.cc
diff --git a/extensions/browser/api/vpn_provider/vpn_service.cc b/extensions/browser/api/vpn_provider/vpn_service.cc
index 672a683a165b7be113e71d590f7bf5f425388764..92ddf1719f54e1594ff730a5c31af792551b3cc7 100644
--- a/extensions/browser/api/vpn_provider/vpn_service.cc
+++ b/extensions/browser/api/vpn_provider/vpn_service.cc
@@ -14,6 +14,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
@@ -28,6 +29,8 @@
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_type_pattern.h"
+#include "content/public/browser/pepper_vpn_provider_resource_host_proxy.h"
+#include "content/public/browser/vpn_service_proxy.h"
 #include "crypto/sha2.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_registry.h"
@@ -63,6 +66,14 @@ class VpnService::VpnConfiguration : public ShillThirdPartyVpnObserver {
   }
   const std::string& object_path() const { return object_path_; }
 
+  void set_nacl_bound(

[emaxx, 2016/06/21 23:47:48]

Is there a situation possible in which nacl_bound==true along with NULL
pepper_vpn_provider_proxy would be passed here?
If not, then it would be clearer to rid of the boolean variable and member.

[adrian.belgun, 2016/06/22 08:23:46]

Done. Removed superfluous nacl_bound. 
It was a remnant from the old logic, without pepper_vpn_provider_proxy.

+      bool nacl_bound,
+      std::unique_ptr<content::PepperVpnProviderResourceHostProxy>
+          pepper_vpn_provider_proxy) {
+    nacl_bound_ = nacl_bound;
+    pepper_vpn_provider_proxy_ = std::move(pepper_vpn_provider_proxy);
+  }
+
   // ShillThirdPartyVpnObserver:
   void OnPacketReceived(const std::vector<char>& data) override;
   void OnPlatformMessage(uint32_t message) override;
@@ -73,6 +84,10 @@ class VpnService::VpnConfiguration : public ShillThirdPartyVpnObserver {
   const std::string key_;
   const std::string object_path_;
 
+  bool nacl_bound_;
+  std::unique_ptr<content::PepperVpnProviderResourceHostProxy>
+      pepper_vpn_provider_proxy_;
+
   std::string service_path_;
 
   base::WeakPtr<VpnService> vpn_service_;
@@ -89,8 +104,8 @@ VpnService::VpnConfiguration::VpnConfiguration(
       configuration_name_(configuration_name),
       key_(key),
       object_path_(shill::kObjectPathBase + key_),
-      vpn_service_(vpn_service) {
-}
+      nacl_bound_(false),
+      vpn_service_(vpn_service) {}
 
 VpnService::VpnConfiguration::~VpnConfiguration() {
 }
@@ -100,11 +115,17 @@ void VpnService::VpnConfiguration::OnPacketReceived(
   if (!vpn_service_) {
     return;
   }
-  std::unique_ptr<base::ListValue> event_args =
-      api_vpn::OnPacketReceived::Create(data);
-  vpn_service_->SendSignalToExtension(
-      extension_id_, extensions::events::VPN_PROVIDER_ON_PACKET_RECEIVED,
-      api_vpn::OnPacketReceived::kEventName, std::move(event_args));
+  // If the configuration was added via a PPAPI plugin

[emaxx, 2016/06/21 23:47:47]

nit: This comment looks unfinished, and describes only the the first branch of
the condition.

[adrian.belgun, 2016/06/22 08:23:46]

Done.

+  if (nacl_bound_) {
+    if (pepper_vpn_provider_proxy_)
+      pepper_vpn_provider_proxy_->SendOnPacketReceived(data);
+  } else {
+    std::unique_ptr<base::ListValue> event_args =
+        api_vpn::OnPacketReceived::Create(data);
+    vpn_service_->SendSignalToExtension(
+        extension_id_, extensions::events::VPN_PROVIDER_ON_PACKET_RECEIVED,
+        api_vpn::OnPacketReceived::kEventName, std::move(event_args));
+  }
 }
 
 void VpnService::VpnConfiguration::OnPlatformMessage(uint32_t message) {
@@ -121,6 +142,14 @@ void VpnService::VpnConfiguration::OnPlatformMessage(uint32_t message) {
   } else if (platform_message == api_vpn::PLATFORM_MESSAGE_DISCONNECTED ||
              platform_message == api_vpn::PLATFORM_MESSAGE_ERROR) {
     vpn_service_->SetActiveConfiguration(nullptr);
+
+    // Disconnect NaCl-bound configuration
+    if (nacl_bound_) {
+      nacl_bound_ = false;
+      if (pepper_vpn_provider_proxy_)
+        pepper_vpn_provider_proxy_->SendOnUnbind();
+      pepper_vpn_provider_proxy_.reset();
+    }
   }
 
   // TODO(kaliamoorthi): Update the lower layers to get the error message and
@@ -134,6 +163,65 @@ void VpnService::VpnConfiguration::OnPlatformMessage(uint32_t message) {
       api_vpn::OnPlatformMessage::kEventName, std::move(event_args));
 }
 
+class VpnService::VpnServiceProxyImpl : public content::VpnServiceProxy {
+ public:
+  VpnServiceProxyImpl(base::WeakPtr<VpnService> vpn_service);
+  ~VpnServiceProxyImpl() override;
+
+  void Bind(const std::string& extension_id,
+            const std::string& configuration_id,
+            const std::string& configuration_name,
+            const SuccessCallback& success,
+            const FailureCallback& failure,
+            std::unique_ptr<content::PepperVpnProviderResourceHostProxy>
+                pepper_vpn_provider_proxy) override;
+  void SendPacket(const std::string& extension_id,
+                  const std::vector<char>& data,
+                  const SuccessCallback& success,
+                  const FailureCallback& failure) override;
+
+ private:
+  base::WeakPtr<VpnService> vpn_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(VpnServiceProxyImpl);
+};
+
+VpnService::VpnServiceProxyImpl::VpnServiceProxyImpl(
+    base::WeakPtr<VpnService> vpn_service)
+    : vpn_service_(vpn_service) {}
+
+VpnService::VpnServiceProxyImpl::~VpnServiceProxyImpl() {}

[emaxx, 2016/06/21 23:47:47]

nit: Is this necessary?

[adrian.belgun, 2016/06/22 08:23:46]

Removed it. Technically not necessary, but added it to be consistent with the
above VpnConfiguration.

+
+void VpnService::VpnServiceProxyImpl::Bind(
+    const std::string& extension_id,
+    const std::string& configuration_id,
+    const std::string& configuration_name,
+    const SuccessCallback& success,
+    const FailureCallback& failure,
+    std::unique_ptr<content::PepperVpnProviderResourceHostProxy>
+        pepper_vpn_provider_proxy) {
+  if (!vpn_service_) {
+    NOTREACHED();
+    return;
+  }
+
+  vpn_service_->Bind(extension_id, configuration_id, configuration_name,
+                     success, failure, std::move(pepper_vpn_provider_proxy));
+}
+
+void VpnService::VpnServiceProxyImpl::SendPacket(
+    const std::string& extension_id,
+    const std::vector<char>& data,
+    const SuccessCallback& success,
+    const FailureCallback& failure) {
+  if (!vpn_service_) {
+    NOTREACHED();
+    return;
+  }
+
+  vpn_service_->SendPacket(extension_id, data, success, failure);
+}
+
 VpnService::VpnService(
     content::BrowserContext* browser_context,
     const std::string& userid_hash,
@@ -576,4 +664,47 @@ bool VpnService::DoesActiveConfigurationExistAndIsAccessAuthorized(
          active_configuration_->extension_id() == extension_id;
 }
 
+void VpnService::Bind(
+    const std::string& extension_id,
+    const std::string& configuration_id,
+    const std::string& configuration_name,
+    const SuccessCallback& success,
+    const FailureCallback& failure,
+    std::unique_ptr<content::PepperVpnProviderResourceHostProxy>
+        pepper_vpn_provider_proxy) {
+  // The ID is the configuration name for now. This may change in the future.
+  const std::string key = GetKey(extension_id, configuration_id);
+  if (!ContainsKey(key_to_configuration_map_, key)) {
+    failure.Run(std::string(), std::string("Unauthorized access 1."));
+    return;
+  }
+
+  VpnConfiguration* configuration = key_to_configuration_map_[key];
+  if (active_configuration_ != configuration) {
+    failure.Run(std::string(), std::string("Unauthorized access 2."));
+    return;
+  }
+
+  if (configuration->extension_id() != extension_id ||
+      configuration->configuration_name() != configuration_name) {
+    failure.Run(std::string(), std::string("Unauthorized access 3."));

[emaxx, 2016/06/21 23:47:47]

Could you please make the error messages more informative?

[adrian.belgun, 2016/06/22 08:23:46]

Done.

+    return;
+  }
+
+  const std::string service_path = configuration->service_path();
+  if (service_path.empty()) {
+    failure.Run(std::string(), std::string("Pending create."));
+    return;
+  }
+
+  // Connection authorized. All packets will be routed through NaCl.
+  configuration->set_nacl_bound(true, std::move(pepper_vpn_provider_proxy));
+
+  success.Run();
+}
+
+std::unique_ptr<content::VpnServiceProxy> VpnService::GetVpnServiceProxy() {
+  return base::WrapUnique(new VpnServiceProxyImpl(weak_factory_.GetWeakPtr()));
+}
+
 }  // namespace chromeos