Skip to content
Commit 7354959c authored by Samuel Huang's avatar Samuel Huang Committed by Chromium LUCI CQ
Browse files

[Android TRM] Refresh TRM whenever a suggested Local Tab closes.

LocalTabTabResumptionDataProvider produces at most one suggestion (the
most recent valid Local Tab), and listens to its closure event to
trigger refresh, to avoid the situation where a Local Tab suggestions
leads nowhere when clicked.

This CL generalizes the above behavior to all Local Tab suggestions,
including ones from Fetch and Rank service. Three challenges are:
1. TRM needs to look up Tab from |tabId|.
2. Mediator (instead of the Provider) needs to trigger refresh.
3. Tab closure -> TRM refresh tracking goes from one-to-one to
   many-to-many (for the general case where TRM can have multiple Local
   Tab suggestions).

For 1: Perhaps this is best done in ModuleDelegate. But for now, we're
using an approach similar to SingleTabModuleBuilder's: The Builder's
caller ChromeTabbedActivity passes ObservableSupplier<TabModelSelector>,
from which TabModelSelector, then TabModel is obtained (injected into
Mediator). This gives us getTabById().

For 2: Have Coordinator pass |reloadSessionCallback| to Mediator. It's
bound to updateModule().

For 3: Add helper MultiTabObserver to dynamically track multiple Tabs.
Instantiate this in Mediator.Session, and handle any tab closing by
invoking |mReloadSessionCallback|. Observer only Local Tabs, updating in
onSuggestionReceived().

Additional details:
* Mediator.isSuggestionValid(): For Local Tabs, ensure getTabById()
  returns non-null. For now we're skipping |!tab.isClosing()| checks,
  i.e., no race condition with Fetch and Rank service on reload.

Testing:
* TabResumptionModuleMediatorUnitTest:
  * Add |mTabModel|, |mTabObserverMap| and bindBasicTabObservation() to
    simulate Tab observer add / remove. Use doAnswer() for flexibility.
  * Take createLocalTabModelSuggestion() from TestSupport
    * Create mock Tab, and use bindBasicTabObservation().
  * Update testShowOnlyOneLocalTab() to also simulate Tab closing, using
    |mReloadSessionCounter| to detect calls.
    * This exercises MultiTabObserver directly.

Bug: 343095625
Low-Coverage-Reason: TabResumptionModuleCoordinator is mostly glue code, but it's grown big. We'll add test later; tracked in crbug.com/346337515.
Change-Id: Ic30ee1cf758c3b6b256b5691cc23d93ea462e98d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5605667


Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: default avatarXi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1312939}
parent b6dacd79
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment