android: make ViewAndroidDelegate.getContainerView() return a View

While ViewAndroidDelegate takes a ViewGroup, all other places
should not be using ViewGroup, but rather View. This is because
the container-view is a ContentView, and ContentView should not
have children other than those added by ViewAndroidDelegate.

BUG=1075585
TEST=none

Change-Id: I9ba6e1b3aad6d1c780b9383de78f73e5cdebfd28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2191379
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769871}

android_webview/java/src/org/chromium/android_webview/AwViewAndroidDelegate.java

@@ -65,7 +65,7 @@ public class AwViewAndroidDelegate extends ViewAndroidDelegate {
 
     @Override
     public View acquireView() {
-        ViewGroup containerView = getContainerView();
+        ViewGroup containerView = getContainerViewGroup();
         if (containerView == null) return null;
         View anchorView = new View(containerView.getContext());
         containerView.addView(anchorView);
@@ -77,7 +77,7 @@ public class AwViewAndroidDelegate extends ViewAndroidDelegate {
     @Override
     public void removeView(View anchorView) {
         mAnchorViews.remove(anchorView);
-        ViewGroup containerView = getContainerView();
+        ViewGroup containerView = getContainerViewGroup();
         if (containerView != null) {
             containerView.removeView(anchorView);
         }
@@ -104,7 +104,7 @@ public class AwViewAndroidDelegate extends ViewAndroidDelegate {
     @Override
     public void setViewPosition(View anchorView, float x, float y, float width, float height,
             int leftMargin, int topMargin) {
-        ViewGroup containerView = getContainerView();
+        ViewGroup containerView = getContainerViewGroup();
         if (!mAnchorViews.containsKey(anchorView) || containerView == null) return;
 
         mAnchorViews.put(anchorView, new Position(x, y, width, height, leftMargin, topMargin));

chrome/android/features/keyboard_accessory/javatests/src/org/chromium/chrome/browser/keyboard_accessory/ManualFillingTestHelper.java

@@ -259,7 +259,7 @@ public class ManualFillingTestHelper {
 
     public DropdownPopupWindowInterface waitForAutofillPopup(String filterInput) {
         final WebContents webContents = mActivityTestRule.getActivity().getCurrentWebContents();
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
+        final View view = webContents.getViewAndroidDelegate().getContainerView();
 
         // Wait for InputConnection to be ready and fill the filterInput. Then wait for the anchor.
         CriteriaHelper.pollUiThread(

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

@@ -8,6 +8,7 @@ import android.app.Activity;
 import android.content.Context;
 import android.graphics.RectF;
 import android.os.Handler;
+import android.view.View;
 import android.view.ViewGroup;
 
 import androidx.annotation.IntDef;
@@ -343,7 +344,7 @@ public class OverlayPanel extends OverlayPanelAnimation implements ActivityState
     // Claim or lose focus of the content view of the base WebContents. This keeps
     // the state of the text selected for overlay in consistent way.
     private static void updateFocus(WebContents baseWebContents, boolean focusBaseView) {
-        ViewGroup baseContentView = baseWebContents.getViewAndroidDelegate() != null
+        View baseContentView = baseWebContents.getViewAndroidDelegate() != null
                 ? baseWebContents.getViewAndroidDelegate().getContainerView()
                 : null;
         if (baseContentView == null) return;

chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupTest.java

@@ -11,7 +11,6 @@ import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
 import android.text.TextUtils;
 import android.view.View;
-import android.view.ViewGroup;
 
 import androidx.annotation.Nullable;
 
@@ -209,7 +208,7 @@ public class AutofillPopupTest {
         loadForm(formDataUrl, inputText, updateActivity);
 
         final WebContents webContents = mActivityTestRule.getActivity().getCurrentWebContents();
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
+        final View view = webContents.getViewAndroidDelegate().getContainerView();
         waitForAnchorViewAdd(view);
         View anchorView = view.findViewById(R.id.dropdown_popup_window);
 
@@ -359,7 +358,7 @@ public class AutofillPopupTest {
         final Configuration config = activity.getResources().getConfiguration();
         final boolean shouldShowPopup = config.orientation == Configuration.ORIENTATION_PORTRAIT
                 || config.isLayoutSizeAtLeast(Configuration.SCREENLAYOUT_SIZE_XLARGE);
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
+        final View view = webContents.getViewAndroidDelegate().getContainerView();
         if (shouldShowPopup) {
             waitForAnchorViewAdd(view);
         } else {
@@ -385,7 +384,7 @@ public class AutofillPopupTest {
                 Criteria.equals(count, () -> immw.getShowSoftInputCounter()));
     }
 
-    private void waitForAnchorViewAdd(final ViewGroup view) {
+    private void waitForAnchorViewAdd(final View view) {
         CriteriaHelper.pollUiThread(new Criteria(
                 "Autofill Popup anchor view was never added.") {
             @Override

chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillUpstreamTest.java

@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.autofill;
 
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
-import android.view.ViewGroup;
 
 import org.junit.After;
 import org.junit.Assert;
@@ -81,7 +80,7 @@ public class AutofillUpstreamTest {
         Assert.assertEquals(buttonLabel, primaryButton.getText().toString());
     }
 
-    private void waitForSaveCardInfoBar(final ViewGroup view) {
+    private void waitForSaveCardInfoBar() {
         CriteriaHelper.pollUiThread(
                 new Criteria("Autofill Save Card Infobar view was never added.") {
                     @Override
@@ -122,8 +121,7 @@ public class AutofillUpstreamTest {
 
         DOMUtils.clickNode(webContents, "fill_form");
         DOMUtils.clickNode(webContents, "submit");
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
-        waitForSaveCardInfoBar(view);
+        waitForSaveCardInfoBar();
 
         assertPrimaryButtonLabel(SAVE_BUTTON_LABEL);
     }
@@ -139,8 +137,7 @@ public class AutofillUpstreamTest {
         // Clear the month field
         DOMUtils.clickNode(webContents, "clear_month");
         DOMUtils.clickNode(webContents, "submit");
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
-        waitForSaveCardInfoBar(view);
+        waitForSaveCardInfoBar();
 
         assertPrimaryButtonLabel(CONTINUE_BUTTON_LABEL);
     }
@@ -156,8 +153,7 @@ public class AutofillUpstreamTest {
         // Clear the year field
         DOMUtils.clickNode(webContents, "clear_year");
         DOMUtils.clickNode(webContents, "submit");
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
-        waitForSaveCardInfoBar(view);
+        waitForSaveCardInfoBar();
 
         assertPrimaryButtonLabel(CONTINUE_BUTTON_LABEL);
     }
@@ -173,8 +169,7 @@ public class AutofillUpstreamTest {
         // Clear the month and year field
         DOMUtils.clickNode(webContents, "clear_expiration_date");
         DOMUtils.clickNode(webContents, "submit");
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
-        waitForSaveCardInfoBar(view);
+        waitForSaveCardInfoBar();
 
         assertPrimaryButtonLabel(CONTINUE_BUTTON_LABEL);
     }
@@ -191,8 +186,7 @@ public class AutofillUpstreamTest {
         // Clear the month and year field
         DOMUtils.clickNode(webContents, "clear_expiration_date");
         DOMUtils.clickNode(webContents, "submit");
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
-        waitForSaveCardInfoBar(view);
+        waitForSaveCardInfoBar();
         // Click on the continue button
         TestThreadUtils.runOnUiThreadBlocking(
                 () -> getAutofillSaveCardInfoBar().onButtonClicked(true));
@@ -213,8 +207,7 @@ public class AutofillUpstreamTest {
         // Clear the name field
         DOMUtils.clickNode(webContents, "clear_name");
         DOMUtils.clickNode(webContents, "submit");
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
-        waitForSaveCardInfoBar(view);
+        waitForSaveCardInfoBar();
 
         assertPrimaryButtonLabel(CONTINUE_BUTTON_LABEL);
     }
@@ -231,8 +224,7 @@ public class AutofillUpstreamTest {
         // Clear the name field
         DOMUtils.clickNode(webContents, "clear_name");
         DOMUtils.clickNode(webContents, "submit");
-        final ViewGroup view = webContents.getViewAndroidDelegate().getContainerView();
-        waitForSaveCardInfoBar(view);
+        waitForSaveCardInfoBar();
         // Click on the continue button
         TestThreadUtils.runOnUiThreadBlocking(
                 () -> getAutofillSaveCardInfoBar().onButtonClicked(true));

components/embedder_support/android/java/src/org/chromium/components/embedder_support/view/ContentView.java

@@ -36,6 +36,11 @@ import org.chromium.ui.base.EventForwarder;
 /**
  * The containing view for {@link WebContents} that exists in the Android UI hierarchy and exposes
  * the various {@link View} functionality to it.
+ *
+ * While ContentView is a ViewGroup, the only place that should add children is ViewAndroidDelegate,
+ * and only for cases that WebContentsAccessibility handles (such as anchoring popups). This is
+ * because the accessibility support provided by WebContentsAccessibility ignores all child views.
+ * In other words, any children added to this are *not* accessible.
  */
 public class ContentView extends FrameLayout
         implements ViewEventSink.InternalAccessDelegate, SmartClipProvider,

content/public/android/java/src/org/chromium/content/browser/accessibility/WebContentsAccessibilityImpl.java

@@ -20,7 +20,6 @@ import android.text.style.URLSpan;
 import android.util.SparseArray;
 import android.view.MotionEvent;
 import android.view.View;
-import android.view.ViewGroup;
 import android.view.ViewParent;
 import android.view.ViewStructure;
 import android.view.accessibility.AccessibilityEvent;
@@ -127,7 +126,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
     private boolean mIsHovering;
     private int mLastHoverId = View.NO_ID;
     private int mCurrentRootId;
-    protected ViewGroup mView;
+    protected View mView;
     private boolean mUserHasTouchExplored;
     private boolean mPendingScrollToMakeNodeVisible;
     private boolean mNotifyFrameInfoInitializedCalled;
@@ -244,7 +243,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
                                 buildAccessibilityEvent(virtualViewId, eventType);
                         if (event == null) return false;
 
-                        mView.requestSendAccessibilityEvent(mView, event);
+                        requestSendAccessibilityEvent(event);
 
                         // Always send the ENTER and then the EXIT event, to match a standard
                         // Android View.
@@ -252,7 +251,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
                             AccessibilityEvent exitEvent = buildAccessibilityEvent(
                                     mLastHoverId, AccessibilityEvent.TYPE_VIEW_HOVER_EXIT);
                             if (exitEvent != null) {
-                                mView.requestSendAccessibilityEvent(mView, exitEvent);
+                                requestSendAccessibilityEvent(exitEvent);
                                 mLastHoverId = virtualViewId;
                             }
                         }
@@ -954,8 +953,8 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
         traverseEvent.setContentDescription(text);
         traverseEvent.setAction(AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY);
 
-        mView.requestSendAccessibilityEvent(mView, selectionEvent);
-        mView.requestSendAccessibilityEvent(mView, traverseEvent);
+        requestSendAccessibilityEvent(selectionEvent);
+        requestSendAccessibilityEvent(traverseEvent);
 
         // Suppress the next event since we have already sent traverse and selection for this move
         mSuppressNextSelectionEvent = true;
@@ -1012,8 +1011,8 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
         traverseEvent.setContentDescription(text);
         traverseEvent.setAction(AccessibilityNodeInfo.ACTION_PREVIOUS_AT_MOVEMENT_GRANULARITY);
 
-        mView.requestSendAccessibilityEvent(mView, selectionEvent);
-        mView.requestSendAccessibilityEvent(mView, traverseEvent);
+        requestSendAccessibilityEvent(selectionEvent);
+        requestSendAccessibilityEvent(traverseEvent);
 
         // Suppress the next event since we have already sent traverse and selection for this move
         mSuppressNextSelectionEvent = true;
@@ -1514,6 +1513,14 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
         if (rect.bottom > viewportRectBottom) rect.bottom = viewportRectBottom;
     }
 
+    private void requestSendAccessibilityEvent(AccessibilityEvent event) {
+        // If there is no parent, then the event can be ignored. In general the parent is only
+        // transiently null (such as during teardown, switching tabs...).
+        if (mView.getParent() != null) {
+            mView.getParent().requestSendAccessibilityEvent(mView, event);
+        }
+    }
+
     @CalledByNative
     private void setAccessibilityNodeInfoLocation(AccessibilityNodeInfo node,
             final int virtualViewId, int absoluteLeft, int absoluteTop, int parentRelativeLeft,

content/public/android/javatests/src/org/chromium/content/browser/accessibility/WebContentsAccessibilityTest.java

@@ -258,8 +258,10 @@ public class WebContentsAccessibilityTest {
     private void setUpEditTextDelegate(int editTextVirtualViewId) throws Throwable {
         // Add an accessibility delegate to capture TEXT_TRAVERSED_AT_MOVEMENT_GRANULARITY and
         // TYPE_VIEW_TEXT_SELECTION_CHANGED events and store their ToIndex and FromIndex.
-        mActivityTestRule.getContainerView().setAccessibilityDelegate(
-                new View.AccessibilityDelegate() {
+        // The delegate is set on the parent as WebContentsAccessibilityImpl sends events using the
+        // parent.
+        ((ViewGroup) mActivityTestRule.getContainerView().getParent())
+                .setAccessibilityDelegate(new View.AccessibilityDelegate() {
                     @Override
                     public boolean onRequestSendAccessibilityEvent(
                             ViewGroup host, View child, AccessibilityEvent event) {

content/shell/android/java/src/org/chromium/content_shell/Shell.java

@@ -13,7 +13,6 @@ import android.view.KeyEvent;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.View;
-import android.view.ViewGroup;
 import android.view.inputmethod.EditorInfo;
 import android.view.inputmethod.InputMethodManager;
 import android.widget.EditText;
@@ -378,9 +377,9 @@ public class Shell extends LinearLayout {
     }
 
     /**
-     * @return The {@link ViewGroup} currently shown by this Shell.
+     * @return The {@link View} currently shown by this Shell.
      */
-    public ViewGroup getContentView() {
+    public View getContentView() {
         ViewAndroidDelegate viewDelegate = mWebContents.getViewAndroidDelegate();
         return viewDelegate != null ? viewDelegate.getContainerView() : null;
     }

content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellActivityTestRule.java

@@ -13,7 +13,7 @@ import android.os.Build;
 import android.os.PowerManager;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.rule.ActivityTestRule;
-import android.view.ViewGroup;
+import android.view.View;
 
 import org.hamcrest.Matchers;
 import org.junit.Assert;
@@ -219,7 +219,7 @@ public class ContentShellActivityTestRule extends ActivityTestRule<ContentShellA
     /**
      * Returns the current container view or null if there is no WebContents.
      */
-    public ViewGroup getContainerView() {
+    public View getContainerView() {
         final WebContents webContents = getWebContents();
         try {
             return TestThreadUtils.runOnUiThreadBlocking(() -> {

ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java

@@ -110,7 +110,7 @@ public class ViewAndroidDelegate {
      */
     @CalledByNative
     public View acquireView() {
-        ViewGroup containerView = getContainerView();
+        ViewGroup containerView = getContainerViewGroup();
         if (containerView == null || containerView.getParent() == null) return null;
         View anchorView = new View(containerView.getContext());
         containerView.addView(anchorView);
@@ -123,7 +123,7 @@ public class ViewAndroidDelegate {
      */
     @CalledByNative
     public void removeView(View anchorView) {
-        ViewGroup containerView = getContainerView();
+        ViewGroup containerView = getContainerViewGroup();
         if (containerView == null) return;
         containerView.removeView(anchorView);
     }
@@ -140,7 +140,7 @@ public class ViewAndroidDelegate {
     @CalledByNative
     public void setViewPosition(View anchorView, float x, float y, float width, float height,
             int leftMargin, int topMargin) {
-        ViewGroup containerView = getContainerView();
+        ViewGroup containerView = getContainerViewGroup();
         if (containerView == null) return;
         assert anchorView.getParent() == containerView;
 
@@ -175,7 +175,7 @@ public class ViewAndroidDelegate {
     private boolean startDragAndDrop(String text, Bitmap shadowImage) {
         if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.M) return false;
 
-        ViewGroup containerView = getContainerView();
+        ViewGroup containerView = getContainerViewGroup();
         if (containerView == null) return false;
 
         ImageView imageView = new ImageView(containerView.getContext());
@@ -192,7 +192,7 @@ public class ViewAndroidDelegate {
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
             PointerIcon icon =
                     ApiHelperForN.createPointerIcon(customCursorBitmap, hotspotX, hotspotY);
-            ApiHelperForN.setPointerIcon(getContainerView(), icon);
+            ApiHelperForN.setPointerIcon(getContainerViewGroup(), icon);
         }
     }
 
@@ -322,7 +322,7 @@ public class ViewAndroidDelegate {
                 assert false : "onCursorChangedToCustom must be called instead";
                 break;
         }
-        ViewGroup containerView = getContainerView();
+        ViewGroup containerView = getContainerViewGroup();
         PointerIcon icon = PointerIcon.getSystemIcon(containerView.getContext(), pointerIconType);
         ApiHelperForN.setPointerIcon(containerView, icon);
     }
@@ -363,10 +363,19 @@ public class ViewAndroidDelegate {
     }
 
     /**
+     * While ViewAndroidDelegate takes a ViewGroup, and internally adds Views to it, all other
+     * consumers should *not* be manipulating child Views. This is particularly important as the
+     * container view is usually ContentView, and ContentView only supports children directly added
+     * by this class. See ContentView for details on this.
+     *
      * @return container view that the anchor views are added to. May be null.
      */
     @CalledByNative
-    public final ViewGroup getContainerView() {
+    public final View getContainerView() {
+        return mContainerView;
+    }
+
+    protected final ViewGroup getContainerViewGroup() {
         return mContainerView;
     }
 
@@ -375,7 +384,7 @@ public class ViewAndroidDelegate {
      */
     @CalledByNative
     private int getXLocationOfContainerViewInWindow() {
-        ViewGroup container = getContainerView();
+        View container = getContainerView();
         if (container == null) return 0;
 
         container.getLocationInWindow(mTemporaryContainerLocation);
@@ -387,7 +396,7 @@ public class ViewAndroidDelegate {
      */
     @CalledByNative
     private int getYLocationOfContainerViewInWindow() {
-        ViewGroup container = getContainerView();
+        View container = getContainerView();
         if (container == null) return 0;
 
         container.getLocationInWindow(mTemporaryContainerLocation);
@@ -399,7 +408,7 @@ public class ViewAndroidDelegate {
      */
     @CalledByNative
     private int getXLocationOnScreen() {
-        ViewGroup container = getContainerView();
+        View container = getContainerView();
         if (container == null) return 0;
 
         container.getLocationOnScreen(mTemporaryContainerLocation);
@@ -411,7 +420,7 @@ public class ViewAndroidDelegate {
      */
     @CalledByNative
     private int getYLocationOnScreen() {
-        ViewGroup container = getContainerView();
+        View container = getContainerView();
         if (container == null) return 0;
 
         container.getLocationOnScreen(mTemporaryContainerLocation);
@@ -420,26 +429,26 @@ public class ViewAndroidDelegate {
 
     @CalledByNative
     private void requestDisallowInterceptTouchEvent() {
-        ViewGroup container = getContainerView();
+        ViewGroup container = getContainerViewGroup();
         if (container != null) container.requestDisallowInterceptTouchEvent(true);
     }
 
     @CalledByNative
     @TargetApi(Build.VERSION_CODES.LOLLIPOP)
     private void requestUnbufferedDispatch(MotionEvent event) {
-        ViewGroup container = getContainerView();
+        ViewGroup container = getContainerViewGroup();
         if (container != null) container.requestUnbufferedDispatch(event);
     }
 
     @CalledByNative
     private boolean hasFocus() {
-        ViewGroup containerView = getContainerView();
+        View containerView = getContainerView();
         return containerView == null ? false : ViewUtils.hasFocus(containerView);
     }
 
     @CalledByNative
     private void requestFocus() {
-        ViewGroup containerView = getContainerView();
+        View containerView = getContainerViewGroup();
         if (containerView != null) ViewUtils.requestFocus(containerView);
     }