Add |texture_target_| and |pixel_format_| to media::PictureBuffer

media/gpu/ipc/client/gpu_video_decode_accelerator_host.cc

Index: media/gpu/ipc/client/gpu_video_decode_accelerator_host.cc
diff --git a/media/gpu/ipc/client/gpu_video_decode_accelerator_host.cc b/media/gpu/ipc/client/gpu_video_decode_accelerator_host.cc
index 6820ea1db7964ad7094d665da4313859733e3d7a..de43cf92938ec6aa83a9bb882ece615520e67b61 100644
--- a/media/gpu/ipc/client/gpu_video_decode_accelerator_host.cc
+++ b/media/gpu/ipc/client/gpu_video_decode_accelerator_host.cc
@@ -51,7 +51,7 @@ bool GpuVideoDecodeAcceleratorHost::OnMessageReceived(const IPC::Message& msg) {
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_BitstreamBufferProcessed,
                         OnBitstreamBufferProcessed)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_ProvidePictureBuffers,
-                        OnProvidePictureBuffer)
+                        OnProvidePictureBuffers)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_PictureReady,
                         OnPictureReady)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_FlushDone, OnFlushDone)
@@ -130,7 +130,18 @@ void GpuVideoDecodeAcceleratorHost::AssignPictureBuffers(
   // Rearrange data for IPC command.
   std::vector<int32_t> buffer_ids;
   std::vector<PictureBuffer::TextureIds> texture_ids;
-  for (uint32_t i = 0; i < buffers.size(); i++) {
+  uint32_t texture_target = 0;
+  VideoPixelFormat format = PIXEL_FORMAT_UNKNOWN;
+  gfx::Size size;
+
+  uint32_t buffers_size = buffers.size();
+  if (buffers_size) {
+    texture_target = buffers[0].texture_target();

[dcheng, 2017/05/18 14:15:41]

Is it possible (via something malicious) that the buffers don't all have
matching attributes? In particular, size. This code can send a message from the
renderer to the GPU right? Does the GPU service check all these parameters for
correctness?

[Chandan, 2017/05/18 19:38:53]

Can you pls elaborate a bit on 'matching attributes'?
I assume you are referring to GPU process by 'GPU'. Then, yes.
These extra parameters are passed on to the underlying decoder. 
However, these are unused currently. Their correctness will be checked may be
when we actually switch picture buffer formats mid-stream.
liberato@, your views on this?

[liberato (no reviews please), 2017/05/18 20:07:01]

hrm, it's a good point that i hadn't considerd.

i suppose that one could remove it from IPC and fill it in on the GPU side,
based on the most recently requested parameters.  that's what size was doing
before, i think.

if one does that, then we avoid the security issue but still get buffers with
correct metadata on the gpu side.

[Chandan, 2017/05/18 20:36:36]

Ok. I will remove these 3 extra params from IPC then and cache format, target
similar to what was done for size?

+    format = buffers[0].pixel_format();
+    size = buffers[0].size();
+  }
+
+  for (uint32_t i = 0; i < buffers_size; i++) {
     const PictureBuffer& buffer = buffers[i];
     if (buffer.size() != picture_buffer_dimensions_) {
       DLOG(ERROR) << "buffer.size() invalid: expected "
@@ -142,8 +153,10 @@ void GpuVideoDecodeAcceleratorHost::AssignPictureBuffers(
     texture_ids.push_back(buffer.client_texture_ids());
     buffer_ids.push_back(buffer.id());
   }
+
   Send(new AcceleratedVideoDecoderMsg_AssignPictureBuffers(
-      decoder_route_id_, buffer_ids, texture_ids));
+      decoder_route_id_, buffer_ids, texture_ids, texture_target, format,
+      size));
 }
 
 void GpuVideoDecodeAcceleratorHost::ReusePictureBuffer(
@@ -225,7 +238,7 @@ void GpuVideoDecodeAcceleratorHost::OnBitstreamBufferProcessed(
     client_->NotifyEndOfBitstreamBuffer(bitstream_buffer_id);
 }
 
-void GpuVideoDecodeAcceleratorHost::OnProvidePictureBuffer(
+void GpuVideoDecodeAcceleratorHost::OnProvidePictureBuffers(
     uint32_t num_requested_buffers,
     VideoPixelFormat format,
     uint32_t textures_per_buffer,