Implement OverlayPanels as SceneOverlays

chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java

Index: chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
index bd9dc361f4ca6f52f149966704ce50987c1325db..ba4234f59cfe29270e027916d7b6ba65dd9caec7 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
@@ -11,7 +11,9 @@ import android.os.Handler;
 import org.chromium.base.ActivityState;
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.VisibleForTesting;
+
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.compositor.LayerTitleCache;
 import org.chromium.chrome.browser.compositor.bottombar.OverlayContentProgressObserver;
 import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel;
 import org.chromium.chrome.browser.compositor.bottombar.OverlayPanelContent;
@@ -19,10 +21,12 @@ import org.chromium.chrome.browser.compositor.bottombar.OverlayPanelManager;
 import org.chromium.chrome.browser.compositor.bottombar.OverlayPanelManager.PanelPriority;
 import org.chromium.chrome.browser.compositor.bottombar.contextualsearch.ContextualSearchPromoControl.ContextualSearchPromoHost;
 import org.chromium.chrome.browser.compositor.layouts.LayoutUpdateHost;
+import org.chromium.chrome.browser.compositor.layouts.eventfilter.EventFilterHost;
 import org.chromium.chrome.browser.compositor.scene_layer.ContextualSearchSceneLayer;
-import org.chromium.chrome.browser.compositor.scene_layer.SceneLayer;
+import org.chromium.chrome.browser.compositor.scene_layer.SceneOverlayLayer;
 import org.chromium.chrome.browser.contextualsearch.ContextualSearchManagementDelegate;
 import org.chromium.chrome.browser.util.MathUtils;
+import org.chromium.content.browser.ContentViewCore;
 import org.chromium.ui.resources.ResourceManager;
 
 /**
@@ -72,12 +76,12 @@ public class ContextualSearchPanel extends OverlayPanel {
     /**
      * @param context The current Android {@link Context}.
      * @param updateHost The {@link LayoutUpdateHost} used to request updates in the Layout.
+     * @param eventHost The {@link EventFilterHost} for propagating events.
      * @param panelManager The object managing the how different panels are shown.
      */
     public ContextualSearchPanel(Context context, LayoutUpdateHost updateHost,
-                OverlayPanelManager panelManager) {
-        super(context, updateHost, panelManager);
-
+                EventFilterHost eventHost, OverlayPanelManager panelManager) {
+        super(context, updateHost, eventHost, panelManager);
         mSceneLayer = createNewContextualSearchSceneLayer();
         mPanelMetrics = new ContextualSearchPanelMetrics();
 
@@ -124,30 +128,34 @@ public class ContextualSearchPanel extends OverlayPanel {
     }
 
     // ============================================================================================
-    // Scene layer
+    // Scene Overlay
     // ============================================================================================
 
-    @Override
-    public SceneLayer getSceneLayer() {
-        return mSceneLayer;
+    /**
+     * Create a new scene layer for this panel. This should be overridden by tests as necessary.
+     */
+    protected ContextualSearchSceneLayer createNewContextualSearchSceneLayer() {
+        return new ContextualSearchSceneLayer(mContext.getResources().getDisplayMetrics().density);
     }
 
     @Override
-    public void updateSceneLayer(ResourceManager resourceManager) {
-        if (mSceneLayer == null) return;
-
+    public SceneOverlayLayer getUpdatedSceneOverlayTree(LayerTitleCache layerTitleCache,
+            ResourceManager resourceManager, float yOffset) {
+        if (isShowing()) {
+            updateContentFocus();

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

I think we should only call the code to focus/unfocus once when first
showing/hiding the panel layer, and not calling on every frame update.

Also a method named get<Something> should not have side effects.

At least, the only side effect I would expect to happen here is updating the
layer, since it's called getUpdatedSceneOverlayTree(). But other than that, I
would not expect any other side effects.

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

The contentviewcore may not be created by the time this is called. Unlike in the
past, this method is now called even when the panel is not showing. The panel
also needs to give/take focus when it goes from peek/open/peek without
destroying the CVC. I can re-architect to have events like
onContentViewCoreVisible but I'd rather save that for a separate cl.

+        }

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

Not knowing the history of this code particularly well, can mSceneLayer no
longer null when this is called (where as it could in the old
updateSceneLayer())?

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

mSceneLayer is built in the constructor. The creator method can be overridden,
but it is only supposed to be for testing.

         mSceneLayer.update(resourceManager, this,
                 getSearchBarControl(),
                 getPeekPromoControl(),
                 getPromoControl(),
                 getIconSpriteControl());
+
+        return mSceneLayer;
     }
 
-    /**
-     * Create a new scene layer for this panel. This should be overridden by tests as necessary.
-     */
-    protected ContextualSearchSceneLayer createNewContextualSearchSceneLayer() {
-        return new ContextualSearchSceneLayer(mContext.getResources().getDisplayMetrics().density);
+    private void focusBasePage() {
+        ContentViewCore baseContentView = mActivity.getActivityTab().getContentViewCore();
+

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

Left over method? It doesn't look like this is used. If it is necessary, is it
actually focusing the base page?

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

Definitely a method I cut the code from and forgot to remove the shell.

     }
 
     // ============================================================================================
@@ -257,6 +265,8 @@ public class ContextualSearchPanel extends OverlayPanel {
         setProgressBarVisible(false);
 
         super.onClosed(reason);
+
+        if (mSceneLayer != null) mSceneLayer.hideTree();

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

Can't this be done by the OverlayPanel class?

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

OverlayPanel currently doesn't have a concept of SceneLayer. Further, hideTree
is not a shared method of SceneLayer so we would need a base class for java
SceneLayers. I can do this, but as I mentioned earlier, I think it might be
worth having a follow-up CL.

     }
 
     // ============================================================================================
@@ -329,11 +339,6 @@ public class ContextualSearchPanel extends OverlayPanel {
     }
 
     @Override
-    public boolean supportsContextualSearchLayout() {
-        return mManagementDelegate != null && !mManagementDelegate.isRunningInCompatibilityMode();
-    }
-
-    @Override
     public void notifyBarTouched(float x) {
         if (canDisplayContentInPanel()) {
             getOverlayPanelContent().showContent();