Replaces VideoLayer with a SurfaceLayer in WebMediaPlayerImpl

media/blink/webmediaplayer_impl.cc

Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index 2986a27c8a2daacba2205b99fca390f6d153e882..4161628e2dbb586ec678a28298bb06c210e9667a 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -61,6 +61,7 @@
 #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"
@@ -267,6 +268,9 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
   DCHECK(client_);
   DCHECK(delegate_);
 
+  if (base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo))

[liberato (no reviews please), 2017/07/06 16:56:17]

please cache this, similar to |overlay_mode_|, since it's called quite a lot.

[CJ, 2017/07/06 20:45:18]

Done.

+    bridge_ = base::WrapUnique(blink::WebVideoSurfaceLayerBridge::Create());
+
   force_video_overlays_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kForceVideoOverlays);
 
@@ -323,8 +327,14 @@ WebMediaPlayerImpl::~WebMediaPlayerImpl() {
 
   // Destruct compositor resources in the proper order.
   client_->SetWebLayer(nullptr);
-  if (video_weblayer_)
+  if (!base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo) &&
+      video_weblayer_) {
     static_cast<cc::VideoLayer*>(video_weblayer_->layer())->StopUsingProvider();
+  }
+  if (base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo) &&
+      bridge_->GetWebLayer()) {
+    static_cast<cc::VideoLayer*>(bridge_->GetCCLayer())->StopUsingProvider();

[liberato (no reviews please), 2017/07/06 16:56:18]

the weblayer is a SurfaceLayer, not a VideoLayer.  this functionality won't end
up being routed to the bridge at all, but rather to whatever it is that's
sending VideoFrame-backed quads to the frame sink that |bridge_| is embedding. 
we haven't really figured that out yet.

probably just delete the whole if @334, open a bug, assign it to yourself, and
add a TODO to reference it.  as we design that part, we'll handle this.

[CJ, 2017/07/06 20:45:18]

Done.

+  }
   compositor_task_runner_->DeleteSoon(FROM_HERE, compositor_);
 
   media_log_->AddEvent(
@@ -1349,12 +1359,21 @@ void WebMediaPlayerImpl::OnMetadata(PipelineMetadata metadata) {
         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)));
-    video_weblayer_->layer()->SetContentsOpaque(opaque_);
-    video_weblayer_->SetContentsOpaqueIsFixed(true);
-    client_->SetWebLayer(video_weblayer_.get());
+    if (!base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo)) {
+      DCHECK(!video_weblayer_);
+      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());
+    } else {
+      if (bridge_->GetWebLayer()) {
+        bridge_->GetCCLayer()->SetContentsOpaque(opaque_);
+        // TODO(lethalantidote): Figure out how to persist opaque setting

[liberato (no reviews please), 2017/07/06 16:56:18]

maybe open a bug for this, add it as a blocker to the tracking bug for the
cc::Surfaces conversion, and add a reference to it here.

[CJ, 2017/07/06 20:45:18]

Done.

+        // without calling WebLayerImpl's SetContentsOpaueIsFixed;
+        client_->SetWebLayer(bridge_->GetWebLayer());
+      }
+    }
   }
 
   if (observer_)
@@ -1548,8 +1567,14 @@ void WebMediaPlayerImpl::OnVideoOpacityChange(bool opaque) {
   opaque_ = opaque;
   // Modify content opaqueness of cc::Layer directly so that
   // SetContentsOpaqueIsFixed is ignored.
-  if (video_weblayer_)
-    video_weblayer_->layer()->SetContentsOpaque(opaque_);
+  if (!base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo)) {
+    if (video_weblayer_)
+      video_weblayer_->layer()->SetContentsOpaque(opaque_);
+  } else {
+    if (bridge_->GetWebLayer()) {
+      bridge_->GetCCLayer()->SetContentsOpaque(opaque_);
+    }
+  }
 }
 
 void WebMediaPlayerImpl::OnVideoAverageKeyframeDistanceUpdate() {
@@ -1723,10 +1748,17 @@ void WebMediaPlayerImpl::SuspendForRemote() {
 }
 
 gfx::Size WebMediaPlayerImpl::GetCanvasSize() const {
-  if (!video_weblayer_)
-    return pipeline_metadata_.natural_size;
+  if (!base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo) {
+    if (!video_weblayer_)

[liberato (no reviews please), 2017/07/06 16:56:17]

maybe add a weblayer() helper that would return |video_weblayer_| or
GetWebLayer(), as appropriate.  we could use it in several places.  everywhere,
i think, except the ~1 place where we actually need the WebLayerImpl.

[CJ, 2017/07/06 20:45:18]

Wont all the |video_weblayer_| code be going away anyway? Leaving it like this
will make it easy to delete.

+      return pipeline_metadata_.natural_size;
 
-  return video_weblayer_->Bounds();
+    return video_weblayer_->Bounds();
+  } else {
+    if (!bridge_->GetWebLayer())
+      return pipeline_metadate_.natural_size;

[liberato (no reviews please), 2017/07/06 16:56:18]

s/metadate/metadata

[CJ, 2017/07/06 20:45:18]

Thanks! I wonder why the compiler is not catching that...

[liberato (no reviews please), 2017/07/07 15:26:43]

the entire group of fns is #if android.

+
+    return bridge_->GetWebLayer()->Bounds();
+  }
 }
 
 void WebMediaPlayerImpl::SetDeviceScaleFactor(float scale_factor) {