I think we likely want to disallow Contextual Search when the activity is showing an overlay, so I suspect we don't need any changes, but over to donnd@ to confirm.

*.. showing an overlay whether it is fullscreen or not...

what goes wrong if it's showing an overlay?

There are a couple of issues. Contextual search is itself an overlay, so the UIs don't work well together. Also, for video overlays contextual search doesn't really make sense -- that aren't any text components in a video overlay (that we're aware of) that users would want to search the web for.

I'd like to disambiguate "Overlay" in this context.

For the media team, Overlay means an instance of AndroidOverlay, which is a power efficient way of displaying video on screen, but looks exactly like regular video to users, and does not seem to appear over/under anything else when it is composited.

I suspect that the rest of Chrome has a definition of Overlay as something like "A UI element that appears *over* other Chrome UI elements".

We appreciate the early heads up. For the short-term, keeping contextual search disabled when isOverlayVideo() is true is the safest and easiest approach. At some point in the future, if Donn or myself has extra cycles we can look into whether it should actually be enabled in some situations where isOverlayVideo() returns true.


For overlay, I mean the simplest definition of the word "something laid over something else."

The current code documentation very clearly states that if FullscreenManager#isOverlayVideo() returns true there is a fullscreen view playing a video:
 - From FullscreenManager#isOverlayouVideoMode(): "@return Check whether ContentVideoView is shown."
 - From ContentVideoVideo: "A fullscreen view for accelerated video playback using surface view."

Chrome uses #isOverlayVideo() in some other places where we except a fullscreen view playing a video. Based on this bug report it sounds like you're doing an audit of uses of #isOverlayVideo(), which is great! I'd encourage you to look at all of the current uses and try to understand the implications of the re-definition.


Looking at your examples:

 - "if one full-screens a <div> enclosing the video player...." We may not want Contextual Search to be enabled in that scenario but we haven't actually encountered any reports where unexpected things happening in the wild. The worst-case scenario is that contextual search will show up when the user long-presses on text. I assume most full-screen div's showing videos don't have a lot of text.

 - "webmediaplayer_impl might decide not to enable overlay mode..." In this scenario I would expect FullscreenManager#isOverlayVideoMode() to return false based on the documentation. If a ContentVideoView isn't actually shown, #isOverlayVideoMode() should be returning false.

 - "shortly, there will be changes that will entirely decouple..."  I don't know what protected content overlays are so it's hard to saw whether Contextual Search should be enabled while they are showing.

I agree with all the comments by twellington@.  An example of a bad interaction we've seen long ago was the video's text controls (for video position and duration) were clickable, but Contextual Search couldn't produce any useful result.  So that possibility seems like another reason to just disable Contextual Search when we think a full screen video is shown.

 Issue 736514  has been merged into this issue.

tguilbert@ or liberato@ please feel free to reprioritize once action is required for clients of FullscreenManager.isOverlayVideoMode.