Replaces VideoLayer with a SurfaceLayer in WebMediaPlayerImpl

media/blink/webmediaplayer_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 43 matching lines @@
 #include "third_party/WebKit/public/platform/WebMediaPlayerClient.h"
 #include "third_party/WebKit/public/platform/WebMediaPlayerEncryptedMediaClient.h"
 #include "third_party/WebKit/public/platform/WebMediaPlayerSource.h"
 #include "third_party/WebKit/public/platform/WebMediaSource.h"
 #include "third_party/WebKit/public/platform/WebRect.h"
 #include "third_party/WebKit/public/platform/WebRuntimeFeatures.h"
 #include "third_party/WebKit/public/platform/WebSecurityOrigin.h"
 #include "third_party/WebKit/public/platform/WebSize.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
+#include "third_party/WebKit/public/platform/WebVideoSurfaceLayerBridge.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
 #include "third_party/WebKit/public/web/WebView.h"
 
 #if defined(OS_ANDROID)
 #include "media/base/android/media_codec_util.h"
 #endif
 
@@ skipping 170 matching lines @@
       volume_(1.0),
       volume_multiplier_(1.0),
       renderer_factory_selector_(std::move(renderer_factory_selector)),
       surface_manager_(params->surface_manager()),
       overlay_surface_id_(SurfaceManager::kNoSurfaceID),
       suppress_destruction_errors_(false),
       suspend_enabled_(params->allow_suspend()),
       is_encrypted_(false),
       preroll_attempt_pending_(false),
       observer_(params->media_observer()),
+      bridge_(
+          base::WrapUnique(blink::WebVideoSurfaceLayerBridge::Create(this))),

[liberato (no reviews please), 2017/06/26 22:39:30]

maybe only do this if |kVideoSurface|?

[CJ, 2017/06/29 19:51:33]

Done.

       max_keyframe_distance_to_disable_background_video_(
           params->max_keyframe_distance_to_disable_background_video()),
       max_keyframe_distance_to_disable_background_video_mse_(
           params->max_keyframe_distance_to_disable_background_video_mse()),
       enable_instant_source_buffer_gc_(
           params->enable_instant_source_buffer_gc()),
       embedded_media_experience_enabled_(
           params->embedded_media_experience_enabled()),
       request_routing_token_cb_(params->request_routing_token_cb()),
       overlay_routing_token_(OverlayInfo::RoutingToken()) {
@@ skipping 52 matching lines @@
   watch_time_reporter_.reset();
 
   // The underlying Pipeline must be stopped before it is destroyed.
   pipeline_controller_.Stop();
 
   if (last_reported_memory_usage_)
     adjust_allocated_memory_cb_.Run(-last_reported_memory_usage_);
 
   // Destruct compositor resources in the proper order.
   client_->SetWebLayer(nullptr);
-  if (video_weblayer_)
+  if (video_weblayer_ && !base::FeatureList::IsEnabled(kVideoSurface))
     static_cast<cc::VideoLayer*>(video_weblayer_->layer())->StopUsingProvider();
   compositor_task_runner_->DeleteSoon(FROM_HERE, compositor_);
 
   media_log_->AddEvent(
       media_log_->CreateEvent(MediaLogEvent::WEBMEDIAPLAYER_DESTROYED));
 }
 
 void WebMediaPlayerImpl::Load(LoadType load_type,
                               const blink::WebMediaPlayerSource& source,
                               CORSMode cors_mode) {
@@ skipping 1005 matching lines @@
       // SurfaceView doesn't support rotated video, so transition back if
       // the video is now rotated.  If |force_video_overlays_|, we keep the
       // overlay anyway so that the state machine keeps working.
       if (!force_video_overlays_ && !DoesOverlaySupportMetadata())
         DisableOverlay();
       else if (surface_manager_)
         surface_manager_->NaturalSizeChanged(pipeline_metadata_.natural_size);
     }
 
     DCHECK(!video_weblayer_);
-    video_weblayer_.reset(new cc_blink::WebLayerImpl(cc::VideoLayer::Create(
-        compositor_, pipeline_metadata_.video_rotation)));
+    if (base::FeatureList::IsEnabled(media::kVideoSurface)) {
+      video_weblayer_.reset(new cc_blink::WebLayerImpl(bridge_->GetLayer()));

[liberato (no reviews please), 2017/06/26 15:04:03]

for your SurfaceLayer ownership question, i'd expect that we'd want a similar
ownership model here as we have now: WMPI would create and own it.

wmpi probably shouldn't need to call back into blink to create the layer, then
immediately wrap it back up and hand it back to blink in a WebLayerImpl.  i'm
not sure that one needs |bridge_| at all.

[enne (OOO), 2017/06/26 18:35:58]

Yes, I think the same ownership model should apply for the surface layers that
applies for the video layers now.
It seems like this code would want to get used by both WebMediaPlayerImpl and
WebMediaPlayerMS, which both create cc::VideoLayer and interact with
WebMediaPlayerClient.  I could imagine a helper class in content/renderer/media
that does this bridge code that both players own.  Maybe it could own or have a
pointer to the surface layer and update the surface id when that changed.  I'm
not really an expert in this code though, so take that thought with a grain of
salt.

[CJ, 2017/06/29 19:51:33]

So, to summarize, we want to pull out all SurfaceLayer creation/modification out
of bridge, and have that owned by a helper class that updates itself via bridge
callbacks.

We don't have to anything to the WebLayer when we update the Surfacelayer?

[enne (OOO), 2017/06/29 20:29:29]

I was suggesting that you don't need a bridge at all, or the bridge and the
helper become the same thing.  Said differently, WebMediaPlayerImpl/MS own
VideoSurfaceHelper.  VideoSurfaceHelper creates and owns the cc::SurfaceLayer
and handles the surface callbacks that the bridge does.  WebMediaPlayer can
access the cc::SurfaceLayer off of the helper and call SetWebLayer when
applicable.

Just trying to clarify what I was suggesting.  I was trying to elaborate a
thought off of liberato's suggestion of not having the bridge live in Blink.
If the WebLayer hasn't changed to a different cc::Layer, then there shouldn't be
anything to do.  I suspect CanvasSurfaceLayerBridge calls OnWebLayerReplaced in
too many cases.  It should only need it when the web_layer_ changes and not when
values change on the cc::SurfaceLayer.

[CJ, 2017/06/29 20:46:06]

So, is this just a naming issue? (Sorry if I keep missing the point.) I feel
like what this helper would be doing is what the bridge is already doing. I am
confused.

Okay cool, that means we would not need the WebSurfaceLayerBridgeObserver class?

+    } else {
+      video_weblayer_.reset(new cc_blink::WebLayerImpl(cc::VideoLayer::Create(
+          compositor_, pipeline_metadata_.video_rotation)));
+    }
     video_weblayer_->layer()->SetContentsOpaque(opaque_);
     video_weblayer_->SetContentsOpaqueIsFixed(true);
     client_->SetWebLayer(video_weblayer_.get());
   }
 
   if (observer_)
     observer_->OnMetadataChanged(pipeline_metadata_);
 
   CreateWatchTimeReporter();
   UpdatePlayState();
@@ skipping 1012 matching lines @@
 bool WebMediaPlayerImpl::DoesOverlaySupportMetadata() const {
   return pipeline_metadata_.video_rotation == VIDEO_ROTATION_0;
 }
 
 void WebMediaPlayerImpl::ActivateViewportIntersectionMonitoring(bool activate) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   client_->ActivateViewportIntersectionMonitoring(activate);
 }
 
+void WebMediaPlayerImpl::OnWebLayerReplaced() {}
+
 bool WebMediaPlayerImpl::ShouldPauseVideoWhenHidden() const {
   // If suspending background video, pause any video that's not remoted or
   // not unlocked to play in the background.
   if (IsBackgroundedSuspendEnabled()) {
     if (!HasVideo())
       return false;
 
 #if defined(OS_ANDROID)
     if (IsRemote())
       return false;
@@ skipping 194 matching lines @@
 }
 
 #undef UMA_HISTOGRAM_VIDEO_HEIGHT
 
 void WebMediaPlayerImpl::SetTickClockForTest(base::TickClock* tick_clock) {
   tick_clock_.reset(tick_clock);
   buffered_data_source_host_.SetTickClockForTest(tick_clock);
 }
 
 }  // namespace media