From 9e3dedd19349d812d7dba808f07ebee91302623e Mon Sep 17 00:00:00 2001
From: Jinsuk Kim <[email protected]>
Date: Tue, 07 Jan 2025 09:44:22 -0800
Subject: [PATCH] [Android] Fix floating action bar/top control overlapping bug

In order to avoid the floating bar and the top control overlapping
with each other, this CL ensures that the action bar will be drawn
above the selected text only if there is enough space.

Bug: 329476341
Change-Id: I7095863a845db2aa38a4d9d93efde2f2075e82a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6148920
Code-Coverage: [email protected] <[email protected]>
Reviewed-by: Theresa Sullivan <[email protected]>
Commit-Queue: Jinsuk Kim <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1403053}
---

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActionModeHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActionModeHandler.java
index b658d84..d01c14ba0 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActionModeHandler.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActionModeHandler.java
@@ -23,6 +23,7 @@
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
 import org.chromium.chrome.browser.firstrun.FirstRunStatus;
 import org.chromium.chrome.browser.locale.LocaleManager;
 import org.chromium.chrome.browser.readaloud.ReadAloudController;
@@ -60,12 +61,15 @@
      * @param searchCallback Callback to run when search action is selected in the action mode.
      * @param shareDelegateSupplier The {@link Supplier} of the {@link ShareDelegate} that will be
      *     notified when a share action is performed.
+     * @param controlsState Provides browser controls visibility state.
+     * @param readAloudControllerSupplier Supplies {@link ReadAloudController}.
      */
     public ChromeActionModeHandler(
             ActivityTabProvider activityTabProvider,
             Callback<String> searchCallback,
             boolean showWebSearch,
             Supplier<ShareDelegate> shareDelegateSupplier,
+            BrowserControlsStateProvider controlsState,
             Supplier<ReadAloudController> readAloudControllerSupplier) {
         mInitWebContentsObserver =
                 (webContents) -> {
@@ -78,6 +82,7 @@
                                     searchCallback,
                                     showWebSearch,
                                     shareDelegateSupplier,
+                                    controlsState,
                                     readAloudControllerSupplier));
                     spc.setDropdownMenuDelegate(new ChromeSelectionDropdownMenuDelegate());
                 };
@@ -124,6 +129,7 @@
         private final boolean mShowWebSearch;
         private final Supplier<ShareDelegate> mShareDelegateSupplier;
         private final Supplier<ReadAloudController> mReadAloudControllerSupplier;
+        private final BrowserControlsStateProvider mControlsState;
 
         // Used for recording UMA histograms.
         private long mContextMenuStartTime;
@@ -134,12 +140,14 @@
                 Callback<String> searchCallback,
                 boolean showWebSearch,
                 Supplier<ShareDelegate> shareDelegateSupplier,
+                BrowserControlsStateProvider controlsState,
                 Supplier<ReadAloudController> readAloudControllerSupplier) {
             mTab = tab;
             mHelper = getActionModeCallbackHelper(webContents);
             mShowWebSearch = showWebSearch;
             mSearchCallback = searchCallback;
             mShareDelegateSupplier = shareDelegateSupplier;
+            mControlsState = controlsState;
             mReadAloudControllerSupplier = readAloudControllerSupplier;
         }
 
@@ -287,6 +295,17 @@
         @Override
         public void onGetContentRect(ActionMode mode, View view, Rect outRect) {
             mHelper.onGetContentRect(mode, view, outRect);
+            boolean controlsVisible = mControlsState.getBrowserControlHiddenRatio() < 1.f;
+            int controlsHeight = mControlsState.getTopControlsHeight();
+            if (controlsVisible && outRect.top < 2 * controlsHeight) {
+                // Make |outRect| taller to so the framework thinks there is not enough space
+                // above the selected text to place the floating action mode. This helps the action
+                // mode and the top controls avoid overlapping - the action mode will be positioned
+                // below the text.
+                // The right condition should be |outRect.top < controlsHeight + actionModeHeight|
+                // but we do not know |actionModeHeight|. Assume actionModeHeight ~= controlsHeight.
+                outRect.top -= controlsHeight;
+            }
         }
 
         private Set<String> getPackageNames(List<ResolveInfo> list) {
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java b/chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java
index 29173959..08cc31e 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java
@@ -843,6 +843,7 @@
                 },
                 showWebSearchInActionMode(),
                 mShareDelegateSupplier,
+                mBrowserControlsManager,
                 mReadAloudControllerSupplier);
 
         mCaptureController =
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ChromeActionModeHandlerUnitTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/ChromeActionModeHandlerUnitTest.java
index b98108e..a08755e 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/ChromeActionModeHandlerUnitTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/ChromeActionModeHandlerUnitTest.java
@@ -14,6 +14,7 @@
 import android.content.Intent;
 import android.content.pm.ActivityInfo;
 import android.content.pm.ResolveInfo;
+import android.graphics.Rect;
 import android.view.ActionMode;
 import android.view.Menu;
 import android.view.MenuItem;
@@ -35,6 +36,7 @@
 import org.chromium.base.Callback;
 import org.chromium.base.PackageManagerUtils;
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
 import org.chromium.chrome.browser.firstrun.FirstRunStatus;
 import org.chromium.chrome.browser.locale.LocaleManager;
 import org.chromium.chrome.browser.locale.LocaleManagerDelegate;
@@ -62,6 +64,7 @@
     @Mock private Menu mMenu;
     @Mock private ShareDelegate mShareDelegate;
     @Mock private ReadAloudController mReadAloudController;
+    @Mock private BrowserControlsStateProvider mControlsState;
 
     private class TestChromeActionModeCallback
             extends ChromeActionModeHandler.ChromeActionModeCallback {
@@ -72,6 +75,7 @@
                     urlParams -> {},
                     true,
                     () -> mShareDelegate,
+                    mControlsState,
                     () -> mReadAloudController);
         }
 
@@ -237,6 +241,51 @@
         verify(mReadAloudController).maybePauseForOutgoingIntent(eq(intent));
     }
 
+    @Test
+    public void testAvoidOverlapWithTopControls() {
+        final int topControlsHeight = 150;
+        final int height = 80;
+        Mockito.when(mControlsState.getTopControlsHeight()).thenReturn(topControlsHeight);
+
+        // Set up for the case where top controls are hidden.
+        Mockito.when(mControlsState.getBrowserControlHiddenRatio()).thenReturn(1.f);
+
+        // If there's enough space between the selected text and the top of the content view for
+        // action mode, the content rect is left untouched.
+        int top = topControlsHeight * 3;
+        Rect outRect = new Rect(20, top, 500, top + height);
+        mActionModeCallback.onGetContentRect(mActionMode, null, outRect);
+        Assert.assertEquals(top, outRect.top);
+        Assert.assertEquals(height, outRect.height());
+
+        // Not enough space for action mode to fit in. The content rect is left untouched.
+        top = topControlsHeight;
+        outRect = new Rect(20, top, 500, top + height);
+        mActionModeCallback.onGetContentRect(mActionMode, null, outRect);
+        Assert.assertEquals(top, outRect.top);
+        Assert.assertEquals(height, outRect.height());
+
+        // Set up for the case where top controls are visible.
+        Mockito.when(mControlsState.getBrowserControlHiddenRatio()).thenReturn(0.f);
+
+        // We have enough space for action mode to fit in. The content rect is left untouched.
+        top = topControlsHeight * 3;
+        outRect = new Rect(20, top, 500, top + height);
+        mActionModeCallback.onGetContentRect(mActionMode, null, outRect);
+        Assert.assertEquals(top, outRect.top);
+        Assert.assertEquals(height, outRect.height());
+
+        // Not enough space for action mode to fit in. Verify that |onGetContentRect| bloated
+        // the content rect (top got taller) so action mode won't fit between the top controls
+        // and the selected text, therefore will be positioned below the text. This helps action
+        // mode avoid overlapping top controls.
+        top = topControlsHeight;
+        outRect = new Rect(20, top, 500, top + height);
+        mActionModeCallback.onGetContentRect(mActionMode, null, outRect);
+        Assert.assertEquals(top - topControlsHeight, outRect.top);
+        Assert.assertEquals(topControlsHeight + height, outRect.height());
+    }
+
     private ResolveInfo createResolveInfo(String packageName) {
         ResolveInfo resolveInfo = new ResolveInfo();
         ActivityInfo activityInfo = new ActivityInfo();