Implement OverlayPanels as SceneOverlays

chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java

Index: chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java
index 9a031f21440c8daacf671ebb2034808e601f187b..9a23ff2823df053bf217596404e45c2d479a70b5 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java
@@ -14,13 +14,18 @@ import org.chromium.base.ApplicationStatus;
 import org.chromium.base.ApplicationStatus.ActivityStateListener;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.compositor.LayerTitleCache;
 import org.chromium.chrome.browser.compositor.bottombar.OverlayPanelManager.PanelPriority;
 import org.chromium.chrome.browser.compositor.layouts.LayoutUpdateHost;
+import org.chromium.chrome.browser.compositor.layouts.components.VirtualView;
 import org.chromium.chrome.browser.compositor.layouts.eventfilter.EdgeSwipeEventFilter.ScrollDirection;
 import org.chromium.chrome.browser.compositor.layouts.eventfilter.EdgeSwipeHandler;
 import org.chromium.chrome.browser.compositor.layouts.eventfilter.EventFilter;
+import org.chromium.chrome.browser.compositor.layouts.eventfilter.EventFilterHost;
 import org.chromium.chrome.browser.compositor.layouts.eventfilter.GestureHandler;
-import org.chromium.chrome.browser.compositor.scene_layer.SceneLayer;
+import org.chromium.chrome.browser.compositor.layouts.eventfilter.OverlayPanelEventFilter;
+import org.chromium.chrome.browser.compositor.overlays.SceneOverlay;
+import org.chromium.chrome.browser.compositor.scene_layer.SceneOverlayLayer;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.content.browser.ContentVideoViewEmbedder;
 import org.chromium.content.browser.ContentViewClient;
@@ -29,21 +34,19 @@ import org.chromium.content_public.common.TopControlsState;
 import org.chromium.ui.base.LocalizationUtils;
 import org.chromium.ui.resources.ResourceManager;
 
+import java.util.List;
+
 /**
  * Controls the Overlay Panel.
  */
 public class OverlayPanel extends OverlayPanelAnimation implements ActivityStateListener,
-        EdgeSwipeHandler, GestureHandler, OverlayPanelContentFactory {
+        EdgeSwipeHandler, GestureHandler, OverlayPanelContentFactory, SceneOverlay {
 
-    /**
-     * The extra dp added around the close button touch target.
-     */
+    /** The extra dp added around the close button touch target. */
     private static final int CLOSE_BUTTON_TOUCH_SLOP_DP = 5;
 
-    /**
-     * State of the Overlay Panel.
-     */
-    public enum PanelState {
+    /** State of the Overlay Panel. */
+    public static enum PanelState {
         // TODO(pedrosimonetti): consider removing the UNDEFINED state
         UNDEFINED,
         CLOSED,
@@ -108,12 +111,12 @@ public class OverlayPanel extends OverlayPanelAnimation implements ActivityState
     /** Container for content the panel will show. */
     private OverlayPanelContent mContent;
 
-    /** The {@link OverlayPanelHost} used to communicate with the supported layout. */
-    private OverlayPanelHost mOverlayPanelHost;
-
     /** OverlayPanel manager handle for notifications of opening and closing. */
     protected OverlayPanelManager mPanelManager;
 
+    /** If the panel is focused. */
+    private boolean mHasFocus;
+
     // ============================================================================================
     // Constructor
     // ============================================================================================
@@ -121,15 +124,17 @@ public class OverlayPanel extends OverlayPanelAnimation implements ActivityState
     /**
      * @param context The current Android {@link Context}.
      * @param updateHost The {@link LayoutUpdateHost} used to request updates in the Layout.
+     * @param eventHost The {@link EventFilterHost} used to propagate events.
      * @param panelManager The {@link OverlayPanelManager} responsible for showing panels.
      */
-    public OverlayPanel(Context context, LayoutUpdateHost updateHost,
+    public OverlayPanel(Context context, LayoutUpdateHost updateHost, EventFilterHost eventHost,
                 OverlayPanelManager panelManager) {
         super(context, updateHost);
         mContentFactory = this;
 
         mPanelManager = panelManager;
         mPanelManager.registerPanel(this);
+        mEventFilter = new OverlayPanelEventFilter(mContext, eventHost, this);
     }
 
     /**
@@ -150,6 +155,7 @@ public class OverlayPanel extends OverlayPanelAnimation implements ActivityState
 
     @Override
     protected void onClosed(StateChangeReason reason) {
+        updateContentFocus();
         destroyComponents();
         mPanelManager.notifyPanelClosed(this, reason);
     }
@@ -177,13 +183,6 @@ public class OverlayPanel extends OverlayPanelAnimation implements ActivityState
         if (!isShowing()) return;
 
         super.closePanel(reason, animate);
-
-        // If the close action is animated, the Layout will be hidden when
-        // the animation is finished, so we should only hide the Layout
-        // here when not animating.
-        if (!animate && mOverlayPanelHost != null) {
-            mOverlayPanelHost.hideLayout(true);
-        }
     }
 
     /**
@@ -284,6 +283,31 @@ public class OverlayPanel extends OverlayPanelAnimation implements ActivityState
         return -mActivity.getFullscreenManager().getControlOffset() * mPxToDp;
     }
 
+    /**
+     * Set focus on the panel's content and reomve it from the base page.

[Theresa, 2016/04/21 21:22:38]

s/reomve/remove

I think this could also be expanded, e.g.
"If the panel is open and doesn't have focus, set focus on the panel's content
and remove it from the base page. If the panel is not open and has focus, set
focus on the base page."

[mdjones, 2016/04/22 17:03:41]

Done.

+     */
+    protected void updateContentFocus() {
+        if (mActivity == null) return;
+        ContentViewCore baseContentView = mActivity.getActivityTab().getContentViewCore();
+        ContentViewCore panelContentView = getContentViewCore();
+        // Remove focus from the base page.
+        if (!mHasFocus && isPanelOpened()) {
+            baseContentView.preserveSelectionOnNextLossOfFocus();
+            baseContentView.onDetachedFromWindow();

[pedro (no code reviews), 2016/04/22 02:11:04]

Please confirm with someone more familiar with the ContentView/ContentViewCore
if this is okay. It seems weird that we would call onDetachedFromWindow without
actually detaching it.

It seems that what you really want here is to set/unset the focus on the base
page's ContentView. The ContentView already has the requestFocus() method (from
Android's View class) and it seems that trying to call that instead might be
better here.

[pedro (no code reviews), 2016/04/22 06:30:30]

Just to be clear. Once preserveSelectionOnNextLossOfFocus() is called, all we
should do is to unfocus the base content view (by focusing the panel's content
view), and then ContentViewCore should handle preserving the selection and other
things (hiding Context Menu, etc.). If this is not happening, it is a bug in the
ContentViewCore code.

+        }
+
+        // Set focus on the overlay content.
+        if (!mHasFocus && isPanelOpened() && panelContentView != null) {

[Theresa, 2016/04/21 21:22:38]

I think this would read a little better if the this block was combined w/ the if
above. maybe:

if (!mHasFocus && isPanelOpened()) {
    // Remove focus from the base page.
    baseContentView.preserveSelectionOnNextLossOfFocus();
    baseContentView.onDetachedFromWindow();

    // Set focus on the overlay content.
    if (panelContentView != null) {
        mHasFocus = true;
        panelContentView.getContainerView().requestFocus();
        panelContentView.onAttachedToWindow();
    }
} else if (mHasFocus && !isPanelOpened() && baseContentView != null) {
   // Give focus to the base page.
   ....
}

[pedro (no code reviews), 2016/04/22 02:11:04]

I think it would be even better if there was separate methods focusBaseContent()
and unfocusBaseContent(), then we wouldn't have to add lots of comments
explaining the complexity of the method since each one would have a clear and
specific purpose.

[mdjones, 2016/04/22 17:03:41]

Done.

+            mHasFocus = true;
+            panelContentView.getContainerView().requestFocus();
+            panelContentView.onAttachedToWindow();

[pedro (no code reviews), 2016/04/22 02:11:04]

Same comment about the onDetachedFromWindow() applies here.

+        } else if (mHasFocus && !isPanelOpened() && baseContentView != null) {
+            mHasFocus = false;
+            baseContentView.getContainerView().requestFocus();
+            baseContentView.onAttachedToWindow();
+        }
+    }
+
     // ============================================================================================
     // ActivityStateListener
     // ============================================================================================
@@ -456,32 +480,6 @@ public class OverlayPanel extends OverlayPanelAnimation implements ActivityState
     }
 
     // ============================================================================================
-    // Animation Handling
-    // ============================================================================================
-
-    @Override
-    protected void onAnimationFinished() {
-        super.onAnimationFinished();
-
-        if (shouldHideOverlayPanelLayout()) {
-            if (mOverlayPanelHost != null) {
-                mOverlayPanelHost.hideLayout(false);
-            }
-        }
-    }
-
-    /**
-     * Whether the Overlay Panel Layout should be hidden.
-     *
-     * @return Whether the Overlay Panel Layout should be hidden.
-     */
-    private boolean shouldHideOverlayPanelLayout() {
-        final PanelState state = getPanelState();
-        return (state == PanelState.PEEKED || state == PanelState.CLOSED)
-                && getHeight() == getPanelHeightFromState(state);
-    }
-
-    // ============================================================================================
     // ContextualSearchPanelBase methods.
     // ============================================================================================
 
@@ -497,38 +495,6 @@ public class OverlayPanel extends OverlayPanelAnimation implements ActivityState
     // ============================================================================================
 
     /**
-     * Sets the {@link OverlayPanelHost} used to communicate with the supported layout.
-     * @param host The {@link OverlayPanelHost}.
-     */
-    public void setHost(OverlayPanelHost host) {
-        mOverlayPanelHost = host;
-    }
-
-    /**
-     * @return The scene layer used to draw this panel.
-     */
-    public SceneLayer getSceneLayer() {
-        return null;
-    }
-
-    /**
-     * Update this panel's scene layer. This should be implemented by each panel type.
-     * @param resourceManager Used to access static resources.
-     */
-    public void updateSceneLayer(ResourceManager resourceManager) {
-    }
-
-    /**
-     * Determine if using a second layout for showing the overlay panel is possible. This should
-     * be overridden by each panel and returns true by default.
-     * @return True if the layout is supported.
-     * TODO(mdjones): Rename to supportsOverlayPanelLayout once the corresponding class is renamed.
-     */
-    public boolean supportsContextualSearchLayout() {
-        return true;
-    }
-
-    /**
      * Handles the device orientation change.
      */
     public void onOrientationChanged() {
@@ -790,4 +756,102 @@ public class OverlayPanel extends OverlayPanelAnimation implements ActivityState
     public boolean isSwipeEnabled(ScrollDirection direction) {
         return direction == ScrollDirection.UP && isShowing();
     }
+
+    // ============================================================================================
+    // SceneOverlay implementation.
+    // ============================================================================================
+
+    @Override
+    public SceneOverlayLayer getUpdatedSceneOverlayTree(LayerTitleCache layerTitleCache,
+            ResourceManager resourceManager, float yOffset) {
+        return null;
+    }
+
+    @Override
+    public EventFilter getEventFilter() {
+        return mEventFilter;
+    }
+
+    @Override
+    public void onSizeChanged(float width, float height, float visibleViewportOffsetY,
+            int orientation) {
+        onSizeChanged(width, height);
+    }
+
+    @Override
+    public void getVirtualViews(List<VirtualView> views) {
+        // TODO(mdjones): Add views for accessibility.
+    }
+
+    @Override
+    public boolean handlesTabCreating() {
+        // If the panel is not opened, do not handle tab creating.
+        if (!isPanelOpened()) return false;
+        // Updates TopControls' State so the Toolbar becomes visible.
+        // TODO(pedrosimonetti): The transition when promoting to a new tab is only smooth
+        // if the SearchContentView's vertical scroll position is zero. Otherwise the
+        // ContentView will appear to jump in the screen. Coordinate with @dtrainor to solve
+        // this problem.
+        updateTopControlsState(TopControlsState.BOTH, false);
+        return true;
+    }
+
+    @Override
+    public boolean shouldHideAndroidTopControls() {
+        return isPanelOpened();
+    }
+
+    @Override
+    public boolean updateOverlay(long time, long dt) {
+        return super.onUpdateAnimation(time, false);
+    }
+
+    @Override
+    public void onHideLayout() {
+        if (!isShowing()) return;
+        closePanel(StateChangeReason.UNKNOWN, false);
+    }
+
+    @Override
+    public boolean onBackPressed() {
+        if (!isPanelOpened()) return false;
+        closePanel(StateChangeReason.BACK_PRESS, true);
+        return true;
+    }
+
+    @Override
+    public void tabTitleChanged(int tabId, String title) {}

[pedro (no code reviews), 2016/04/22 02:11:04]

It feels really weird that a SceneOverlay needs all of these Tab methods. It
looks like a bunch of things that only StripLayoutHelperManager() cares about
was included in the SceneOverlay interface, which is sad.

[mdjones, 2016/04/22 17:03:41]

This interface was originally built with tab related additions in mind. It may
be possible to separate the functionality in the future.

+
+    @Override
+    public void tabStateInitialized() {}
+
+    @Override
+    public void tabModelSwitched(boolean incognito) {}
+
+    @Override
+    public void tabSelected(long time, boolean incognito, int id, int prevId) {}
+
+    @Override
+    public void tabMoved(long time, boolean incognito, int id, int oldIndex, int newIndex) {}
+
+    @Override
+    public void tabClosed(long time, boolean incognito, int id) {}
+
+    @Override
+    public void tabClosureCancelled(long time, boolean incognito, int id) {}
+
+    @Override
+    public void tabCreated(long time, boolean incognito, int id, int prevId, boolean selected) {}
+
+    @Override
+    public void tabPageLoadStarted(int id, boolean incognito) {}
+
+    @Override
+    public void tabPageLoadFinished(int id, boolean incognito) {}
+
+    @Override
+    public void tabLoadStarted(int id, boolean incognito) {}
+
+    @Override
+    public void tabLoadFinished(int id, boolean incognito) {}
 }