Send enter / exit fullscreen signal to AVDA

media/blink/webmediaplayer_impl.h

Index: media/blink/webmediaplayer_impl.h
diff --git a/media/blink/webmediaplayer_impl.h b/media/blink/webmediaplayer_impl.h
index 8a6c51b9c487bbc5eb0baaec8cdbe351055fe592..842437e9664242040b9af767719be1e69fd7a911 100644
--- a/media/blink/webmediaplayer_impl.h
+++ b/media/blink/webmediaplayer_impl.h
@@ -259,8 +259,11 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // AndroidOverlay, this is the routing token.
   bool HaveOverlayInfo();
 
-  // Send the overlay surface ID / routing token to the decoder if we have it
-  // and if it has been requested.
+  // Send OverlayInfo to the decoder.
+  //
+  // If we've requested but not yet received the surface id or routing token, or
+  // if there's no decoder-provided callback to send the overlay info, then this
+  // call will do nothing.
   void MaybeSendOverlayInfoToDecoder();
 
   void OnPipelineSuspended();
@@ -775,11 +778,12 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // Optional callback to request the routing token for AndroidOverlay.
   RequestRoutingTokenCallback request_routing_token_cb_;
 
-  // Routing token, if we have one.  No value if we have a request pending to
-  // get it via |request_routing_token_cb_|.  A has_value() is_empty() token
-  // indicates that we requested and received an empty token.  Note that we
-  // can't send an empty token via IPC, so we handle that specially.
-  base::Optional<base::UnguessableToken> overlay_routing_token_;
+  // Routing token, if we have one.  We always have one, unless we've requested
+  // one via |request_routing_token_cb_| but not yet received it.  In that case,
+  // |overlay_routing_token_| won't have a value.

[watk, 2017/05/25 21:36:20]

Nice, thanks

+  base::Optional<OverlayInfo::RoutingToken> overlay_routing_token_;

[dcheng, 2017/06/02 15:30:06]

Hmm, isn't this an Optional<Optional<OverlayInfo::RoutingToken>> due to the
typedefs? Can this be just one level of Optional?

[liberato (no reviews please), 2017/06/02 18:18:59]

it is O<O<>>, but i don't see how it can be reduced easily.

the problem is that we need a way to denote explicitly "no token". however,
there's no distiguished value for UnguessableToken; an is_null() token isn't
serializable.

the inner Optional<> is used to denote this -- a !has_value() RoutingToken means
"no token" here and in other places like AVDA.

the outer Optional<> is used by wmpi to figure out if it knows the answer to the
question 'what is my routing token', like it does for Optional<int> surface_id
elsewhere.  a !has_value |overlay_routing_token_| means that the question has
been asked via callback, but not yet answered.

i could make an explicit bool out of the outer one, but that's pretty much what
Optional<> does.  keeping it together seemed clearer.

suggestions welcome.

[dcheng, 2017/06/05 18:32:10]

Perhaps just integrate your reply here into the comment; I found it quite
instructive.

[liberato (no reviews please), 2017/06/07 20:06:33]

i ended up taking your other suggestion, to split up the Optional<> into a
RoutingToken and a bool.

+
+  OverlayInfo overlay_info_;
 
   DISALLOW_COPY_AND_ASSIGN(WebMediaPlayerImpl);
 };