Send enter / exit fullscreen signal to AVDA

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 4c2650ad321497537dc85ebba77687bfcfe8bdb4..c6da81bc70a4d4d2910e1ce85349e420ba1e46f4 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -251,7 +251,8 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       embedded_media_experience_enabled_(
           params->embedded_media_experience_enabled()),
       request_routing_token_cb_(params->request_routing_token_cb()),
-      overlay_routing_token_(base::UnguessableToken()) {
+      overlay_routing_token_(
+          OverlayInfo::RoutingToken(base::UnguessableToken())) {

[watk, 2017/05/25 18:09:36]

Shouldn't this be a nullopt RoutingToken? I don't understand what it means for:
overlay_routing_token_ && *overlay_routing_token_ &&
*overlay_routing_token_->is_empty().

[liberato (no reviews please), 2017/05/25 21:26:56]

it should be ort_(RoutingToken()), which means "we are not allowed to use
overlays".

compare to left unintialized, which would mean "we have asked the callback for a
routing token, but it hasn't gotten back to us yet".

   DVLOG(1) << __func__;
   DCHECK(!adjust_allocated_memory_cb_.is_null());
   DCHECK(renderer_factory_selector_);
@@ -378,19 +379,22 @@ void WebMediaPlayerImpl::DisableOverlay() {
     overlay_surface_id_ = SurfaceManager::kNoSurfaceID;
   } else if (overlay_mode_ == OverlayMode::kUseAndroidOverlay) {
     token_available_cb_.Cancel();
-    overlay_routing_token_ = base::UnguessableToken();
+    overlay_routing_token_ =
+        OverlayInfo::RoutingToken(base::UnguessableToken());
   }
 
   if (decoder_requires_restart_for_overlay_)
     ScheduleRestart();
   else
-    MaybeSendOverlayInfoToDecoder();
+    MaybeSendOverlayInfoToDecoder(true);
 }
 
 void WebMediaPlayerImpl::EnteredFullscreen() {
   // |force_video_overlays_| implies that we're already in overlay mode, so take
   // no action here.  Otherwise, switch to an overlay if it's allowed and if
   // it will display properly.
+  is_fullscreen_ = true;
+
   if (!force_video_overlays_ && overlay_mode_ != OverlayMode::kNoOverlays &&
       DoesOverlaySupportMetadata()) {
     EnableOverlay();
@@ -398,20 +402,30 @@ void WebMediaPlayerImpl::EnteredFullscreen() {
   if (observer_)
     observer_->OnEnteredFullscreen();
 
-  // TODO(liberato): if the decoder provided a callback for fullscreen state,
-  // then notify it now.
+  // We send this only if we can send multiple calls.  Otherwise, either (a)
+  // we already sent it and we don't have a callback anyway (we reset it when
+  // it's called in restart mode), or (b) we'll send this later when the surface
+  // actually arrives.  GVD assumes that the first overlay info will have the
+  // routing information.  Note that we set |is_fullscreen_| earlier, so that
+  // if EnableOverlay() can include fullscreen info in case it sends the overlay
+  // info before returning.
+  if (!decoder_requires_restart_for_overlay_)
+    MaybeSendOverlayInfoToDecoder(false);
 }
 
 void WebMediaPlayerImpl::ExitedFullscreen() {
   // If we're in overlay mode, then exit it unless we're supposed to be in
   // overlay mode all the time.

[watk, 2017/05/25 18:09:36]

Can you move this comment back to the if statement

[liberato (no reviews please), 2017/05/25 21:26:56]

Done.

+  is_fullscreen_ = false;
+
   if (!force_video_overlays_ && overlay_enabled_)
     DisableOverlay();
   if (observer_)
     observer_->OnExitedFullscreen();
 
-  // TODO(liberato): if the decoder provided a callback for fullscreen state,
-  // then notify it now.
+  // See EnteredFullscreen for why we do this.
+  if (!decoder_requires_restart_for_overlay_)
+    MaybeSendOverlayInfoToDecoder(false);
 }
 
 void WebMediaPlayerImpl::BecameDominantVisibleContent(bool isDominant) {
@@ -1718,14 +1732,15 @@ void WebMediaPlayerImpl::NotifyDownloading(bool is_downloading) {
 void WebMediaPlayerImpl::OnSurfaceCreated(int surface_id) {
   DCHECK(overlay_mode_ == OverlayMode::kUseContentVideoView);
   overlay_surface_id_ = surface_id;
-  MaybeSendOverlayInfoToDecoder();
+  MaybeSendOverlayInfoToDecoder(true);
 }
 
 void WebMediaPlayerImpl::OnOverlayRoutingToken(
     const base::UnguessableToken& token) {
   DCHECK(overlay_mode_ == OverlayMode::kUseAndroidOverlay);
-  overlay_routing_token_ = token;
-  MaybeSendOverlayInfoToDecoder();
+  // TODO(liberato): |token| should already be a RoutingToken.
+  overlay_routing_token_ = OverlayInfo::RoutingToken(token);
+  MaybeSendOverlayInfoToDecoder(true);
 }
 
 void WebMediaPlayerImpl::OnOverlayInfoRequested(
@@ -1759,50 +1774,47 @@ void WebMediaPlayerImpl::OnOverlayInfoRequested(
   // without an overlay if the restart that happens on entering fullscreen
   // succeeds before we have the overlay info.  Post-M, we could send what we
   // have unconditionally.  When the info arrives, it will be sent.
-  MaybeSendOverlayInfoToDecoder();
+  MaybeSendOverlayInfoToDecoder(true);
 }
 
-void WebMediaPlayerImpl::MaybeSendOverlayInfoToDecoder() {
+void WebMediaPlayerImpl::MaybeSendOverlayInfoToDecoder(bool send_factory_info) {
   // If the decoder didn't request overlay info, then don't send it.
   if (!provide_overlay_info_cb_)
     return;
 
+  OverlayInfo overlay_info;
+
   // We should send the overlay info as long as we know it.  This includes the
   // case where |!overlay_enabled_|, since we want to tell the decoder to avoid
   // using overlays.  Assuming that the decoder has requested info, the only
   // case in which we don't want to send something is if we've requested the
   // info but not received it yet.  Then, we should wait until we do.
+  //
+  // Note that we exit early even if |!send_factory_info|; if we have a callback
+  // pending, then we wait, so that we can send all of it at once.
+  // Initialization requires this; AVDA should start with the factory info.  In
+  // non-init cases, it would probably be fine.
   if (overlay_mode_ == OverlayMode::kUseContentVideoView) {
     if (!overlay_surface_id_.has_value())
       return;
+
+    if (send_factory_info)
+      overlay_info.surface_id = *overlay_surface_id_;
   } else if (overlay_mode_ == OverlayMode::kUseAndroidOverlay) {
     if (!overlay_routing_token_.has_value())
       return;
-  }
 
-  // Note that we're guaranteed that both |overlay_surface_id_| and
-  // |overlay_routing_token_| have values, since both have values unless there
-  // is a request pending.  Nobody calls us if a request is pending.
-
-  int surface_id = SurfaceManager::kNoSurfaceID;
-  if (overlay_surface_id_)
-    surface_id = *overlay_surface_id_;
+    if (send_factory_info)
+      overlay_info.routing_token = *overlay_routing_token_;
+  }
 
-  // Since we represent "no token" as a null UnguessableToken, we translate it
-  // into an optional here.  Alternatively, we could represent it as a
-  // base::Optional in |overlay_routing_token_|, but then we'd have a
-  // base::Optional<base::Optional<base::UnguessableToken> >.  We don't do that
-  // because... just because.
-  base::Optional<base::UnguessableToken> routing_token;
-  if (overlay_routing_token_.has_value() && !overlay_routing_token_->is_empty())
-    routing_token = *overlay_routing_token_;
+  overlay_info.is_fullscreen = is_fullscreen_;
 
   // If restart is required, the callback is one-shot only.
   if (decoder_requires_restart_for_overlay_) {
-    base::ResetAndReturn(&provide_overlay_info_cb_)
-        .Run(surface_id, routing_token);
+    base::ResetAndReturn(&provide_overlay_info_cb_).Run(overlay_info);
   } else {
-    provide_overlay_info_cb_.Run(surface_id, routing_token);
+    provide_overlay_info_cb_.Run(overlay_info);
   }
 }