From 01a39dba3ee4135ed55d99083d37ed89b8699e82 Mon Sep 17 00:00:00 2001 From: samartnik Date: Fri, 19 Mar 2021 17:18:11 -0400 Subject: [PATCH] [Android] Fixes Tab Groups controls behaviour on tab closing --- android/java/apk_for_test.flags | 1 - .../browser/toolbar/BraveToolbarManager.java | 11 ----- ...crollingBottomViewResourceFrameLayout.java | 42 +++++++++---------- .../res/layout/bottom_control_container.xml | 3 +- .../chromium/chrome/browser/BytecodeTest.java | 3 -- .../bottom/BraveBottomControlsMediator.java | 28 ++++--------- ...aveBottomControlsMediatorClassAdapter.java | 3 -- 7 files changed, 31 insertions(+), 60 deletions(-) diff --git a/android/java/apk_for_test.flags b/android/java/apk_for_test.flags index db2514c561b..43f63876dac 100644 --- a/android/java/apk_for_test.flags +++ b/android/java/apk_for_test.flags @@ -175,7 +175,6 @@ *** mBottomControlsHeight; *** mModel; *** mBrowserControlsSizer; - *** updateCompositedViewVisibility(...); } -keep class org.chromium.chrome.browser.toolbar.bottom.BraveBottomControlsMediator { diff --git a/android/java/org/chromium/chrome/browser/toolbar/BraveToolbarManager.java b/android/java/org/chromium/chrome/browser/toolbar/BraveToolbarManager.java index a82285a5d60..eb4e61b1a6e 100644 --- a/android/java/org/chromium/chrome/browser/toolbar/BraveToolbarManager.java +++ b/android/java/org/chromium/chrome/browser/toolbar/BraveToolbarManager.java @@ -27,7 +27,6 @@ import org.chromium.chrome.browser.ActivityTabProvider; import org.chromium.chrome.browser.app.ChromeActivity; import org.chromium.chrome.browser.bookmarks.BookmarkBridge; import org.chromium.chrome.browser.browser_controls.BrowserControlsSizer; -import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider; import org.chromium.chrome.browser.compositor.CompositorViewHolder; import org.chromium.chrome.browser.compositor.Invalidator; import org.chromium.chrome.browser.compositor.layouts.LayoutManagerImpl; @@ -111,7 +110,6 @@ public class BraveToolbarManager extends ToolbarManager { private final Object mLock = new Object(); private boolean mBottomControlsEnabled; private BraveScrollingBottomViewResourceFrameLayout mBottomControls; - private final BrowserControlsStateProvider.Observer mBraveBrowserControlsObserver; public BraveToolbarManager(AppCompatActivity activity, BrowserControlsSizer controlsSizer, FullscreenManager fullscreenManager, ToolbarControlContainer controlContainer, @@ -168,15 +166,6 @@ public class BraveToolbarManager extends ToolbarManager { } }; HomepageManager.getInstance().addListener(mBraveHomepageStateListener); - - mBraveBrowserControlsObserver = new BrowserControlsStateProvider.Observer() { - @Override - public void onControlsOffsetChanged(int topOffset, int topControlsMinHeightOffset, - int bottomOffset, int bottomControlsMinHeightOffset, boolean needsAnimate) { - updateBottomToolbarVisibility(); - } - }; - mBrowserControlsSizer.addObserver(mBraveBrowserControlsObserver); } @Override diff --git a/android/java/org/chromium/chrome/browser/toolbar/bottom/BraveScrollingBottomViewResourceFrameLayout.java b/android/java/org/chromium/chrome/browser/toolbar/bottom/BraveScrollingBottomViewResourceFrameLayout.java index 3c0bfd63cdc..4fd5a43c0f9 100644 --- a/android/java/org/chromium/chrome/browser/toolbar/bottom/BraveScrollingBottomViewResourceFrameLayout.java +++ b/android/java/org/chromium/chrome/browser/toolbar/bottom/BraveScrollingBottomViewResourceFrameLayout.java @@ -23,14 +23,25 @@ public class BraveScrollingBottomViewResourceFrameLayout private SwipeGestureListener mSwipeGestureListener; private Supplier mBottomControlsCoordinatorSupplier; private final CallbackController mCallbackController; - private boolean mIsBottomToolbarVisible; - private boolean mIsTabGroupUiVisible; + View mBottomToolbar; + View mBottomContainerSlot; public BraveScrollingBottomViewResourceFrameLayout(Context context, AttributeSet attrs) { super(context, attrs); mCallbackController = new CallbackController(); } + @Override + protected void onFinishInflate() { + super.onFinishInflate(); + + mBottomToolbar = findViewById(R.id.bottom_toolbar); + assert mBottomToolbar != null : "Something has changed in upstream!"; + + mBottomContainerSlot = findViewById(R.id.bottom_container_slot); + assert mBottomContainerSlot != null : "Something has changed in upstream!"; + } + /** * Set the swipe handler for this view and set {@link #isClickable()} to true to allow motion * events to be intercepted by the view itself. @@ -61,21 +72,6 @@ public class BraveScrollingBottomViewResourceFrameLayout return handledEvent || super.onTouchEvent(event); } - private void updateBottomControlsVisibility() { - if (braveBottomControlsCoordinator() != null) { - View bottomToolbar = findViewById(R.id.bottom_toolbar); - assert (bottomToolbar != null); - if (bottomToolbar != null) { - bottomToolbar.setVisibility(mIsBottomToolbarVisible ? View.VISIBLE : View.GONE); - } - View bottomContainerSlot = findViewById(R.id.bottom_container_slot); - assert (bottomContainerSlot != null); - if (bottomContainerSlot != null) { - bottomContainerSlot.setVisibility(mIsTabGroupUiVisible ? View.VISIBLE : View.GONE); - } - } - } - public void setBottomControlsCoordinatorSupplier( Supplier bottomControlsCoordinatorSupplier) { if (mBottomControlsCoordinatorSupplier != null) { @@ -85,13 +81,17 @@ public class BraveScrollingBottomViewResourceFrameLayout mBottomControlsCoordinatorSupplier = bottomControlsCoordinatorSupplier; braveBottomControlsCoordinator().getBottomToolbarVisibleSupplier().addObserver( mCallbackController.makeCancelable((visible) -> { - mIsBottomToolbarVisible = visible; - updateBottomControlsVisibility(); + if (mBottomToolbar != null) { + mBottomToolbar.setVisibility(visible ? View.VISIBLE : View.GONE); + getResourceAdapter().dropCachedBitmap(); + } })); braveBottomControlsCoordinator().getTabGroupUiVisibleSupplier().addObserver( mCallbackController.makeCancelable((visible) -> { - mIsTabGroupUiVisible = visible; - updateBottomControlsVisibility(); + if (mBottomContainerSlot != null) { + mBottomContainerSlot.setVisibility(visible ? View.VISIBLE : View.GONE); + getResourceAdapter().dropCachedBitmap(); + } })); } diff --git a/android/java/res/layout/bottom_control_container.xml b/android/java/res/layout/bottom_control_container.xml index 0c2d5ebb2b0..73e880d531b 100644 --- a/android/java/res/layout/bottom_control_container.xml +++ b/android/java/res/layout/bottom_control_container.xml @@ -27,7 +27,8 @@ + android:id="@+id/bottom_container_slot" + android:visibility="gone" /> diff --git a/android/javatests/org/chromium/chrome/browser/BytecodeTest.java b/android/javatests/org/chromium/chrome/browser/BytecodeTest.java index 92baf38cf51..fa8af09f368 100644 --- a/android/javatests/org/chromium/chrome/browser/BytecodeTest.java +++ b/android/javatests/org/chromium/chrome/browser/BytecodeTest.java @@ -195,9 +195,6 @@ public class BytecodeTest { "showBookmarkBottomSheet", false, null)); Assert.assertTrue(methodExists("org/chromium/chrome/browser/bookmarks/BookmarkUtils", "addBookmarkAndShowSnackbar", false, null)); - Assert.assertTrue( - methodExists("org/chromium/chrome/browser/toolbar/bottom/BottomControlsMediator", - "updateCompositedViewVisibility", false, null)); } @Test diff --git a/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/bottom/BraveBottomControlsMediator.java b/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/bottom/BraveBottomControlsMediator.java index 0cf965d0747..ff15037e016 100644 --- a/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/bottom/BraveBottomControlsMediator.java +++ b/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/bottom/BraveBottomControlsMediator.java @@ -40,17 +40,17 @@ class BraveBottomControlsMediator extends BottomControlsMediator { @Override public void setBottomControlsVisible(boolean visible) { - mTabGroupUiVisibleSupplier.set(visible); - updateBottomControlsHeight(); + updateBottomControlsHeight(mBottomToolbarVisibleSupplier.get() && visible); // We should keep it visible if bottom toolbar is visible. super.setBottomControlsVisible(mBottomToolbarVisibleSupplier.get() || visible); + mTabGroupUiVisibleSupplier.set(visible); } public void setBottomToolbarVisible(boolean visible) { - mBottomToolbarVisibleSupplier.set(visible); - updateBottomControlsHeight(); + updateBottomControlsHeight(mTabGroupUiVisibleSupplier.get() && visible); // We should keep it visible if tag group UI is visible. super.setBottomControlsVisible(mTabGroupUiVisibleSupplier.get() || visible); + mBottomToolbarVisibleSupplier.set(visible); } public ObservableSupplierImpl getBottomToolbarVisibleSupplier() { @@ -61,21 +61,9 @@ class BraveBottomControlsMediator extends BottomControlsMediator { return mTabGroupUiVisibleSupplier; } - private void updateBottomControlsHeight() { - if (mBottomToolbarVisibleSupplier.get() && mTabGroupUiVisibleSupplier.get()) { - // Double the height if both bottom controls are visible - mBottomControlsHeight = mBottomControlsHeightDouble; - } else { - mBottomControlsHeight = mBottomControlsHeightSingle; - } - } - - public void updateCompositedViewVisibility() { - final boolean isCompositedViewVisible = isCompositedViewVisible(); - mModel.set(BottomControlsProperties.COMPOSITED_VIEW_VISIBLE, isCompositedViewVisible); - mBrowserControlsSizer.setBottomControlsHeight(isCompositedViewVisible - ? mBottomControlsHeight - : (mBottomControlsHeight - mBottomControlsHeightSingle), - mBrowserControlsSizer.getBottomControlsMinHeight()); + private void updateBottomControlsHeight(boolean bothBottomControlsVisible) { + // Double the height if both bottom controls are visible + mBottomControlsHeight = bothBottomControlsVisible ? mBottomControlsHeightDouble + : mBottomControlsHeightSingle; } } diff --git a/build/android/bytecode/java/org/brave/bytecode/BraveBottomControlsMediatorClassAdapter.java b/build/android/bytecode/java/org/brave/bytecode/BraveBottomControlsMediatorClassAdapter.java index 02f0c997c0e..32716a2bd4e 100644 --- a/build/android/bytecode/java/org/brave/bytecode/BraveBottomControlsMediatorClassAdapter.java +++ b/build/android/bytecode/java/org/brave/bytecode/BraveBottomControlsMediatorClassAdapter.java @@ -27,8 +27,5 @@ public class BraveBottomControlsMediatorClassAdapter extends BraveClassVisitor { deleteField(sBraveBottomControlsMediatorClassName, "mBrowserControlsSizer"); makeProtectedField(sBottomControlsMediatorClassName, "mBrowserControlsSizer"); - - addMethodAnnotation(sBraveBottomControlsMediatorClassName, "updateCompositedViewVisibility", - "Ljava/lang/Override;"); } }