ppapi: PPB_VpnProvider: Implement Resource Host

content/browser/renderer_host/pepper/pepper_vpn_provider_message_filter_chromeos.cc

Index: content/browser/renderer_host/pepper/pepper_vpn_provider_message_filter_chromeos.cc
diff --git a/content/browser/renderer_host/pepper/pepper_vpn_provider_message_filter_chromeos.cc b/content/browser/renderer_host/pepper/pepper_vpn_provider_message_filter_chromeos.cc
new file mode 100644
index 0000000000000000000000000000000000000000..95d6497db6506dfa691a77d0e7584ec4066d727e
--- /dev/null
+++ b/content/browser/renderer_host/pepper/pepper_vpn_provider_message_filter_chromeos.cc
@@ -0,0 +1,333 @@
+// Copyright 2016 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 "content/browser/renderer_host/pepper/pepper_vpn_provider_message_filter_chromeos.h"
+
+#include "base/memory/ptr_util.h"
+#include "content/public/browser/browser_ppapi_host.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/pepper_vpn_provider_resource_host_proxy.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/site_instance.h"
+#include "content/public/common/content_client.h"
+#include "ppapi/host/dispatch_host_message.h"
+#include "ppapi/host/host_message_context.h"
+#include "ppapi/proxy/ppapi_messages.h"
+
+namespace {
+
+// Shared memory buffer configuration.
+const size_t kMaxQueuedPackets = 128;
+const size_t kMaxPacketSizeInBytes = 2048;                             // 2 KB
+const size_t kBufferSize = kMaxQueuedPackets * kMaxPacketSizeInBytes;  // 256 KB
+
+class PepperVpnProviderResourceHostProxyImpl
+    : public content::PepperVpnProviderResourceHostProxy {
+ public:
+  PepperVpnProviderResourceHostProxyImpl(
+      base::WeakPtr<content::PepperVpnProviderMessageFilter>
+          vpn_message_filter);
+
+  void SendOnPacketReceived(const std::vector<char>& packet) override;
+  void SendOnUnbind() override;
+
+ private:
+  base::WeakPtr<content::PepperVpnProviderMessageFilter> vpn_message_filter_;
+
+  DISALLOW_COPY_AND_ASSIGN(PepperVpnProviderResourceHostProxyImpl);
+};
+
+PepperVpnProviderResourceHostProxyImpl::PepperVpnProviderResourceHostProxyImpl(
+    base::WeakPtr<content::PepperVpnProviderMessageFilter> vpn_message_filter)
+    : vpn_message_filter_(vpn_message_filter) {}
+
+void PepperVpnProviderResourceHostProxyImpl::SendOnPacketReceived(
+    const std::vector<char>& packet) {
+  if (!vpn_message_filter_) {
+    NOTREACHED();

[emaxx, 2016/06/29 16:36:39]

Having both NOTREACHED and the handling code ("return" in the next line) is
unrecommended practice according to our style guide:
https://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREAC...

[adrian.belgun, 2016/06/30 15:03:18]

Done. Removed NOTREACHED().

+    return;
+  }
+  vpn_message_filter_->SendOnPacketReceived(packet);
+}
+
+void PepperVpnProviderResourceHostProxyImpl::SendOnUnbind() {
+  if (!vpn_message_filter_) {
+    NOTREACHED();
+    return;
+  }
+  vpn_message_filter_->SendOnUnbind();
+}
+
+}  // namespace
+
+namespace content {
+
+PepperVpnProviderMessageFilter::PepperVpnProviderMessageFilter(
+    BrowserPpapiHostImpl* host,
+    PP_Instance instance)
+    : browser_context_(nullptr), bound_(false), weak_factory_(this) {
+  DCHECK(host);
+
+  int render_process_id;
+  int render_frame_id;
+  if (!host->GetRenderFrameIDsForInstance(instance, &render_process_id,
+                                          &render_frame_id)) {
+    NOTREACHED();
+    return;
+  }
+
+  document_url_ = host->GetDocumentURLForInstance(instance);
+
+  RenderProcessHost* render_process_host =
+      RenderProcessHost::FromID(render_process_id);
+  if (render_process_host)
+    browser_context_ = render_process_host->GetBrowserContext();
+
+  if (browser_context_) {
+    vpn_service_proxy_ =
+        content::GetContentClient()->browser()->GetVpnServiceProxy(
+            browser_context_);
+  }
+}
+
+PepperVpnProviderMessageFilter::~PepperVpnProviderMessageFilter() {
+  if (bound_ && resource_host()) {
+    resource_host()->host()->SendUnsolicitedReply(
+        resource_host()->pp_resource(), PpapiPluginMsg_VpnProvider_OnUnbind());
+  }
+}
+
+scoped_refptr<base::TaskRunner>
+PepperVpnProviderMessageFilter::OverrideTaskRunnerForMessage(
+    const IPC::Message& message) {
+  switch (message.type()) {
+    case PpapiHostMsg_VpnProvider_Bind::ID:
+    case PpapiHostMsg_VpnProvider_SendPacket::ID:
+    case PpapiHostMsg_VpnProvider_OnPacketReceivedReply::ID:
+      return BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI);
+  }
+  return NULL;
+}
+
+int32_t PepperVpnProviderMessageFilter::OnResourceMessageReceived(
+    const IPC::Message& msg,
+    ppapi::host::HostMessageContext* context) {
+  PPAPI_BEGIN_MESSAGE_MAP(PepperVpnProviderMessageFilter, msg)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_VpnProvider_Bind, OnBind)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_VpnProvider_SendPacket,
+                                      OnSendPacket)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(
+        PpapiHostMsg_VpnProvider_OnPacketReceivedReply, OnPacketReceivedReply)
+  PPAPI_END_MESSAGE_MAP()
+  return PP_ERROR_FAILED;
+}
+
+void PepperVpnProviderMessageFilter::SendOnPacketReceived(
+    const std::vector<char>& packet) {
+  if (packet.size() > kMaxPacketSizeInBytes)
+    return;
+
+  uint32_t id;
+  if (recv_packet_buffer_.get() && recv_packet_buffer_->GetAvailable(&id)) {

[emaxx, 2016/06/29 16:36:39]

nit: Calling the get method is unnecessary to perform the null/non-null check.
unique_ptr already has operator bool. (This occurs in this file several times.)

[adrian.belgun, 2016/06/30 15:03:18]

Done.

+    recv_packet_buffer_->SetAvailable(id, false);
+    DoPacketReceived(packet, id);
+  } else {
+    received_packets_.push(packet);
+  }
+}
+
+void PepperVpnProviderMessageFilter::SendOnUnbind() {
+  configuration_name_.clear();
+  configuration_id_.clear();
+  bound_ = false;
+  if (resource_host()) {
+    resource_host()->host()->SendUnsolicitedReply(
+        resource_host()->pp_resource(), PpapiPluginMsg_VpnProvider_OnUnbind());
+  }
+}

[emaxx, 2016/06/29 16:36:39]

Shouldn't send_packet_buffer_ and recv_packet_buffer_ be cleared too?

[adrian.belgun, 2016/06/30 15:03:18]

Done.

+
+int32_t PepperVpnProviderMessageFilter::OnBind(
+    ppapi::host::HostMessageContext* context,
+    const std::string& configuration_id,
+    const std::string& configuration_name) {
+  if (!vpn_service_proxy_)
+    return PP_ERROR_FAILED;
+  if (!content::GetContentClient()->browser()->IsPepperVpnProviderAPIAllowed(
+          browser_context_, document_url_)) {
+    LOG(ERROR) << "Host " << document_url_.host()
+               << " cannot use vpnProvider API";
+    return PP_ERROR_NOACCESS;
+  }
+
+  configuration_id_ = configuration_id;

[emaxx, 2016/06/29 16:36:39]

Is it possible for this OnBind method to be called twice, without unbinding in
between?
In that case, for example, this configuration_id_ member would be just
overwritten - is it the expected behavior?

[emaxx, 2016/06/29 16:36:39]

Is it possible for this OnBind method to be called twice, without unbinding in
between?
In that case, for example, this configuration_id_ member would be just
overwritten - is it the expected behavior?

[adrian.belgun, 2016/06/30 15:03:18]

Yes. Only the active configuration can be bound.

+  configuration_name_ = configuration_name;
+
+  return DoBind(base::Bind(&PepperVpnProviderMessageFilter::OnBindSuccess,
+                           weak_factory_.GetWeakPtr(),
+                           context->MakeReplyMessageContext()),
+                base::Bind(&PepperVpnProviderMessageFilter::OnBindFailure,
+                           weak_factory_.GetWeakPtr(),
+                           context->MakeReplyMessageContext()));
+}
+
+int32_t PepperVpnProviderMessageFilter::OnSendPacket(
+    ppapi::host::HostMessageContext* context,
+    uint32_t packet_size,
+    uint32_t id) {
+  if (!vpn_service_proxy_)
+    return PP_ERROR_FAILED;
+  if (packet_size > kMaxPacketSizeInBytes)
+    return PP_ERROR_MESSAGE_TOO_BIG;
+
+  char* packet_pointer = static_cast<char*>(send_packet_buffer_->GetBuffer(id));
+  std::vector<char> packet(packet_pointer, packet_pointer + packet_size);
+
+  return DoSendPacket(
+      packet, base::Bind(&PepperVpnProviderMessageFilter::OnSendPacketSuccess,
+                         weak_factory_.GetWeakPtr(),
+                         context->MakeReplyMessageContext(), id),
+      base::Bind(&PepperVpnProviderMessageFilter::OnSendPacketFailure,
+                 weak_factory_.GetWeakPtr(), context->MakeReplyMessageContext(),
+                 id));
+}
+
+int32_t PepperVpnProviderMessageFilter::OnPacketReceivedReply(
+    ppapi::host::HostMessageContext* context,
+    uint32_t id) {
+  if (!received_packets_.empty()) {
+    DoPacketReceived(received_packets_.front(), id);
+    received_packets_.pop();
+  } else {
+    recv_packet_buffer_->SetAvailable(id, true);
+  }
+
+  return PP_OK;
+}
+
+int32_t PepperVpnProviderMessageFilter::DoBind(
+    SuccessCallback success_callback,
+    FailureCallback failure_callback) {
+  // Initialize shared memory
+  if (!send_packet_buffer_.get()) {
+    std::unique_ptr<base::SharedMemory> send_buffer(new base::SharedMemory);
+    if (!send_buffer.get() || !send_buffer->CreateAndMapAnonymous(kBufferSize))

[emaxx, 2016/06/29 16:36:39]

Is there any evidence that operator new may return NULL? I've never seen such
checks in the Chrome code base.

[adrian.belgun, 2016/06/30 15:03:18]

Done. Cleaned-up the code. Now only checking CreateAndMapAnonymous().

+      return PP_ERROR_NOMEMORY;
+
+    send_packet_buffer_ = base::WrapUnique(new ppapi::VpnProviderSharedBuffer(
+        kMaxQueuedPackets, kMaxPacketSizeInBytes, std::move(send_buffer)));
+    if (!send_packet_buffer_.get())
+      return PP_ERROR_NOMEMORY;
+  }
+
+  if (!recv_packet_buffer_.get()) {
+    std::unique_ptr<base::SharedMemory> recv_buffer(new base::SharedMemory);
+    if (!recv_buffer.get() ||
+        !recv_buffer->CreateAndMapAnonymous(kBufferSize)) {
+      send_packet_buffer_.reset();
+      return PP_ERROR_NOMEMORY;
+    }
+
+    recv_packet_buffer_ = base::WrapUnique(new ppapi::VpnProviderSharedBuffer(
+        kMaxQueuedPackets, kMaxPacketSizeInBytes, std::move(recv_buffer)));
+    if (!recv_packet_buffer_.get()) {
+      send_packet_buffer_.reset();
+      return PP_ERROR_NOMEMORY;
+    }
+  }
+
+  vpn_service_proxy_->Bind(
+      document_url_.host(), configuration_id_, configuration_name_,
+      success_callback, failure_callback,
+      base::WrapUnique(new PepperVpnProviderResourceHostProxyImpl(
+          weak_factory_.GetWeakPtr())));
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void PepperVpnProviderMessageFilter::OnBindSuccess(
+    const ppapi::host::ReplyMessageContext& context) {
+  bound_ = true;
+
+  context.params.AppendHandle(ppapi::proxy::SerializedHandle(
+      send_packet_buffer_->GetHandle(), kBufferSize));
+  context.params.AppendHandle(ppapi::proxy::SerializedHandle(
+      recv_packet_buffer_->GetHandle(), kBufferSize));
+
+  OnBindReply(context, PP_OK);
+}
+
+void PepperVpnProviderMessageFilter::OnBindFailure(
+    const ppapi::host::ReplyMessageContext& context,
+    const std::string& error_name,
+    const std::string& error_message) {
+  LOG(ERROR) << "PepperVpnProviderMessageFilter::OnBindFailure(): "
+             << "error_name: "
+             << "\"" << error_name << "\", "
+             << "error_message: "
+             << "\"" << error_message << "\"";
+
+  // Clear buffers on error.
+  send_packet_buffer_.reset();
+  recv_packet_buffer_.reset();
+
+  OnBindReply(context, PP_ERROR_FAILED);
+}
+
+void PepperVpnProviderMessageFilter::OnBindReply(
+    const ppapi::host::ReplyMessageContext& context,
+    int32_t reply) {
+  SendReply(context, PpapiPluginMsg_VpnProvider_BindReply(
+                         kMaxQueuedPackets, kMaxPacketSizeInBytes, reply));
+}
+
+int32_t PepperVpnProviderMessageFilter::DoSendPacket(
+    const std::vector<char>& packet,
+    SuccessCallback success_callback,
+    FailureCallback failure_callback) {
+  vpn_service_proxy_->SendPacket(document_url_.host(), packet, success_callback,
+                                 failure_callback);
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void PepperVpnProviderMessageFilter::OnSendPacketSuccess(
+    const ppapi::host::ReplyMessageContext& context,
+    uint32_t id) {
+  OnSendPacketReply(context, id);
+}
+
+void PepperVpnProviderMessageFilter::OnSendPacketFailure(
+    const ppapi::host::ReplyMessageContext& context,
+    uint32_t id,
+    const std::string& error_name,
+    const std::string& error_message) {
+  LOG(ERROR) << "PepperVpnProviderMessageFilter::OnSendPacketFailure(): "
+             << "error_name: "
+             << "\"" << error_name << "\", "
+             << "error_message: "
+             << "\"" << error_message << "\"";
+  OnSendPacketReply(context, id);
+}
+
+void PepperVpnProviderMessageFilter::OnSendPacketReply(
+    const ppapi::host::ReplyMessageContext& context,
+    int32_t reply) {
+  SendReply(context, PpapiPluginMsg_VpnProvider_SendPacketReply(reply));
+}
+
+void PepperVpnProviderMessageFilter::DoPacketReceived(
+    const std::vector<char>& packet,
+    uint32_t id) {
+  const void* packet_pointer = &packet.front();

[emaxx, 2016/06/29 16:36:39]

Can the packet be an empty vector?
Then it would be incorrect to take the address of the first element here.

[adrian.belgun, 2016/06/30 15:03:18]

Made sure that this is not called with empty vectors. Added a DCHECK here to
further enforce this.

+  uint32_t packet_size = base::checked_cast<uint32_t>(packet.size());
+  memcpy(recv_packet_buffer_->GetBuffer(id), packet_pointer, packet_size);
+
+  if (resource_host()) {
+    resource_host()->host()->SendUnsolicitedReply(
+        resource_host()->pp_resource(),
+        PpapiPluginMsg_VpnProvider_OnPacketReceived(packet_size, id));
+  }
+}
+
+}  // namespace content