Introduce a client minimum picture pool size

ppapi/tests/test_video_decoder.cc

Index: ppapi/tests/test_video_decoder.cc
diff --git a/ppapi/tests/test_video_decoder.cc b/ppapi/tests/test_video_decoder.cc
index f396bda8e3fa2b70570967ece1d0b4285aa630ae..cb5ca43063435a82cd5658daa34d25e44dba6382 100644
--- a/ppapi/tests/test_video_decoder.cc
+++ b/ppapi/tests/test_video_decoder.cc
@@ -9,18 +9,19 @@
 #include "ppapi/lib/gl/gles2/gl2ext_ppapi.h"
 #include "ppapi/tests/testing_instance.h"
 
+const uint32_t kMaxRequestedPictureCount = 100;

[bbudge, 2015/08/12 17:47:09]

This could break if the constant ever changes in the resource. Why not make it
much bigger?

[lpique, 2015/08/12 22:48:54]

I wrote the tests to test the boundary case. Allocating the maximum allowed
should be possible, and allocating one more should fail. It seems like a
reasonable test to have.

What I would have like to have done was include
ppapi/proxy/video-decoder_resource.h, and use the "kMaximumPictureCount"
constant that was defined there. Unfortunately I ran into issues compiling the
code (the included headers required C++11 support when the tests are built
without the flag enabling it).

I considered defining an enum constant in the .idl, but that didn't seem like
the right thing to do. I also didn't observe any other place in the ppapi tree
where I could add a quick internal implementation constant/header file to be
shared between implementation and test.

The only other place that made sense was to add it to media/base/limits.h. But
then its a maximum that needs to be enforced in a few other places in the code,
and not just in the PPAPI host implementation. Also what would be the difference
between the new constant and the existing kMaxVideoFrames constant there?

For now I've left the value, but changed the name, and added comments both here
and in video_decoder_resource.cc that the constants should match. If someone
ignores the comment and changes one and not the other, the test will fail.

I'm open to suggestions for doing it in some other way.

[bbudge, 2015/08/12 23:39:22]

A comment to connect the two is fine. Thanks.

+
 REGISTER_TEST_CASE(VideoDecoder);
 
 bool TestVideoDecoder::Init() {
-  video_decoder_interface_ = static_cast<const PPB_VideoDecoder_0_1*>(
-      pp::Module::Get()->GetBrowserInterface(PPB_VIDEODECODER_INTERFACE_0_1));
   const int width = 16;
   const int height = 16;
   const int32_t attribs[] = {PP_GRAPHICS3DATTRIB_WIDTH, width,
                              PP_GRAPHICS3DATTRIB_HEIGHT, height,
                              PP_GRAPHICS3DATTRIB_NONE};
   graphics_3d_ = pp::Graphics3D(instance_, attribs);
-  return video_decoder_interface_ && CheckTestingInterface();
+  return (pp::Module::Get()->GetBrowserInterface(PPB_VIDEODECODER_INTERFACE) &&
+          CheckTestingInterface());
 }
 
 void TestVideoDecoder::RunTests(const std::string& filter) {
@@ -39,6 +40,7 @@ std::string TestVideoDecoder::TestCreate() {
         video_decoder.Initialize(null_graphics_3d,
                                  PP_VIDEOPROFILE_VP8_ANY,
                                  PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 0,
                                  callback.GetCallback()));
     ASSERT_EQ(PP_ERROR_BADRESOURCE, callback.result());
   }
@@ -51,6 +53,7 @@ std::string TestVideoDecoder::TestCreate() {
         video_decoder.Initialize(graphics_3d_,
                                  kInvalidProfile,
                                  PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 0,
                                  callback.GetCallback()));
     ASSERT_EQ(PP_ERROR_BADARGUMENT, callback.result());
   }
@@ -63,9 +66,37 @@ std::string TestVideoDecoder::TestCreate() {
         video_decoder.Initialize(graphics_3d_,
                                  PP_VIDEOPROFILE_VP8_ANY,
                                  PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 0,
+                                 callback.GetCallback()));
+    ASSERT_EQ(PP_OK, callback.result());
+  }
+  // Test that Initialize succeeds with a larger than normal number of requested
+  // picture buffers, if we can create a Graphics3D resources and if we allow

[bbudge, 2015/08/12 17:47:09]

nit s/resources/resource

[lpique, 2015/08/12 22:48:54]

Done.

+  // software fallback to VP8, which should always be supported.
+  if (!graphics_3d_.is_null()) {
+    pp::VideoDecoder video_decoder(instance_);
+    TestCompletionCallback callback(instance_->pp_instance(), callback_type());
+    callback.WaitForResult(
+        video_decoder.Initialize(graphics_3d_,
+                                 PP_VIDEOPROFILE_VP8_ANY,
+                                 PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 kMaxRequestedPictureCount,
                                  callback.GetCallback()));
     ASSERT_EQ(PP_OK, callback.result());
   }
+  // Test that Initialize fails if we request an unreasonable number of picture
+  // buffers.
+  if (!graphics_3d_.is_null()) {
+    pp::VideoDecoder video_decoder(instance_);
+    TestCompletionCallback callback(instance_->pp_instance(), callback_type());
+    callback.WaitForResult(
+        video_decoder.Initialize(graphics_3d_,
+                                 PP_VIDEOPROFILE_VP8_ANY,
+                                 PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 kMaxRequestedPictureCount + 1,
+                                 callback.GetCallback()));
+    ASSERT_EQ(PP_ERROR_BADARGUMENT, callback.result());
+  }
 
   PASS();
 }