Do not use overlays when VR shell is enabled.

media/blink/webmediaplayer_impl.h

 // 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.
 
 #ifndef MEDIA_BLINK_WEBMEDIAPLAYER_IMPL_H_
 #define MEDIA_BLINK_WEBMEDIAPLAYER_IMPL_H_
 
 #include <stdint.h>
 
 #include <memory>
@@ skipping 189 matching lines @@
 
   void SetMediaPlayerManager(
       RendererMediaPlayerManagerInterface* media_player_manager);
   void OnRemotePlaybackEnded();
   void OnDisconnectedFromRemoteDevice(double t);
   void SuspendForRemote();
   void DisplayCastFrameAfterSuspend(const scoped_refptr<VideoFrame>& new_frame,
                                     PipelineStatus status);
   gfx::Size GetCanvasSize() const;
   void SetDeviceScaleFactor(float scale_factor);
+  void SetEnableFullscreenOverlays(bool disable_overlays);

[mthiesse, 2016/12/14 16:08:19]

Does it makes sense to have a function for this? Looks like it's only ever set
once on creation, so maybe it should be part of the constructor?

[amp, 2016/12/14 18:15:04]

After the inversion and pulling out the other feature check it never changes so
it could work in the constructor.

@liberato, WDYT?

[liberato (no reviews please), 2016/12/14 19:39:07]

i find adding booleans in the constructor to be error-prone.  since we already
have plenty of setters that are used in render_frame_impl, i'd prefer to keep
this explicit too.

to be honest, i preferred when WMPI figured it out for itself.  Once we remove
this flag for VR, we'll probably want to put it back that way.

also, you've got |disable_overlays| instead of |enable_overlays| here.

[amp, 2016/12/14 22:02:09]

Done.  Going with a setter in latest patch.

   void setPoster(const blink::WebURL& poster) override;
   void SetUseFallbackPath(bool use_fallback_path);
 #endif
 
   // Called from WebMediaPlayerCast.
   // TODO(hubbe): WMPI_CAST make private.
   void OnPipelineSeeked(bool time_updated);
 
   // Restart the player/pipeline as soon as possible. This will destroy the
   // current renderer, if any, and create a new one via the RendererFactory; and
@@ skipping 308 matching lines @@
 
   // If a surface is requested before it's finished being created, the request
   // is saved and satisfied once the surface is available. If the decoder does
   // not require restart to change surfaces, this is callback is kept until
   // cleared by the decoder.
   SurfaceCreatedCB set_surface_cb_;
 
   // Force to use SurfaceView instead of SurfaceTexture on Android.
   bool force_video_overlays_;
 
-  // Prevent use of SurfaceView on Android. (Ignored when
+  // Allow use of SurfaceView on Android. (Ignored when
   // |force_video_overlays_| is true.)
-  bool disable_fullscreen_video_overlays_;
+  bool enable_fullscreen_video_overlays_;
 
   // Suppresses calls to OnPipelineError() after destruction / shutdown has been
   // started; prevents us from spuriously logging errors that are transient or
   // unimportant.
   bool suppress_destruction_errors_;
 
   // State indicating if it's okay to suspend or not. Updated on the first time
   // OnSuspendRequested() is called. If the state is UNKNOWN, the current frame
   // from the compositor will be queried to see if suspend is supported; the
   // state will be set to YES or NO respectively if a frame is available.
@@ skipping 27 matching lines @@
 
   // Whether the player is currently in autoplay muted state.
   bool autoplay_muted_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(WebMediaPlayerImpl);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BLINK_WEBMEDIAPLAYER_IMPL_H_