Revert "Android: Fix a bug of text selection not working in SERP"

This reverts commit 6b78be80.

Reason for revert: Broke web search from text selection popup

Original change's description:
> Android: Fix a bug of text selection not working in SERP
> 
> ChromeActionModeHandler adds an observer for init-web-contents event
> of TabWebContentsObserver for each tab in order to register
> ChromeActionModeCallback to its WebContents, therefore enabling
> floating/toolbar action mode menu.
> 
> For newly created tab, there was a bug this registration of the callback
> not working (ok for the tab already created and swapping into
> foreground). It's because init-web-contents event occurs before
> adding an observer(mInitWebContentsObserver) to TabWebContentsObserver,
> missing the chance to register the callback.
> 
> This CL manually triggers the mInitWebContentsObserver for all the newly
> created tab to fix the bug. Previously it was applied to only the very
> first tab being added, but now extended to all those the newly created
> and added.
> 
> Bug: 1067524
> Change-Id: I511f15363e1e77f13347c5358aaf6eb3874a1a7b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2136569
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Theresa  <twellington@chromium.org>
> Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#758641}

TBR=dtrainor@chromium.org,twellington@chromium.org,jinsukkim@chromium.org

Change-Id: I39041b67d4d145f1b287ec1c221e3bd5293b637b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1067524
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2149752Reviewed-by: Theresa  <twellington@chromium.org>
Commit-Queue: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759032}

chrome/android/chrome_test_java_sources.gni

@@ -8,7 +8,6 @@
 chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/ActivityTabProviderTest.java",
   "javatests/src/org/chromium/chrome/browser/AudioTest.java",
-  "javatests/src/org/chromium/chrome/browser/ChromeActionModeHandlerTest.java",
   "javatests/src/org/chromium/chrome/browser/ChromeActivityTest.java",
   "javatests/src/org/chromium/chrome/browser/ChromeBackgroundServiceTest.java",
   "javatests/src/org/chromium/chrome/browser/ContentViewFocusTest.java",

chrome/android/java/src/org/chromium/chrome/browser/ChromeActionModeHandler.java

@@ -45,12 +45,15 @@ import java.util.Set;
  */
 public class ChromeActionModeHandler {
     /** Observes the active WebContents being initialized into a Tab. */
-    private final Callback<WebContents> mInitWebContentsObserver;
+    private final Consumer<WebContents> mInitWebContentsObserver;
 
     private final ActivityTabProvider.ActivityTabTabObserver mActivityTabTabObserver;
 
     private Tab mActiveTab;
 
+    /** Set to {@code true} once the first Tab content is changed. */
+    private boolean mContentChanged;
+
     /**
      * @param activityTabProvider {@link ActivityTabProvider} instance.
      * @param actionBarObserver observer called when the contextual action bar's visibility
@@ -81,6 +84,19 @@ public class ChromeActionModeHandler {
                         TabWebContentsObserver.from(tab).addInitWebContentsObserver(
                                 mInitWebContentsObserver);
                         mActiveTab = tab;
+
+                        // For the very first tab being observed, we miss mInitWebContentsObserver
+                        // because TabObserver.onContentChanged ->
+                        // TabWebContentsObserver.initWebContents occurs before this activity tab
+                        // observer is ready. Manually triggers it here.
+                        if (!mContentChanged && tab.getWebContents() != null) {
+                            mInitWebContentsObserver.accept(tab.getWebContents());
+                        }
+                    }
+
+                    @Override
+                    public void onContentChanged(Tab tab) {
+                        mContentChanged = true;
                     }
                 };
     }

chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java

@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser.tab;
 
-import android.os.Handler;
 import android.view.View;
 
 import androidx.annotation.IntDef;
@@ -13,10 +12,9 @@ import androidx.annotation.VisibleForTesting;
 import org.chromium.base.ActivityState;
 import org.chromium.base.ApplicationState;
 import org.chromium.base.ApplicationStatus;
-import org.chromium.base.Callback;
+import org.chromium.base.Consumer;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
-import org.chromium.base.ObserverList;
 import org.chromium.base.ObserverList.RewindableIterator;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.browser.AppHooks;
@@ -34,6 +32,8 @@ import org.chromium.content_public.browser.WebContentsObserver;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * WebContentsObserver used by Tab.
@@ -73,8 +73,7 @@ public class TabWebContentsObserver extends TabWebContentsUserData {
     }
 
     private final TabImpl mTab;
-    private final ObserverList<Callback<WebContents>> mInitObservers = new ObserverList<>();
-    private final Handler mHandler = new Handler();
+    private final List<Consumer<WebContents>> mInitObservers = new ArrayList<>();
     private WebContentsObserver mObserver;
 
     public static TabWebContentsObserver from(Tab tab) {
@@ -98,22 +97,16 @@ public class TabWebContentsObserver extends TabWebContentsUserData {
 
     /**
      * Adds an observer triggered when |initWebContents| is invoked.
-     * <p>A newly created tab adding this observer misses the event because
-     * |TabObserver.onContentChanged| -&gt; |TabWebContentsObserver.initWebContents|
-     * occurs before the observer is added. Manually trigger it here.
-     * @param observer Observer to add.
      */
-    public void addInitWebContentsObserver(Callback<WebContents> observer) {
-        if (mInitObservers.addObserver(observer) && mTab.getWebContents() != null) {
-            observer.onResult(mTab.getWebContents());
-        }
+    public void addInitWebContentsObserver(Consumer<WebContents> initObserver) {
+        mInitObservers.add(initObserver);
     }
 
     /**
      * Remove the InitWebContents observer from the list.
      */
-    public void removeInitWebContentsObserver(Callback<WebContents> observer) {
-        mInitObservers.removeObserver(observer);
+    public void removeInitWebContentsObserver(Consumer<WebContents> initObserver) {
+        mInitObservers.remove(initObserver);
     }
 
     @Override
@@ -129,7 +122,7 @@ public class TabWebContentsObserver extends TabWebContentsUserData {
         // is not the default behavior for embedded web views.
         WebContentsAccessibility.fromWebContents(webContents).setShouldFocusOnPageLoad(true);
 
-        for (Callback<WebContents> callback : mInitObservers) callback.onResult(webContents);
+        for (Consumer<WebContents> consumer : mInitObservers) consumer.accept(webContents);
     }
 
     @Override

chrome/android/javatests/src/org/chromium/chrome/browser/ChromeActionModeHandlerTest.java

-// Copyright 2020 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package org.chromium.chrome.browser;
-
-import android.support.test.InstrumentationRegistry;
-import android.support.test.filters.SmallTest;
-
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-
-import org.chromium.base.test.util.CommandLineFlags;
-import org.chromium.chrome.browser.flags.ChromeSwitches;
-import org.chromium.chrome.browser.tab.Tab;
-import org.chromium.chrome.browser.tab.TabLaunchType;
-import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
-import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
-import org.chromium.components.embedder_support.util.UrlConstants;
-import org.chromium.content_public.browser.LoadUrlParams;
-import org.chromium.content_public.browser.test.util.TestThreadUtils;
-import org.chromium.content_public.browser.test.util.WebContentsUtils;
-import org.chromium.content_public.common.ContentUrlConstants;
-import org.chromium.net.test.EmbeddedTestServer;
-
-/**
- * Tests {@link ChromeActionModeHandler} operation.
- */
-@RunWith(ChromeJUnit4ClassRunner.class)
-@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
-public class ChromeActionModeHandlerTest {
-    @Rule
-    public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
-
-    private void assertActionModeIsReady() {
-        // clang-format off
-        TestThreadUtils.runOnUiThreadBlocking(
-                () -> Assert.assertTrue(WebContentsUtils.isActionModeSupported(
-                        mActivityTestRule.getWebContents())));
-        // clang-format on
-    }
-
-    @Test
-    @SmallTest
-    public void testActionModeSetForNewTab() {
-        EmbeddedTestServer testServer = EmbeddedTestServer.createAndStartServer(
-                InstrumentationRegistry.getInstrumentation().getContext());
-
-        mActivityTestRule.startMainActivityWithURL(UrlConstants.NTP_URL);
-        mActivityTestRule.loadUrl(ContentUrlConstants.ABOUT_BLANK_DISPLAY_URL);
-        assertActionModeIsReady();
-
-        LoadUrlParams urlParams = new LoadUrlParams(ContentUrlConstants.ABOUT_BLANK_DISPLAY_URL);
-        Tab tab = mActivityTestRule.getActivity().getActivityTabProvider().get();
-
-        // Assert that a new tab has an action mode callback set as expected.
-        // clang-format off
-        TestThreadUtils.runOnUiThreadBlockingNoException(
-                () -> mActivityTestRule.getActivity().getTabModelSelector().openNewTab(
-                        urlParams, TabLaunchType.FROM_LONGPRESS_FOREGROUND, tab, true));
-        // clang-format on
-        assertActionModeIsReady();
-        testServer.stopAndDestroyServer();
-    }
-}

content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java

@@ -352,8 +352,7 @@ public class SelectionPopupControllerImpl extends ActionModeCallbackHelper
     }
 
     // True if action mode is initialized to a working (not a no-op) mode.
-    @VisibleForTesting
-    public boolean isActionModeSupported() {
+    private boolean isActionModeSupported() {
         return mCallback != EMPTY_CALLBACK;
     }
 

content/public/test/android/javatests/src/org/chromium/content_public/browser/test/util/WebContentsUtils.java

@@ -122,18 +122,6 @@ public class WebContentsUtils {
         return SelectionPopupControllerImpl.createForTesting(webContents);
     }
 
-    /**
-     * Checks if the given WebContents has a valid {@link ActionMode.Callback} set in place.
-     * @return {@code true} if WebContents (its SelectionPopupController) has a valid
-     *         action mode callback object.
-     */
-    public static boolean isActionModeSupported(WebContents webContents) {
-        SelectionPopupControllerImpl controller =
-                ((SelectionPopupControllerImpl) SelectionPopupController.fromWebContents(
-                        webContents));
-        return controller.isActionModeSupported();
-    }
-
     private static native void nativeReportAllFrameSubmissions(
             WebContents webContents, boolean enabled);
     private static native RenderFrameHost nativeGetFocusedFrame(WebContents webContents);