Skip to content

Commit eac7030

Browse files
AlexeyBarabashsamartnik
AlexeyBarabash
and
samartnik
committed
[Android] Wait for native initialization when creating bottom strip (#28728)
* [cr137] [Android] Wait for native initialization when creating bottom strip Chromium change: https://chromium.googlesource.com/chromium/src/+/ea006a988d2a36db0eb25801e1c02e9c6e9f5b86 [Tab Group UI] Wait for native initialization when creating bottom strip Wait for native initialization to complete before initializing the bottom tab strip. Otherwise it will try to initialize pre-native and crash. This appears to have regressed in M134/M135 likely due to some lifecycle change for the BottomControlsCoordinator. Fixed: 407305088 * Suppressed warning around ContextUtils.getAppSharedPreferences() --------- Co-authored-by: Artem Samoilenko <artem@brave.com>
1 parent 574f4dc commit eac7030

File tree

5 files changed

+87
-64
lines changed

5 files changed

+87
-64
lines changed

‎android/java/apk_for_test.flags

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@
154154
*** mReadAloudControllerSupplier;
155155
*** mTopUiThemeColorProvider;
156156
*** mCurrentOrientation;
157+
*** mInitializedWithNative;
158+
*** mTabGroupUiOneshotSupplier;
157159
*** onOrientationChange(...);
158160
*** updateBookmarkButtonStatus(...);
159161
*** updateReloadState(...);

‎android/java/org/chromium/chrome/browser/toolbar/BraveToolbarManager.java

Lines changed: 69 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import org.chromium.base.supplier.ObservableSupplier;
2020
import org.chromium.base.supplier.ObservableSupplierImpl;
2121
import org.chromium.base.supplier.OneshotSupplier;
22-
import org.chromium.base.supplier.OneshotSupplierImpl;
2322
import org.chromium.base.supplier.Supplier;
2423
import org.chromium.chrome.R;
2524
import org.chromium.chrome.browser.ActivityTabProvider;
@@ -53,8 +52,7 @@
5352
import org.chromium.chrome.browser.tabmodel.TabClosureParams;
5453
import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
5554
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
56-
import org.chromium.chrome.browser.tasks.tab_management.TabGroupUi;
57-
import org.chromium.chrome.browser.tasks.tab_management.TabManagementDelegateProvider;
55+
import org.chromium.chrome.browser.tasks.tab_management.TabGroupUiOneshotSupplier;
5856
import org.chromium.chrome.browser.theme.BottomUiThemeColorProvider;
5957
import org.chromium.chrome.browser.theme.ThemeColorProvider;
6058
import org.chromium.chrome.browser.theme.TopUiThemeColorProvider;
@@ -112,14 +110,14 @@ public class BraveToolbarManager extends ToolbarManager {
112110
private TabCreatorManager mTabCreatorManager;
113111
private Supplier<ModalDialogManager> mModalDialogManagerSupplier;
114112
private TabObscuringHandler mTabObscuringHandler;
115-
private LayoutStateProvider.LayoutStateObserver mLayoutStateObserver;
116113
private LayoutStateProvider mLayoutStateProvider;
117114
private ObservableSupplier<ReadAloudController> mReadAloudControllerSupplier;
118115
private TopUiThemeColorProvider mTopUiThemeColorProvider;
119116
private int mCurrentOrientation;
117+
private boolean mInitializedWithNative;
118+
private @Nullable TabGroupUiOneshotSupplier mTabGroupUiOneshotSupplier;
120119

121120
// Own members.
122-
private TabGroupUi mTabGroupUi;
123121
private boolean mIsBraveBottomControlsVisible;
124122
private ObservableSupplier<Boolean> mOmniboxFocusStateSupplier;
125123
private OneshotSupplier<LayoutStateProvider> mLayoutStateProviderSupplier;
@@ -133,10 +131,10 @@ public class BraveToolbarManager extends ToolbarManager {
133131
private ObservableSupplier<EdgeToEdgeController> mEdgeToEdgeControllerSupplier;
134132
private ObservableSupplier<Profile> mProfileSupplier;
135133
private final BrowserControlsSizer mBrowserControlsSizer;
136-
private OneshotSupplierImpl<BottomControlsContentDelegate> mContentDelegateSupplier =
137-
new OneshotSupplierImpl<>();
138134
private final DataSharingTabManager mDataSharingTabManager;
139135
private ObservableSupplier<TabModelSelector> mTabModelSelectorSupplier;
136+
private LayoutStateProvider.LayoutStateObserver mLayoutStateObserver;
137+
private Runnable mOpenGridTabSwitcherHandler;
140138

141139
public BraveToolbarManager(
142140
AppCompatActivity activity,
@@ -279,29 +277,31 @@ public void enableBottomControls() {
279277
mIncognitoStateProvider,
280278
mActivity);
281279

282-
mTabGroupUi =
283-
TabManagementDelegateProvider.getDelegate()
284-
.createTabGroupUi(
285-
mActivity,
286-
mBottomControls.findViewById(R.id.bottom_container_slot),
287-
mBrowserControlsSizer,
288-
mScrimManager,
289-
mOmniboxFocusStateSupplier,
290-
mBottomSheetController,
291-
mDataSharingTabManager,
292-
mTabModelSelector,
293-
mTabContentManager,
294-
mTabCreatorManager,
295-
mLayoutStateProviderSupplier,
296-
mModalDialogManagerSupplier.get(),
297-
bottomUiThemeColorProvider);
298-
mContentDelegateSupplier.set(mTabGroupUi);
280+
mTabGroupUiOneshotSupplier =
281+
new TabGroupUiOneshotSupplier(
282+
mActivityTabProvider,
283+
mTabModelSelector,
284+
mActivity,
285+
mBottomControls.findViewById(R.id.bottom_container_slot),
286+
mBrowserControlsSizer,
287+
mScrimManager,
288+
mOmniboxFocusStateSupplier,
289+
mBottomSheetController,
290+
mDataSharingTabManager,
291+
mTabContentManager,
292+
mTabCreatorManager,
293+
mLayoutStateProviderSupplier,
294+
mModalDialogManagerSupplier.get(),
295+
bottomUiThemeColorProvider);
296+
var bottomControlsContentDelegateSupplier =
297+
(OneshotSupplier<BottomControlsContentDelegate>)
298+
((OneshotSupplier<? extends BottomControlsContentDelegate>)
299+
mTabGroupUiOneshotSupplier);
299300

300301
BrowserStateBrowserControlsVisibilityDelegate controlsVisibilityDelegate =
301302
mBrowserControlsSizer.getBrowserVisibilityDelegate();
302303
assert controlsVisibilityDelegate != null;
303-
304-
mBottomControlsCoordinatorSupplier.set(
304+
var bottomControlsCoordinator =
305305
new BraveBottomControlsCoordinator(
306306
mLayoutStateProviderSupplier,
307307
BottomTabSwitcherActionMenuCoordinator.createOnLongClickListener(
@@ -327,14 +327,49 @@ public void enableBottomControls() {
327327
mFullscreenManager,
328328
mEdgeToEdgeControllerSupplier,
329329
mBottomControls,
330-
mContentDelegateSupplier,
330+
bottomControlsContentDelegateSupplier,
331331
mTabObscuringHandler,
332332
mOverlayPanelVisibilitySupplier,
333333
getConstraintsProxy(),
334334
/* readAloudRestoringSupplier= */ () -> {
335335
final var readAloud = mReadAloudControllerSupplier.get();
336336
return readAloud != null && readAloud.isRestoringPlayer();
337-
}));
337+
});
338+
if (mInitializedWithNative) {
339+
Runnable closeAllTabsAction =
340+
() -> {
341+
mTabModelSelector
342+
.getModel(mIncognitoStateProvider.isIncognitoSelected())
343+
.getTabRemover()
344+
.closeTabs(TabClosureParams.closeAllTabs().build(), false);
345+
};
346+
347+
assert (mActivity instanceof ChromeActivity);
348+
OnClickListener wrappedNewTabClickHandler =
349+
v -> {
350+
recordNewTabClick();
351+
((ChromeActivity) mActivity)
352+
.getMenuOrKeyboardActionController()
353+
.onMenuOrKeyboardAction(
354+
mIncognitoStateProvider.isIncognitoSelected()
355+
? R.id.new_incognito_tab_menu_id
356+
: R.id.new_tab_menu_id,
357+
false);
358+
};
359+
360+
bottomControlsCoordinator.initializeWithNative(
361+
mActivity,
362+
mCompositorViewHolder.getResourceManager(),
363+
mCompositorViewHolder.getLayoutManager(),
364+
/*tabSwitcherListener*/ v -> mOpenGridTabSwitcherHandler.run(),
365+
/*newTabClickListener*/ wrappedNewTabClickHandler,
366+
mWindowAndroid,
367+
mTabModelSelector,
368+
mIncognitoStateProvider,
369+
mActivity.findViewById(R.id.control_container),
370+
closeAllTabsAction);
371+
}
372+
mBottomControlsCoordinatorSupplier.set(bottomControlsCoordinator);
338373
mBottomControls.setBottomControlsCoordinatorSupplier(
339374
mBottomControlsCoordinatorSupplier);
340375
updateBraveBottomControlsVisibility();
@@ -351,6 +386,10 @@ public void enableBottomControls() {
351386
// Also ToolbarManager.initializeWithNative calls
352387
// TopToolbarCoordinator.initializeWithNative where 3rd parameter is
353388
// `OnClickListener tabSwitcherClickHandler`. So it is a tabSwitcherClickHandler.
389+
//
390+
// Suppress to observe SharedPreferences, which is discouraged; use another messaging channel
391+
// instead.
392+
@SuppressWarnings("UseSharedPreferencesManagerFromChromeCheck")
354393
@Override
355394
public void initializeWithNative(
356395
@NonNull LayoutManagerImpl layoutManager,
@@ -370,43 +409,9 @@ public void initializeWithNative(
370409
archivedTabCountSupplier,
371410
tabModelNotificationDotSupplier);
372411

412+
mOpenGridTabSwitcherHandler = openGridTabSwitcherHandler;
413+
373414
if (isToolbarPhone() && BottomToolbarConfiguration.isBraveBottomControlsEnabled()) {
374-
enableBottomControls();
375-
Runnable closeAllTabsAction =
376-
() -> {
377-
mTabModelSelector
378-
.getModel(mIncognitoStateProvider.isIncognitoSelected())
379-
.getTabRemover()
380-
.closeTabs(TabClosureParams.closeAllTabs().build(), false);
381-
};
382-
383-
assert (mActivity instanceof ChromeActivity);
384-
OnClickListener wrappedNewTabClickHandler =
385-
v -> {
386-
recordNewTabClick();
387-
((ChromeActivity) mActivity)
388-
.getMenuOrKeyboardActionController()
389-
.onMenuOrKeyboardAction(
390-
mIncognitoStateProvider.isIncognitoSelected()
391-
? R.id.new_incognito_tab_menu_id
392-
: R.id.new_tab_menu_id,
393-
false);
394-
};
395-
396-
assert (mBottomControlsCoordinatorSupplier.get()
397-
instanceof BraveBottomControlsCoordinator);
398-
((BraveBottomControlsCoordinator) mBottomControlsCoordinatorSupplier.get())
399-
.initializeWithNative(
400-
mActivity,
401-
mCompositorViewHolder.getResourceManager(),
402-
mCompositorViewHolder.getLayoutManager(),
403-
/*tabSwitcherListener*/ v -> openGridTabSwitcherHandler.run(),
404-
/*newTabClickListener*/ wrappedNewTabClickHandler,
405-
mWindowAndroid,
406-
mTabModelSelector,
407-
mIncognitoStateProvider,
408-
mActivity.findViewById(R.id.control_container),
409-
closeAllTabsAction);
410415
mLocationBar.getContainerView().setAccessibilityTraversalBefore(R.id.bottom_toolbar);
411416
}
412417
}

‎android/java/org/chromium/chrome/browser/toolbar/bottom/BraveBottomControlsCoordinator.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ public void initializeWithNative(
115115
IncognitoStateProvider incognitoStateProvider,
116116
ViewGroup topToolbarRoot,
117117
Runnable closeAllTabsAction) {
118+
super.initializeWithNative();
119+
118120
if (BottomToolbarConfiguration.isBraveBottomControlsEnabled()) {
119121
mBottomToolbarCoordinator =
120122
new BottomToolbarCoordinator(

‎android/javatests/org/chromium/chrome/browser/BytecodeTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1971,6 +1971,14 @@ public void testFieldsExist() throws Exception {
19711971
fieldExists(
19721972
"org/chromium/chrome/browser/toolbar/ToolbarManager",
19731973
"mCurrentOrientation"));
1974+
Assert.assertTrue(
1975+
fieldExists(
1976+
"org/chromium/chrome/browser/toolbar/ToolbarManager",
1977+
"mInitializedWithNative"));
1978+
Assert.assertTrue(
1979+
fieldExists(
1980+
"org/chromium/chrome/browser/toolbar/ToolbarManager",
1981+
"mTabGroupUiOneshotSupplier"));
19741982
Assert.assertTrue(
19751983
fieldExists(
19761984
"org/chromium/chrome/browser/toolbar/top/TopToolbarCoordinator",

‎build/android/bytecode/java/org/brave/bytecode/BraveToolbarManagerClassAdapter.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ public BraveToolbarManagerClassAdapter(ClassVisitor visitor) {
9595
deleteField(sBraveToolbarManagerClassName, "mCurrentOrientation");
9696
makeProtectedField(sToolbarManagerClassName, "mCurrentOrientation");
9797

98+
deleteField(sBraveToolbarManagerClassName, "mInitializedWithNative");
99+
makeProtectedField(sToolbarManagerClassName, "mInitializedWithNative");
100+
101+
deleteField(sBraveToolbarManagerClassName, "mTabGroupUiOneshotSupplier");
102+
makeProtectedField(sToolbarManagerClassName, "mTabGroupUiOneshotSupplier");
103+
98104
makePublicMethod(sToolbarManagerClassName, "onOrientationChange");
99105
addMethodAnnotation(
100106
sBraveToolbarManagerClassName, "onOrientationChange", "Ljava/lang/Override;");

0 commit comments

Comments
 (0)