From 540523cc819044acc09453ba587864bec52bb30c Mon Sep 17 00:00:00 2001 From: Artem Samoilenko Date: Thu, 22 Feb 2024 14:39:56 -0500 Subject: [PATCH] [Android] Add Token-based ID support for Tab Groups Chromium change: https://source.chromium.org/chromium/chromium/src/+/4dfe0238c9391e5b9a4284072e7d07926171215b [Tab Groups] add Token-based ID support behind flag Add and manage Token-based TabGroupIds for Tabs in TabGroupModelFilter. RootId must be the TabId of one of the tabs in the group. This is unstable because if the Tab with the TabId corresponding to the RootId of the group is removed from the group the group must elect a new TabId from among its tabs to be the new RootId. This leads to issues with managing the state of a tab group over time. Under the RootId paradigm every tab was associated with a TabGroup object and only tab groups with > 1 tab were considered "real" tab groups. In an effort to support tab groups with a single tab it is necessary to come up with a new approach. This CL introduces TabGroupId. This ID is stable in that it is tied to the group the tabs are in and can only be destroyed if all tabs are removed from the tab group OR two groups with TabGroupIds are merged resulting in one of the two TabGroupIds going away. Tabs only have a TabGroupId if they are associated with a "real" tab group of size 1 or larger. Otherwise this field will be null. This means that TabGroup objects will only contain tabs with a TabGroupId if the group is > size 1 OR that single tab is to be treated as a real tab group. If the flag AndroidTabGroupStableIds is enabled then after tab state initialization all tabs that are part of "real" tab groups under the old definition will be allocated TabGroupIds in a 1:1 manner with old RootIds. When the flag is disabled the TabGroupIds will all be reset instead. Logic for maintaining TabGroupIds is as follows: * A Tab Group Id is allocated when: * A single tab is turned into a single tab group. * A single tab is merged with another single tab to form a group. * A list of tabs is merged to a single tab to form a group. If a group is merged to a single tab its Tab Group Id is preserved. * Tab Group Ids are removed from tabs when: * Ungrouping a tab from a group. * Tab Group Ids are lost entirely when: * Merging two tab groups both with tab group ids (only one ID will continue to exist). * Removing the last tab from a group. * Deleting an entire tab group. Bug: b/41496693 --- android/java/proguard.flags | 3 +- .../chromium/chrome/browser/BytecodeTest.java | 30 +++++++++++++++--- .../tab_groups/BraveTabGroupModelFilter.java | 31 ++++++++++++------- .../BraveTabGroupModelFilterClassAdapter.java | 2 +- 4 files changed, 47 insertions(+), 19 deletions(-) diff --git a/android/java/proguard.flags b/android/java/proguard.flags index 697512f8e27..1777b229ee6 100644 --- a/android/java/proguard.flags +++ b/android/java/proguard.flags @@ -53,5 +53,6 @@ -keep,allowobfuscation,allowshrinking class com.google.gson.reflect.TypeToken -keep class org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter { - *** getParentId(...); + *** getParentIds(...); + *** getOrCreateTabGroupId(...); } diff --git a/android/javatests/org/chromium/chrome/browser/BytecodeTest.java b/android/javatests/org/chromium/chrome/browser/BytecodeTest.java index 9c175f56df9..527d20e55d7 100644 --- a/android/javatests/org/chromium/chrome/browser/BytecodeTest.java +++ b/android/javatests/org/chromium/chrome/browser/BytecodeTest.java @@ -13,6 +13,7 @@ import android.content.res.Resources; import android.os.Bundle; import android.os.Handler; import android.util.AttributeSet; +import android.util.Pair; import android.view.ActionMode; import android.view.View; import android.view.ViewGroup; @@ -29,6 +30,7 @@ import org.junit.runner.RunWith; import org.chromium.base.Callback; import org.chromium.base.FeatureMap; +import org.chromium.base.Token; import org.chromium.base.jank_tracker.JankTracker; import org.chromium.base.shared_preferences.PreferenceKeyRegistry; import org.chromium.base.shared_preferences.SharedPreferencesManager; @@ -695,11 +697,28 @@ public class BytecodeTest { Assert.assertTrue(methodExists("org/chromium/components/browser_ui/site_settings/Website", "setContentSetting", true, void.class, BrowserContextHandle.class, int.class, int.class)); - Assert.assertTrue(methodExists("org/chromium/chrome/browser/tab/TabHelpers", - "initTabHelpers", true, void.class, Tab.class, Tab.class)); Assert.assertTrue( - methodExists("org/chromium/chrome/browser/tasks/tab_groups/TabGroupModelFilter", - "getParentId", true, int.class, Tab.class)); + methodExists( + "org/chromium/chrome/browser/tab/TabHelpers", + "initTabHelpers", + true, + void.class, + Tab.class, + Tab.class)); + Assert.assertTrue( + methodExists( + "org/chromium/chrome/browser/tasks/tab_groups/TabGroupModelFilter", + "getParentIds", + true, + Pair.class, + Tab.class)); + Assert.assertTrue( + methodExists( + "org/chromium/chrome/browser/tasks/tab_groups/TabGroupModelFilter", + "getOrCreateTabGroupId", + true, + Token.class, + Tab.class)); Assert.assertTrue( methodExists( @@ -873,7 +892,8 @@ public class BytecodeTest { Supplier.class, HomeSurfaceTracker.class, ObservableSupplier.class, - ObservableSupplier.class)); + ObservableSupplier.class, + OneshotSupplier.class)); Assert.assertTrue( constructorsMatch( "org/chromium/chrome/browser/toolbar/top/TopToolbarCoordinator", diff --git a/browser/tab_group/java/src/org/chromium/chrome/browser/tasks/tab_groups/BraveTabGroupModelFilter.java b/browser/tab_group/java/src/org/chromium/chrome/browser/tasks/tab_groups/BraveTabGroupModelFilter.java index f2970172b35..5d94651dc4b 100644 --- a/browser/tab_group/java/src/org/chromium/chrome/browser/tasks/tab_groups/BraveTabGroupModelFilter.java +++ b/browser/tab_group/java/src/org/chromium/chrome/browser/tasks/tab_groups/BraveTabGroupModelFilter.java @@ -1,14 +1,16 @@ -/** - * Copyright (c) 2023 The Brave Authors. All rights reserved. +/* Copyright (c) 2023 The Brave Authors. All rights reserved. * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at https://mozilla.org/MPL/2.0/. - */ - + * You can obtain one at https://mozilla.org/MPL/2.0/. */ package org.chromium.chrome.browser.tasks.tab_groups; +import android.util.Pair; + +import androidx.annotation.NonNull; + import org.chromium.base.BravePreferenceKeys; import org.chromium.base.BraveReflectionUtil; +import org.chromium.base.Token; import org.chromium.chrome.browser.preferences.ChromeSharedPreferences; import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.TabLaunchType; @@ -30,10 +32,8 @@ public abstract class BraveTabGroupModelFilter extends TabModelFilter { super(tabModel); } - /** - * Call from {@link TabGroupModelFilter} will be redirected here via bytrcode. - */ - public int getParentId(Tab tab) { + /** Call from {@link TabGroupModelFilter} will be redirected here via bytrcode. */ + public Pair getParentIds(Tab tab) { if (linkClicked(tab.getLaunchType()) && ChromeSharedPreferences.getInstance() .readBoolean(BravePreferenceKeys.BRAVE_TAB_GROUPS_ENABLED, true) @@ -41,12 +41,13 @@ public abstract class BraveTabGroupModelFilter extends TabModelFilter { && !mIsResetting) { Tab parentTab = TabModelUtils.getTabById(getTabModel(), tab.getParentId()); if (parentTab != null) { - return parentTab.getRootId(); + return new Pair<>(parentTab.getRootId(), getOrCreateTabGroupId(parentTab)); } } // Otherwise just call parent. - return (int) BraveReflectionUtil.InvokeMethod( - TabGroupModelFilter.class, this, "getParentId", Tab.class, tab); + return (Pair) + BraveReflectionUtil.InvokeMethod( + TabGroupModelFilter.class, this, "getParentIds", Tab.class, tab); } /** @@ -55,4 +56,10 @@ public abstract class BraveTabGroupModelFilter extends TabModelFilter { private boolean linkClicked(@TabLaunchType int type) { return type == TabLaunchType.FROM_LINK || type == TabLaunchType.FROM_LONGPRESS_FOREGROUND; } + + private static Token getOrCreateTabGroupId(@NonNull Tab tab) { + return (Token) + BraveReflectionUtil.InvokeMethod( + TabGroupModelFilter.class, null, "getOrCreateTabGroupId", Tab.class, tab); + } } diff --git a/build/android/bytecode/java/org/brave/bytecode/BraveTabGroupModelFilterClassAdapter.java b/build/android/bytecode/java/org/brave/bytecode/BraveTabGroupModelFilterClassAdapter.java index b000a5d283c..bf62137eabb 100644 --- a/build/android/bytecode/java/org/brave/bytecode/BraveTabGroupModelFilterClassAdapter.java +++ b/build/android/bytecode/java/org/brave/bytecode/BraveTabGroupModelFilterClassAdapter.java @@ -21,6 +21,6 @@ public class BraveTabGroupModelFilterClassAdapter extends BraveClassVisitor { deleteField(sTabGroupModelFilterClassName, "mIsResetting"); changeMethodOwner( - sTabGroupModelFilterClassName, "getParentId", sBraveTabGroupModelFilterClassName); + sTabGroupModelFilterClassName, "getParentIds", sBraveTabGroupModelFilterClassName); } }