Position overlays in AVDACodecImage

media/gpu/avda_picture_buffer_manager.h

Index: media/gpu/avda_picture_buffer_manager.h
diff --git a/media/gpu/avda_picture_buffer_manager.h b/media/gpu/avda_picture_buffer_manager.h
index 377da6003277cc7939164466e41c9472e2c304cc..fc265c496d4601d3177f777e129169ac77dbf881 100644
--- a/media/gpu/avda_picture_buffer_manager.h
+++ b/media/gpu/avda_picture_buffer_manager.h
@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "media/gpu/avda_state_provider.h"
+#include "media/gpu/avda_surface_bundle.h"
 #include "media/gpu/media_gpu_export.h"
 #include "media/gpu/surface_texture_gl_owner.h"
 
@@ -19,11 +20,6 @@ class GLStreamTextureImage;
 }
 }
 
-namespace gl {
-class ScopedJavaSurface;
-class SurfaceTexture;
-}
-
 namespace media {
 class AVDACodecImage;
 class AVDASharedState;
@@ -43,21 +39,24 @@ class MEDIA_GPU_EXPORT AVDAPictureBufferManager {
   explicit AVDAPictureBufferManager(AVDAStateProvider* state_provider);
   virtual ~AVDAPictureBufferManager();
 
-  // Call either InitializeForOverlay or InitializeForSurfaceTexture before
-  // anything else.  InitializeForOverlay will set us up to render codec buffers
-  // at the approrpriate time for display, but will assume that consuming the
-  // resulting buffers is handled elsewhere (e.g., SurfaceFlinger).
+  // Call Initialize, providing the surface bundle that holds the surface that
+  // will back the frames.  If an overlay is present in the bundle, then this
+  // will set us up to render codec buffers at the appropriate time for display,
+  // but will assume that consuming the resulting buffers is handled elsewhere
+  // (e.g., SurfaceFlinger).  We will ensure that any reference to the bundle
+  // is dropped if the overlay sends OnSurfaceDestroyed.
   //
-  // InitializeForSurfaceTexture will create a SurfaceTexture and return the
-  // surface for it.  We will arrange to consume the buffers at the right time,
-  // in addition to releasing codec buffers for rendering.
+  // Without an overlay, we will create a SurfaceTexture and add it (and its
+  // surface) to |surface_bundle|.  We will arrange to consume the buffers at
+  // the right time, in addition to releasing the codec buffers for rendering.
   //
   // One may call these multiple times to change between overlay and ST.
   //
   // Picture buffers will be updated to reflect the new surface during the call
   // to UseCodecBufferForPicture().
-  void InitializeForOverlay();
-  gl::ScopedJavaSurface InitializeForSurfaceTexture();
+  //
+  // Returns true on success, false otherwise.

[watk, 2017/05/17 19:45:14]

nit: Reminds me of this from the styleguide :P

However, do not be unnecessarily verbose or state the completely obvious. Notice
below that it is not necessary to say "returns false otherwise" because this is
implied.

// Returns true if the table cannot hold any more entries.
bool IsTableFull();

[liberato (no reviews please), 2017/05/17 20:19:32]

done, though i'm not sure that i agree with the style guide's use of the word
"implied".

+  bool Initialize(scoped_refptr<AVDASurfaceBundle> surface_bundle);
 
   void Destroy(const PictureBufferMap& buffers);
 
@@ -90,10 +89,6 @@ class MEDIA_GPU_EXPORT AVDAPictureBufferManager {
   // Are there any unrendered picture buffers oustanding?
   bool HasUnrenderedPictures() const;
 
-  scoped_refptr<gl::SurfaceTexture> surface_texture() {
-    return surface_texture_;
-  }
-
   // Returns the GL texture target that the PictureBuffer textures use.
   // Always use OES textures even though this will cause flickering in dev tools
   // when inspecting a fullscreen video.  See http://crbug.com/592798