This class is actually a mix of ContentViewClient, ContentViewCoreClient and other random stuff, often with hooks added specific to Chrome, WebView or something else.
I think this can roughly be moved around as follows:
https://docs.google.com/a/chromium.org/document/d/1Xu9gw6imaXWrhI88VC14awG6WqN1UG-89WK1dz8_TqQ/edit?usp=sharing
Which involves
- moving stuff to other existing interfaces
- overriding behavior through (anonymous) subclassing
- etc.
It also therefore depends on 617750.
If |getContentVideoViewEmbedder| is moved to *WebContentsViewDelegateAndroid, content module cannot refer to it any more (content may not have a dependency on component).
One way I can think of to work it around is to pass a reference to |ContentVideoViewEmbedder| through native upon ContentVideoView creation.
That's the implementation which is accessible through dependency injection from inside content (note that WebContentsDelegateAndroid is the component, probably because it's shared with webview, while WebContents*View*DelegateAndroid are implemented for webview and chrome separately).
Btw.. these are also just suggestions, there might be better places :)
I thought this might be a straightforward way because the call sites that create a ContentVideoView from browser_surface_view_manager.cc and browser_media_player_manager.cc are associated with the WebContents and would be able to 'delegate' the creation of the ContentVideoViewEmbedder to the.. err.. embedder that implements this.
But also: now I don't see anyone implement this different from the ActivityContentVideoViewEmbedder that's already provided in content. So maybe this can be even simpler?
> I thought this might be a straightforward way because the call sites that
> create a ContentVideoView from browser_surface_view_manager.cc and
> browser_media_player_manager.cc are associated with the WebContents and would
> be able to 'delegate' the creation of the ContentVideoViewEmbedder to the..
> err.. embedder that implements this.
This is what I meant by 'pass a reference to ContentVideoViewEmbedder through native'. Embedders provides the instance upon creation of ContentVideoView.
> But also: now I don't see anyone implement this different from the
> ActivityContentVideoViewEmbedder that's already provided in content. So
> maybe this can be even simpler?
I see Tab.java use a slightly different version from ActivityContentVideoViewEmbedder.
Somehow 3th update didn't show up here. This is just for record's sake.
commit 0c5dcfb3579be17d056ef8738b0bb9b0c3e20e09
Author: jinsukkim <jinsukkim@chromium.org>
Date: Mon Dec 5 20:29:46 2016 -0800
Refactor ContentViewClient (3/6)
Replaced a Chrome-specific interface getDesired{Width,Height}MeasureSpec
with explicit calls onContentView to set the size.
BUG= 620172
Review-Url: https://codereview.chromium.org/2536223003
Cr-Commit-Position: refs/heads/master@{#436522}
Comment 1 by siev...@chromium.org
, Jun 15 2016