New issue
Advanced search Search tips

Issue 667904 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: ----



Sign in to add a comment

Remove Tab <-> FullscreenManager cyclical dependency

Project Member Reported by tedc...@chromium.org, Nov 22 2016

Issue description

Tab and FullscreenManager are too tightly coupled.  FullscreenManager listens for all tab changes from TabModelSelector and registers itself to a particular Tab.

The tab should instead broadcast any fullscreen state changes and it should be completely unaware of FullscreenManager.

Look for usages of Tab#getFullscreenManager() outside of tab and start whittling those down.

Some initial issues are:

1.) ActivityContentVideoViewEmbedder in TabWebContentsDelegateAndroid.
  * Plan: Move to TabDelegateFactory (convert this to a proper interface and move initial implementation outside of tab package).

2.) FullscreenManager relies on child view updates and system UI updates via:
    cvc.getContainerView().setOnHierarchyChangeListener(this);
    cvc.getContainerView().setOnSystemUiVisibilityChangeListener(this);
  * Make those broadcasts on TabObserver

3.) FullscreenManager gets position updates from Tab directly through setPositionsForTab*
  * Store positions internally in tab and broadcast them out through TabObserver

4.) Supporting toggleFullscreenModeForTab
  * Have FullscreenManager instead listen to the existing onToggleFullscreenMode

In general, the plan will be to replace the direct calls with new TabObserver methods, or have FullscreenManager register for the necessary listener updates from Tab instead of Tab doing this and then talking to FullscreenManager.
 
For #1, getContentVideoViewEmbedder is exposed in TabWebContentsDelegateAndroid, which is already exposed in TabDelegateFactory.  So instead of adding a new delegate method, we could just require the caller to overwrite that.

But if we move all other callers to use TabObserver, then maybe it makes sense to add onEnterFullscreenVideo and onExitFullscreenVideo instead of requiring the caller to overwrite multiple layers there.

Sign in to add a comment