Add PPAPI tests for VideoSource and VideoDestination resources.

content/renderer/pepper/pepper_video_source_host.cc

Index: content/renderer/pepper/pepper_video_source_host.cc
diff --git a/content/renderer/pepper/pepper_video_source_host.cc b/content/renderer/pepper/pepper_video_source_host.cc
index 462c8aca537a2ba9e2749600bf9bad3514b63ee2..579855c9d4aeb565f17ccd1dfe1426a40b4a6011 100644
--- a/content/renderer/pepper/pepper_video_source_host.cc
+++ b/content/renderer/pepper/pepper_video_source_host.cc
@@ -62,7 +62,7 @@ bool PepperVideoSourceHost::GotFrame(cricket::VideoFrame* frame) {
   main_message_loop_proxy_->PostTask(
       FROM_HERE,
       base::Bind(&PepperVideoSourceHost::OnGotFrame,
-                 weak_factory_.GetWeakPtr(),
+                 base::Unretained(this),

[raymes, 2013/05/10 05:32:10]

I guess this wasn't right before because we can't pass the weak pointer across
threads. Unfortunately I still think this isn't right because the
PepperVideoSourceHost can be destroyed when it tries to call into OnGotFrame. 

One option: have a thread-safe recounted helper object which implements
GotFrame. Pass in a weak pointer to this host on construction. 

[bbudge, 2013/05/10 12:31:12]

The RTC folks say this can't be called after we complete a call to
VIdeoSourceHandler::Close. We do that in our destructor. However, let's be
defensive here. Done.
On 2013/05/10 05:32:10, raymes wrote:

[bbudge, 2013/05/10 13:52:59]

While out on my morning run, around mile 2 I realized that you are absolutely
right - this must be done with a ref counted helper. Thanks for the excellent
and prompt reviews.
On 2013/05/10 12:31:12, bbudge1 wrote:

                  base::Passed(scoped_ptr<cricket::VideoFrame>(frame))));
 
   return true;
@@ -72,12 +72,8 @@ void PepperVideoSourceHost::OnGotFrame(scoped_ptr<cricket::VideoFrame> frame) {
   // Take ownership of the new frame, and possibly delete any unsent one.
   last_frame_.swap(frame);
 
-  if (get_frame_pending_) {
-    ppapi::HostResource image_data_resource;
-    PP_TimeTicks timestamp = 0;
-    int32_t result = ConvertFrame(&image_data_resource, &timestamp);
-    SendFrame(image_data_resource, timestamp, result);
-  }
+  if (get_frame_pending_)
+    SendGetFrameReply(true);  // Send reply even on failure.
 }
 
 int32_t PepperVideoSourceHost::OnHostMsgOpen(HostMessageContext* context,
@@ -107,14 +103,10 @@ int32_t PepperVideoSourceHost::OnHostMsgGetFrame(
   reply_context_ = context->MakeReplyMessageContext();
   get_frame_pending_ = true;
 
-  // If a frame is ready, try to convert it and reply.
+  // If a frame is ready, try to convert it and send the reply.
   if (last_frame_.get()) {
-    ppapi::HostResource image_data_resource;
-    PP_TimeTicks timestamp = 0;
-    int32_t result = ConvertFrame(&image_data_resource, &timestamp);
-    if (result == PP_OK) {
-      SendFrame(image_data_resource, timestamp, result);
-    } else {
+    int32_t result = SendGetFrameReply(false);  // Don't send reply on failure.

[raymes, 2013/05/10 05:32:10]

I think this would be clearer if SendGetFrameReply always sent a reply (even on
failure).

[bbudge, 2013/05/10 12:31:12]

Done.

+    if (result != PP_OK) {
       reply_context_ = ppapi::host::ReplyMessageContext();
       return result;
     }
@@ -128,16 +120,14 @@ int32_t PepperVideoSourceHost::OnHostMsgClose(HostMessageContext* context) {
   return PP_OK;
 }
 
-int32_t PepperVideoSourceHost::ConvertFrame(
-    ppapi::HostResource* image_data_resource,
-    PP_TimeTicks* timestamp) {
+int32_t PepperVideoSourceHost::SendGetFrameReply(bool force_reply) {
   DCHECK(last_frame_.get());
   scoped_ptr<cricket::VideoFrame> frame(last_frame_.release());
 
   int32_t width = base::checked_numeric_cast<int32_t>(frame->GetWidth());
   int32_t height = base::checked_numeric_cast<int32_t>(frame->GetHeight());
   // Create an image data resource to hold the frame pixels.
-  PP_ImageDataDesc desc;
+  PP_ImageDataDesc image_desc;
   IPC::PlatformFileForTransit image_handle;
   uint32_t byte_count;
   ppapi::ScopedPPResource resource(
@@ -148,27 +138,27 @@ int32_t PepperVideoSourceHost::ConvertFrame(
           PP_MakeSize(width, height),
           false /* init_to_zero */,
           false /* is_nacl_plugin */,
-          &desc, &image_handle, &byte_count));
+          &image_desc, &image_handle, &byte_count));
   if (!resource.get())
-    return PP_ERROR_FAILED;
+    return ReportGetFrameError(PP_ERROR_FAILED, force_reply);
 
   ppapi::thunk::EnterResourceNoLock<ppapi::thunk::PPB_ImageData_API>
       enter_resource(resource, false);
   if (enter_resource.failed())
-    return PP_ERROR_FAILED;
+    return ReportGetFrameError(PP_ERROR_FAILED, force_reply);
 
   webkit::ppapi::PPB_ImageData_Impl* image_data =
       static_cast<webkit::ppapi::PPB_ImageData_Impl*>(enter_resource.object());
   webkit::ppapi::ImageDataAutoMapper mapper(image_data);
   if (!mapper.is_valid())
-    return PP_ERROR_FAILED;
+    return ReportGetFrameError(PP_ERROR_FAILED, force_reply);
 
   const SkBitmap* bitmap = image_data->GetMappedBitmap();
   if (!bitmap)
-    return PP_ERROR_FAILED;
+    return ReportGetFrameError(PP_ERROR_FAILED, force_reply);
   uint8_t* bitmap_pixels = static_cast<uint8_t*>(bitmap->getPixels());
   if (!bitmap_pixels)
-    return PP_ERROR_FAILED;
+    return ReportGetFrameError(PP_ERROR_FAILED, force_reply);
 
   size_t bitmap_size = bitmap->getSize();
   frame->ConvertToRgbBuffer(cricket::FOURCC_BGRA,
@@ -176,35 +166,76 @@ int32_t PepperVideoSourceHost::ConvertFrame(
                             bitmap_size,
                             bitmap->rowBytes());
 
-  image_data_resource->SetHostResource(pp_instance(), resource.get());
+  ppapi::HostResource host_resource;
+  host_resource.SetHostResource(pp_instance(), resource.get());
 
   // Convert a video timestamp (int64, in nanoseconds) to a time delta (int64,
   // microseconds) and then to a PP_TimeTicks (a double, in seconds). All times
   // are relative to the Unix Epoch.
   base::TimeDelta time_delta = base::TimeDelta::FromMicroseconds(
       frame->GetTimeStamp() / base::Time::kNanosecondsPerMicrosecond);
-  *timestamp = time_delta.InSecondsF();
+  PP_TimeTicks timestamp = time_delta.InSecondsF();
+
+  DCHECK(get_frame_pending_);
+  reply_context_.params.set_result(PP_OK);
+
+#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_ANDROID)

[raymes, 2013/05/10 05:32:10]

Please add a TODO to unify this with the pdf resource host.

[bbudge, 2013/05/10 12:31:12]

Done.

+  ppapi::proxy::SerializedHandle serialized_handle;
+  PpapiPluginMsg_VideoSource_GetFrameReply reply_msg(host_resource,
+                                                     image_desc,
+                                                     0,
+                                                     timestamp);
+  serialized_handle.set_shmem(image_handle, byte_count);
+  reply_context_.params.AppendHandle(serialized_handle);
+#elif defined(OS_LINUX)
+  // For Linux, we pass the SysV shared memory key in the message.
+  PpapiPluginMsg_VideoSource_GetFrameReply reply_msg(host_resource,
+                                                     image_desc,
+                                                     image_handle.fd,
+                                                     timestamp);
+#else
+  // Not supported on other platforms.
+  // This is a stub reply_msg to not break the build.
+  PpapiPluginMsg_VideoSource_GetFrameReply reply_msg(host_resource,
+                                                     image_desc,
+                                                     0,
+                                                     timestamp);
+  NOTIMPLEMENTED();
+  return PP_ERROR_NOTSUPPORTED;
+#endif
+
+  host()->SendReply(reply_context_, reply_msg);
+
+  reply_context_ = ppapi::host::ReplyMessageContext();
+  get_frame_pending_ = false;
+
+  // Keep a reference once we know this method succeeds.
+  resource.Release();
+
   return PP_OK;
 }
 
-void PepperVideoSourceHost::SendFrame(
-    const ppapi::HostResource& image_data_resource,
-    PP_TimeTicks timestamp,
-    int32_t result) {
+int32_t PepperVideoSourceHost::ReportGetFrameError(int32_t error,
+                                                   bool force_reply) {
   DCHECK(get_frame_pending_);
-  reply_context_.params.set_result(result);
-  host()->SendReply(
-      reply_context_,
-      PpapiPluginMsg_VideoSource_GetFrameReply(image_data_resource, timestamp));
+  if (force_reply) {
+    reply_context_.params.set_result(error);
+    host()->SendReply(
+        reply_context_,
+        PpapiPluginMsg_VideoSource_GetFrameReply(
+            ppapi::HostResource(), PP_ImageDataDesc(), -1, 0.0));
 
+  }
   reply_context_ = ppapi::host::ReplyMessageContext();
   get_frame_pending_ = false;
+  return error;
 }
 
 void PepperVideoSourceHost::Close() {
-  if (source_handler_.get()) {
+  if (source_handler_.get() && !stream_url_.empty()) {
     source_handler_->Close(stream_url_, this);
     source_handler_.reset(NULL);
+    stream_url_.clear();
   }
 }