Notify SharedImageInterface about mailbox

It's possible that a video frame mailbox is not registered in
gpu::SharedImageInterface, but will be passed to
ClientSharedImageInterface::DestroySharedImage which results in DCHECK()
for Android MediaPlayerRenderer code path.

Bug: 1075552
Change-Id: Id24245f349e61784a407b60729f82d44033843fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2165738
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774290}

content/browser/media/media_browsertest.cc

@@ -167,6 +167,25 @@ class MediaTest : public testing::WithParamInterface<bool>,
   }
 };
 
+#if defined(OS_ANDROID)
+class AndroidPlayerMediaTest : public MediaTest {
+ private:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    MediaTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(switches::kDisableAcceleratedVideoDecode);
+  }
+};
+
+IN_PROC_BROWSER_TEST_P(AndroidPlayerMediaTest, VideoBearMp4) {
+  PlayVideo("bear.mp4", GetParam());
+}
+
+INSTANTIATE_TEST_SUITE_P(File,
+                         AndroidPlayerMediaTest,
+                         ::testing::Values(false));
+INSTANTIATE_TEST_SUITE_P(Http, AndroidPlayerMediaTest, ::testing::Values(true));
+#endif  // defined(OS_ANDROID)
+
 // Android doesn't support Theora.
 #if !defined(OS_ANDROID)
 IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearTheora) {

content/renderer/media/android/stream_texture_wrapper_impl.cc

@@ -10,6 +10,7 @@
 #include "cc/layers/video_frame_provider.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/shared_image_interface.h"
+#include "gpu/command_buffer/common/shared_image_usage.h"
 #include "media/base/bind_to_current_loop.h"
 
 namespace {
@@ -62,6 +63,11 @@ void StreamTextureWrapperImpl::CreateVideoFrame(
   gpu::MailboxHolder holders[media::VideoFrame::kMaxPlanes] = {
       gpu::MailboxHolder(mailbox, gpu::SyncToken(), GL_TEXTURE_EXTERNAL_OES)};
 
+  gpu::SharedImageInterface* sii = factory_->SharedImageInterface();
+  sii->NotifyMailboxAdded(mailbox, gpu::SHARED_IMAGE_USAGE_DISPLAY |
+                                       gpu::SHARED_IMAGE_USAGE_GLES2 |
+                                       gpu::SHARED_IMAGE_USAGE_RASTER);
+
   // The pixel format doesn't matter here as long as it's valid for texture
   // frames. But SkiaRenderer wants to ensure that the format of the resource
   // used here which will eventually create a promise image must match the

gpu/command_buffer/client/shared_image_interface.cc

@@ -10,4 +10,7 @@ uint32_t SharedImageInterface::UsageForMailbox(const Mailbox& mailbox) {
   return 0u;
 }
 
+void SharedImageInterface::NotifyMailboxAdded(const Mailbox& /*mailbox*/,
+                                              uint32_t /*usage*/) {}
+
 }  // namespace gpu

gpu/command_buffer/client/shared_image_interface.h

@@ -181,6 +181,10 @@ class GPU_EXPORT SharedImageInterface {
   // Provides the usage flags supported by the given |mailbox|. This must have
   // been created using a SharedImageInterface on the same channel.
   virtual uint32_t UsageForMailbox(const Mailbox& mailbox);
+
+  // Informs that existing |mailbox| with |usage| can be passed to
+  // DestroySharedImage().
+  virtual void NotifyMailboxAdded(const Mailbox& mailbox, uint32_t usage);
 };
 
 }  // namespace gpu

gpu/ipc/client/client_shared_image_interface.cc

@@ -124,6 +124,12 @@ uint32_t ClientSharedImageInterface::UsageForMailbox(const Mailbox& mailbox) {
   return proxy_->UsageForMailbox(mailbox);
 }
 
+void ClientSharedImageInterface::NotifyMailboxAdded(const Mailbox& mailbox,
+                                                    uint32_t usage) {
+  AddMailbox(mailbox);
+  proxy_->NotifyMailboxAdded(mailbox, usage);
+}
+
 Mailbox ClientSharedImageInterface::AddMailbox(const gpu::Mailbox& mailbox) {
   if (mailbox.IsZero())
     return mailbox;

gpu/ipc/client/client_shared_image_interface.h

@@ -62,6 +62,7 @@ class GPU_EXPORT ClientSharedImageInterface : public SharedImageInterface {
   void DestroySharedImage(const SyncToken& sync_token,
                           const Mailbox& mailbox) override;
   uint32_t UsageForMailbox(const Mailbox& mailbox) override;
+  void NotifyMailboxAdded(const Mailbox& mailbox, uint32_t usage) override;
 
  private:
   Mailbox AddMailbox(const Mailbox& mailbox);

gpu/ipc/client/shared_image_interface_proxy.cc

@@ -427,4 +427,10 @@ uint32_t SharedImageInterfaceProxy::UsageForMailbox(const Mailbox& mailbox) {
   return it->second;
 }
 
+void SharedImageInterfaceProxy::NotifyMailboxAdded(const Mailbox& mailbox,
+                                                   uint32_t usage) {
+  base::AutoLock lock(lock_);
+  AddMailbox(mailbox, usage);
+}
+
 }  // namespace gpu

gpu/ipc/client/shared_image_interface_proxy.h

@@ -59,6 +59,7 @@ class SharedImageInterfaceProxy {
   scoped_refptr<gfx::NativePixmap> GetNativePixmap(const gpu::Mailbox& mailbox);
 
   uint32_t UsageForMailbox(const Mailbox& mailbox);
+  void NotifyMailboxAdded(const Mailbox& mailbox, uint32_t usage);
 
  private:
   bool GetSHMForPixelData(base::span<const uint8_t> pixel_data,

media/base/android/media_player_bridge.cc

@@ -74,6 +74,7 @@ MediaPlayerBridge::MediaPlayerBridge(const GURL& url,
                                      bool allow_credentials,
                                      bool is_hls)
     : prepared_(false),
+      playback_completed_(false),
       pending_play_(false),
       should_seek_on_prepare_(false),
       url_(url),
@@ -122,7 +123,7 @@ void MediaPlayerBridge::Initialize() {
     return;
   }
 
-  if (allow_credentials_) {
+  if (allow_credentials_ && !url_.SchemeIsFile()) {
     media::MediaResourceGetter* resource_getter =
         client_->GetMediaResourceGetter();
 
@@ -415,7 +416,10 @@ void MediaPlayerBridge::OnMediaError(int error_type) {
 }
 
 void MediaPlayerBridge::OnPlaybackComplete() {
-  client_->OnPlaybackComplete();
+  if (!playback_completed_) {
+    playback_completed_ = true;
+    client_->OnPlaybackComplete();
+  }
 }
 
 void MediaPlayerBridge::OnMediaPrepared() {
@@ -509,6 +513,8 @@ void MediaPlayerBridge::SeekInternal(base::TimeDelta time) {
     return;
   }
 
+  playback_completed_ = false;
+
   // Note: we do not want to count changes in media time due to seeks as watch
   // time, but tracking pending seeks is not completely trivial. Instead seeks
   // larger than kWatchTimeReportingInterval * 2 will be discarded by the sanity

media/base/android/media_player_bridge.h

@@ -199,6 +199,9 @@ class MEDIA_EXPORT MediaPlayerBridge {
   // Whether the player is prepared for playback.
   bool prepared_;
 
+  // Whether the player completed playback.
+  bool playback_completed_;
+
   // Pending play event while player is preparing.
   bool pending_play_;