Do not use overlays when VR shell is enabled.

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 232 matching lines @@
       observer_(params.media_observer()) {
   DCHECK(!adjust_allocated_memory_cb_.is_null());
   DCHECK(renderer_factory_);
   DCHECK(client_);
 
   tick_clock_.reset(new base::DefaultTickClock());
 
   force_video_overlays_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kForceVideoOverlays);
 
-  disable_fullscreen_video_overlays_ =
-      !base::FeatureList::IsEnabled(media::kOverlayFullscreenVideo);
-
   if (delegate_)
     delegate_id_ = delegate_->AddObserver(this);
 
   media_log_->AddEvent(
       media_log_->CreateEvent(MediaLogEvent::WEBMEDIAPLAYER_CREATED));
 
   if (params.initial_cdm())
     SetCdm(params.initial_cdm());
 
   // TODO(xhwang): When we use an external Renderer, many methods won't work,
@@ skipping 39 matching lines @@
   DVLOG(1) << __func__ << "(" << load_type << ", " << url << ", " << cors_mode
            << ")";
   if (!defer_load_cb_.is_null()) {
     defer_load_cb_.Run(base::Bind(
         &WebMediaPlayerImpl::DoLoad, AsWeakPtr(), load_type, url, cors_mode));
     return;
   }
   DoLoad(load_type, url, cors_mode);
 }
 
+void WebMediaPlayerImpl::SetEnableFullscreenOverlays(bool enable_overlays) {
+  enable_fullscreen_video_overlays_ = enable_overlays;
+}
+
 bool WebMediaPlayerImpl::supportsOverlayFullscreenVideo() {
 #if defined(OS_ANDROID)
   return true;
 #else
   return false;
 #endif
 }
 
 void WebMediaPlayerImpl::EnableOverlay() {
   overlay_enabled_ = true;
@@ skipping 13 matching lines @@
   surface_created_cb_.Cancel();
   overlay_surface_id_ = SurfaceManager::kNoSurfaceID;
 
   if (decoder_requires_restart_for_overlay_)
     ScheduleRestart();
   else if (!set_surface_cb_.is_null())
     set_surface_cb_.Run(overlay_surface_id_);
 }
 
 void WebMediaPlayerImpl::enteredFullscreen() {
-  if (!force_video_overlays_ && !disable_fullscreen_video_overlays_)
+  if (!force_video_overlays_ && enable_fullscreen_video_overlays_)

[ddorwin, 2016/12/16 19:15:18]

Separate from this CL:

This logic is not obvious. Specifically, why we would not call this when
force_video_overlays_ is true. I believe it is because we have already called
EnableOverlay() in CreateRenderer(). But this is inferring the current state
from a cause.

It would seem to make more sense to check the state here, and there is already a
member tracking this.

A)
Thus, I wonder if this shouldn't be:
  if (!overlay_enabled_ && ...

B)
However, this would be better if EnableOverlay() handled this internally rather
than requiring all call sites to handle it. Thus, we could remove the first half
of the condition and add if(overlay_enabled_) return; to EnableOverlay().

[amp, 2016/12/16 19:25:03]

Acknowledged.

     EnableOverlay();
   if (observer_)
     observer_->OnEnteredFullscreen();
 }
 
 void WebMediaPlayerImpl::exitedFullscreen() {
-  if (!force_video_overlays_ && !disable_fullscreen_video_overlays_)
+  if (!force_video_overlays_ && enable_fullscreen_video_overlays_)

[ddorwin, 2016/12/16 19:15:18]

Similarly, enable_fullscreen_video_overlays_ is the cause rather than the state.

A)
And this would be if (!force_video_overlays_ && overlay_enabled_).

B)
If we went with this option above, this could probably just be a call to
DisableOverlay() in all cases. That method would return early if
~overlay_enabled_ || force_video_overlays_.

[amp, 2016/12/16 19:25:03]

Acknowledged.

     DisableOverlay();
   if (observer_)
     observer_->OnExitedFullscreen();
 }
 
 void WebMediaPlayerImpl::becameDominantVisibleContent(bool isDominant) {
   if (observer_)
     observer_->OnBecameDominantVisibleContent(isDominant);
 }
 
@@ skipping 1614 matching lines @@
     watch_time_reporter_->OnShown();
 }
 
 bool WebMediaPlayerImpl::IsHidden() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   return delegate_ && delegate_->IsHidden();
 }
 
 }  // namespace media