Skip to content

[Android] Handle synced tab groups when main toggle is off - #38679

Open
samartnik wants to merge 1 commit into
masterfrom
android_diasble_synced_tab_groups
Open

[Android] Handle synced tab groups when main toggle is off#38679
samartnik wants to merge 1 commit into
masterfrom
android_diasble_synced_tab_groups

Conversation

@samartnik

Copy link
Copy Markdown
Contributor

@samartnik samartnik added this to the 1.95.x - Nightly milestone Jul 31, 2026
@samartnik samartnik self-assigned this Jul 31, 2026
@samartnik
samartnik requested review from a team as code owners July 31, 2026 15:34
@samartnik samartnik added CI/skip-ios Do not run CI builds for iOS CI/skip-windows-x64 Do not run CI builds for Windows x64 CI/skip-macos-arm64 Do not run CI builds for macOS arm64 labels Jul 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

[puLL-Merge] - brave/brave-core@38679

Description

Reworks Android "auto-open synced tab groups" setting into "Show synced tab groups". Previously suppressed auto-opening when master switch off (BraveTabGroupSyncRemoteObserver). Now actively hides/shows synced tab groups based on both "Enable tab groups" master switch and "Show synced tab groups" pref (Pref.AUTO_OPEN_SYNCED_TAB_GROUPS).

New BraveTabGroupSyncControllerImpl subclasses upstream TabGroupSyncControllerImpl, observes pref + settings changes, and hides/opens synced groups accordingly. Switch decoupled from master switch (stays enabled/usable regardless). BraveSyncedTabGroupHelper holds visibility logic + open/hide operations. Removes BraveTabGroupSyncRemoteObserver and its sources.gni/patch mechanism, replaced by proper subclass wired via TabbedRootUiCoordinator rewrite.

Possible Issues

  • Semantic mismatch: UI labeled "Show synced tab groups" backed by Pref.AUTO_OPEN_SYNCED_TAB_GROUPS, which sync engine still treats as auto-open trigger for incoming groups. When master switch on, toggling "show" only affects auto-open of new arrivals, not existing group visibility (per areSyncedTabGroupsVisible returning true whenever tab groups enabled). Users may expect toggling off to hide already-open synced groups while tab groups enabled — it won't. Confirm intended.

  • hideSyncedTabGroups on non-active windows: mSyncBackendInitObserver.onInitialized calls hideSyncedTabGroups() in every window without active-window check. If multiple windows share same tab model/sync backend, redundant close calls possible. onSyncedTabGroupVisibilityChanged gates opening on active window but hiding runs everywhere—verify not double-closing.

  • showSyncedTabGroups restore on startup skipped: comment states startup only applies hiding to avoid reopening user-closed groups. But if user re-enables setting while window not active, and window later becomes active, no catch-up open path exists outside pref/settings change callback. Groups may stay hidden until next explicit toggle. Verify acceptable.

  • getModel(incognito=false) only: hide/show operates on normal model. Synced groups in other models unhandled—likely fine since sync is normal-profile only, but confirm.

  • Test coverage gap: BraveTabGroupSyncControllerImpl (new controller logic, observers, lifecycle) has no unit test. Only BraveSyncedTabGroupHelper tested. Startup catch-up, active-window gating, pref-change wiring untested.

Changes

Changes

  • brave_tabs_and_tab_groups_preferences.xml: renamed key auto_open_synced_tab_groups_switchshow_synced_tab_groups_switch, icon ic_smartphone_laptopic_product_sync, title updated.
  • BraveTabsAndTabGroupsSettings.java: renamed pref constant/method; switch now independent of master switch (removed setPreferenceEnabled gating); calls BraveSyncedTabGroupHelper.notifySettingsChanged() on master switch change; isTabGroupSyncAutoOpenConfigurableisTabGroupSyncConfigurable.
  • BraveTabsAndTabGroupsSettingsTest.java: updated to assert switch stays enabled regardless of master switch.
  • BraveSyncedTabGroupHelper.java (new): visibility check + hide/show ops + settings observer list.
  • BraveTabGroupSyncControllerImpl.java (new): subclass wiring pref/settings observers and startup catch-up.
  • BraveSyncedTabGroupHelperUnitTest.java (new): tests visibility/hide/show helper.
  • Removed BraveTabGroupSyncRemoteObserver.java + test + sources.gni + associated patches.
  • BUILD.gn (both): moved test target, added deps for new sources.
  • android_brave_strings.grd: added IDS_TAB_GROUPS_SHOW_SYNCED_TITLE.
  • icons.gni: dropped ic_smartphone_laptop.xml.
  • patches/rewrite: switched from patching TabGroupSyncRemoteObserver to de-finalizing TabGroupSyncControllerImpl and repointing TabbedRootUiCoordinator import.
sequenceDiagram
    participant User
    participant Settings as BraveTabsAndTabGroupsSettings
    participant Helper as BraveSyncedTabGroupHelper
    participant Ctrl as BraveTabGroupSyncControllerImpl
    participant Pref as PrefService
    participant Sync as TabGroupSyncService
    participant Model as TabModel

    Note over Ctrl: construction
    Ctrl->>Pref: PrefChangeRegistrar.addObserver(AUTO_OPEN_SYNCED)
    Ctrl->>Helper: addSettingsObserver(mSettingsObserver)
    Ctrl->>Sync: addObserver(initObserver) after tab state init

    Sync-->>Ctrl: onInitialized()
    Ctrl->>Helper: areSyncedTabGroupsVisible(pref)
    alt not visible
        Ctrl->>Helper: hideSyncedTabGroups(model, sync)
        Helper->>Model: closeTabs(hideTabGroups=true, allowUndo=false)
    end

    User->>Settings: toggle "Show synced tab groups"
    Settings->>Pref: setBoolean(AUTO_OPEN_SYNCED, val)
    Pref-->>Ctrl: onSyncedTabGroupVisibilityChanged()
    alt visible && active window
        Ctrl->>Helper: showSyncedTabGroups(sync, this)
        Helper->>Sync: getAllGroupIds / getGroup
        Helper->>Ctrl: openTabGroup(syncId)
    else not visible
        Ctrl->>Helper: hideSyncedTabGroups(model, sync)
    end

    User->>Settings: toggle master "Enable tab groups"
    Settings->>Helper: notifySettingsChanged()
    Helper-->>Ctrl: mSettingsObserver.run()
    Ctrl->>Ctrl: onSyncedTabGroupVisibilityChanged()
Loading

@github-actions

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/skip-ios Do not run CI builds for iOS CI/skip-macos-arm64 Do not run CI builds for macOS arm64 CI/skip-windows-x64 Do not run CI builds for Windows x64 puLL-Merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Android] Handle synced tab groups when main toggle is off

1 participant