Use PpCompletionCallback in PepperPortAllocatorSession.

Pepper may call callbacks even after the corresponding resource is destroyed.
PepperPortAllocatorSession didn't handle this case properly. Fixed it by using
PpCompletionCallback and weak pointers.

BUG=157404


Review URL: https://chromiumcodereview.appspot.com/11233071

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@164355 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 23 insertions, 39 deletions

diff --git a/remoting/client/plugin/pepper_port_allocator.cc b/remoting/client/plugin/pepper_port_allocator.cc
index 5d7e52e..904ae0f 100644
--- a/remoting/client/plugin/pepper_port_allocator.cc
+++ b/remoting/client/plugin/pepper_port_allocator.cc
@@ -4,6 +4,7 @@
 
 #include "remoting/client/plugin/pepper_port_allocator.h"
 
+#include "base/bind.h"
 #include "base/string_number_conversions.h"
 #include "net/base/net_util.h"
 #include "ppapi/c/pp_errors.h"
@@ -48,11 +49,6 @@ class PepperPortAllocatorSession
   virtual void SendSessionRequest(const std::string& host, int port) OVERRIDE;
 
  private:
-  // Callback handlers for Pepper APIs.
-  static void HostResolverCallback(void* user_data, int32_t result);
-  static void UrlLoaderOpenCallback(void* user_data, int32_t result);
-  static void UrlLoaderReadCallback(void* user_data, int32_t result);
-
   void ResolveStunServerAddress();
   void OnStunAddressResolved(int32_t result);
 
@@ -70,6 +66,9 @@ class PepperPortAllocatorSession
   std::vector<char> relay_response_body_;
   bool relay_response_received_;
 
+  // Used to safely cancel completion callbacks from PPAPI calls.
+  base::WeakPtrFactory<PepperPortAllocatorSession> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(PepperPortAllocatorSession);
 };
 
@@ -89,7 +88,8 @@ PepperPortAllocatorSession::PepperPortAllocatorSession(
       instance_(instance),
       stun_address_resolver_(instance_),
       stun_port_(0),
-      relay_response_received_(false) {
+      relay_response_received_(false),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   if (stun_hosts.size() > 0) {
     stun_address_ = stun_hosts[0];
   }
@@ -151,8 +151,11 @@ void PepperPortAllocatorSession::ResolveStunServerAddress() {
   hint.flags = 0;
   hint.family = PP_NETADDRESSFAMILY_IPV4;
   int result = stun_address_resolver_.Resolve(
-      hostname, port, hint, pp::CompletionCallback(
-          &PepperPortAllocatorSession::HostResolverCallback, this));
+      hostname, port, hint,
+      PpCompletionCallback(base::Bind(
+          &PepperPortAllocatorSession::OnStunAddressResolved,
+          weak_factory_.GetWeakPtr())));
+
   DCHECK_EQ(result, PP_OK_COMPLETIONPENDING);
 }
 
@@ -211,8 +214,11 @@ void PepperPortAllocatorSession::SendSessionRequest(
   headers << "X-Stream-Type: " << "chromoting" << "\n\r";
   request_info.SetHeaders(headers.str());
 
-  int result = relay_url_loader_->Open(request_info, pp::CompletionCallback(
-      &PepperPortAllocatorSession::UrlLoaderOpenCallback, this));
+  int result = relay_url_loader_->Open(
+      request_info, PpCompletionCallback(base::Bind(
+          &PepperPortAllocatorSession::OnUrlOpened,
+          weak_factory_.GetWeakPtr())));
+
   DCHECK_EQ(result, PP_OK_COMPLETIONPENDING);
 }
 
@@ -245,8 +251,10 @@ void PepperPortAllocatorSession::ReadResponseBody() {
   int pos = relay_response_body_.size();
   relay_response_body_.resize(pos + kReadSize);
   int result = relay_url_loader_->ReadResponseBody(
-      &relay_response_body_[pos], kReadSize, pp::CompletionCallback(
-          &PepperPortAllocatorSession::UrlLoaderReadCallback, this));
+      &relay_response_body_[pos], kReadSize,
+      PpCompletionCallback(base::Bind(
+          &PepperPortAllocatorSession::OnResponseBodyRead,
+          weak_factory_.GetWeakPtr())));
   DCHECK_EQ(result, PP_OK_COMPLETIONPENDING);
 }
 
@@ -278,33 +286,6 @@ void PepperPortAllocatorSession::OnResponseBodyRead(int32_t result) {
   ReadResponseBody();
 }
 
-// static
-void PepperPortAllocatorSession::HostResolverCallback(
-    void* user_data,
-    int32_t result) {
-  PepperPortAllocatorSession* object =
-      static_cast<PepperPortAllocatorSession*>(user_data);
-  object->OnStunAddressResolved(result);
-}
-
-// static
-void PepperPortAllocatorSession::UrlLoaderOpenCallback(
-    void* user_data,
-    int32_t result) {
-  PepperPortAllocatorSession* object =
-      static_cast<PepperPortAllocatorSession*>(user_data);
-  object->OnUrlOpened(result);
-}
-
-// static
-void PepperPortAllocatorSession::UrlLoaderReadCallback(
-    void* user_data,
-    int32_t result) {
-  PepperPortAllocatorSession* object =
-      static_cast<PepperPortAllocatorSession*>(user_data);
-  object->OnResponseBodyRead(result);
-}
-
 }  // namespace
 
 // static
diff --git a/remoting/client/plugin/pepper_util.h b/remoting/client/plugin/pepper_util.h
index b106b2f..63c0bce 100644
--- a/remoting/client/plugin/pepper_util.h
+++ b/remoting/client/plugin/pepper_util.h
@@ -21,6 +21,9 @@ namespace remoting {
 // Adapts a base::Callback to a pp::CompletionCallback, which may be passed to
 // exactly one Pepper API. If the adapted callback is not used then a copy of
 // |callback| will be leaked, including references & passed values bound to it.
+// Pepper guarantees that each completion callback is called once and only once
+// (aborted callbacks are called with PP_ABOIRTED), so there should be no leaks
+// as long as the result of this function is passed to Pepper.
 pp::CompletionCallback PpCompletionCallback(base::Callback<void(int)> callback);
 
 // Helpers to convert between different socket address representations.