Skip to content

Commit

Permalink
[5790] Don't keep Inactive Tabs VC in memory
Browse files Browse the repository at this point in the history
This CL changes the lifecycle of InactiveTabsViewController: instead of
keeping it alive as long as the regular Tab Grid (i.e. forever), it's
created when presented, and destroyed when dismissed.
This is because a Tab Grid incurs a large memory cost due to the tab
snapshots that it displays. By destroying the Inactive Tabs VC when not
used, we ensure a lower memory footprint throughout the use of the app.

This CL regresses on one point: the scroll offset is no longer kept
between showings of the Inactive Tabs VC. Opened crbug.com/1448025 to
track.

(cherry picked from commit 15cbe1e)

Bug: 1446088
Change-Id: I8dee1fd06b04cd22fa679b7a6f6cf637bcbb5624
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4555825
Reviewed-by: Ewann Pellé <ewannpv@chromium.org>
Auto-Submit: Louis Romero <lpromero@google.com>
Commit-Queue: Ewann Pellé <ewannpv@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1148425}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4591849
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Louis Romero <lpromero@google.com>
Cr-Commit-Position: refs/branch-heads/5790@{#378}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Arcank authored and Chromium LUCI CQ committed Jun 5, 2023
1 parent b9631ed commit 9f17187
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 56 deletions.
Expand Up @@ -25,14 +25,16 @@

@end

// Handles interaction with the inactive tabs view controller.
// Handles interaction for Inactive Tabs.
//
// This coordinator lifetime starts when the regular tab grid is started, and
// stops only when the regular tab grid is stopped. `start` creates the relevant
// objects (VC, mediator, etc.), but doesn't show the VC. Call `show`/`hide` to
// display/hide the inactive tabs grid. By having this coordinator alive, the
// mediator can react to "Close All" signals, and the VC can be re-shown as is
// (i.e. same scroll position).
// stops only when the regular tab grid is stopped. `start` creates the mediator
// but not the VC. By having this coordinator and its mediator always alive, the
// mediator can react to "Close All" signals even when the Inactive Tabs UI is
// not shown.
// The VC (i.e. Inactive Tabs UI) is created and shown when calling `show`, and
// hidden and destroyed when calling `hide`. This can be called multiple times.
// TODO(crbug.com/1448025): Keep the scrolling position between showings.
@interface InactiveTabsCoordinator : ChromeCoordinator

// The GridCommands receiver handling "Close All"-related commands.
Expand Down
Expand Up @@ -184,18 +184,6 @@ @implementation InactiveTabsCoordinator {
- (void)start {
[super start];

// Create the view controller.
self.viewController = [[InactiveTabsViewController alloc] init];
self.viewController.delegate = self;
self.viewController.gridViewController.delegate = self;

UIScreenEdgePanGestureRecognizer* edgeSwipeRecognizer =
[[UIScreenEdgePanGestureRecognizer alloc]
initWithTarget:self
action:@selector(onEdgeSwipe:)];
edgeSwipeRecognizer.edges = UIRectEdgeLeft;
[self.viewController.view addGestureRecognizer:edgeSwipeRecognizer];

// Create the mediator.
SessionRestorationBrowserAgent* sessionRestorationBrowserAgent =
SessionRestorationBrowserAgent::FromBrowser(self.browser);
Expand All @@ -206,14 +194,11 @@ - (void)start {
self.browser->GetBrowserState());

self.mediator = [[InactiveTabsMediator alloc]
initWithConsumer:self.viewController.gridViewController
webStateList:self.browser->GetWebStateList()
initWithWebStateList:self.browser->GetWebStateList()
prefService:GetApplicationContext()->GetLocalState()
sessionRestorationAgent:sessionRestorationBrowserAgent
snapshotAgent:snapshotBrowserAgent
tabRestoreService:tabRestoreService];

self.viewController.gridViewController.menuProvider = _menuProvider;
}

- (void)show {
Expand All @@ -223,6 +208,22 @@ - (void)show {
self.showing = YES;
base::RecordAction(base::UserMetricsAction("MobileInactiveTabGridEntered"));

// Create the view controller.
self.viewController = [[InactiveTabsViewController alloc] init];
self.viewController.delegate = self;
self.viewController.gridViewController.delegate = self;

UIScreenEdgePanGestureRecognizer* edgeSwipeRecognizer =
[[UIScreenEdgePanGestureRecognizer alloc]
initWithTarget:self
action:@selector(onEdgeSwipe:)];
edgeSwipeRecognizer.edges = UIRectEdgeLeft;
[self.viewController.view addGestureRecognizer:edgeSwipeRecognizer];

self.mediator.consumer = self.viewController.gridViewController;

self.viewController.gridViewController.menuProvider = _menuProvider;

// Add the Inactive Tabs view controller to the hierarchy.
UIView* baseView = self.baseViewController.view;
UIView* view = self.viewController.view;
Expand Down Expand Up @@ -614,6 +615,8 @@ - (void)animateOut {
[self.baseViewSnapshot removeFromSuperview];
self.baseViewSnapshot = nil;
self.showing = NO;
self.mediator.consumer = nil;
self.viewController = nil;
}];
}

Expand Down
Expand Up @@ -24,22 +24,24 @@ class TabRestoreService;
// interactions.
@interface InactiveTabsMediator : NSObject <GridCommands>

// `consumer` receives `webStateList` and Inactive Tabs info updates.
@property(nonatomic, weak) id<TabCollectionConsumer, InactiveTabsInfoConsumer>
consumer;

// Initializer with:
// `consumer` as the receiver of `webStateList` updates.
// `prefService` the preference service from the application context.
// `sessionRestorationAgent` the session restoration browser agent from the
// inactive browser. `snapshotAgent` the snapshot browser agent from the
// inactive browser. `tabRestoreService` the service that holds the recently
// closed tabs.
- (instancetype)initWithConsumer:
(id<TabCollectionConsumer, InactiveTabsInfoConsumer>)
consumer
webStateList:(WebStateList*)webStateList
prefService:(PrefService*)prefService
sessionRestorationAgent:
(SessionRestorationBrowserAgent*)sessionRestorationAgent
snapshotAgent:(SnapshotBrowserAgent*)snapshotAgent
tabRestoreService:(sessions::TabRestoreService*)tabRestoreService
// - `webStateList`: the list of tabs to observe.
// - `prefService`: the preference service from the application context.
// - `sessionRestorationAgent`: the session restoration browser agent from the
// inactive browser.
// - `snapshotAgent`: the snapshot browser agent from the inactive browser.
// - `tabRestoreService`: the service that holds the recently closed tabs.
- (instancetype)initWithWebStateList:(WebStateList*)webStateList
prefService:(PrefService*)prefService
sessionRestorationAgent:
(SessionRestorationBrowserAgent*)sessionRestorationAgent
snapshotAgent:(SnapshotBrowserAgent*)snapshotAgent
tabRestoreService:
(sessions::TabRestoreService*)tabRestoreService
NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;

Expand Down
Expand Up @@ -87,8 +87,6 @@ @interface InactiveTabsMediator () <CRWWebStateObserver,
PrefObserverDelegate,
SnapshotCacheObserver,
WebStateListObserving> {
// The UI consumer to which updates are made.
__weak id<TabCollectionConsumer, InactiveTabsInfoConsumer> _consumer;
// The list of inactive tabs.
WebStateList* _webStateList;
// The snapshot cache of _webStateList.
Expand Down Expand Up @@ -123,17 +121,14 @@ @interface InactiveTabsMediator () <CRWWebStateObserver,

@implementation InactiveTabsMediator

- (instancetype)
initWithConsumer:
(id<TabCollectionConsumer, InactiveTabsInfoConsumer>)consumer
webStateList:(WebStateList*)webStateList
prefService:(PrefService*)prefService
sessionRestorationAgent:
(SessionRestorationBrowserAgent*)sessionRestorationAgent
snapshotAgent:(SnapshotBrowserAgent*)snapshotAgent
tabRestoreService:(sessions::TabRestoreService*)tabRestoreService {
- (instancetype)initWithWebStateList:(WebStateList*)webStateList
prefService:(PrefService*)prefService
sessionRestorationAgent:
(SessionRestorationBrowserAgent*)sessionRestorationAgent
snapshotAgent:(SnapshotBrowserAgent*)snapshotAgent
tabRestoreService:
(sessions::TabRestoreService*)tabRestoreService {
CHECK(IsInactiveTabsAvailable());
CHECK(consumer);
CHECK(webStateList);
CHECK(prefService);
CHECK(sessionRestorationAgent);
Expand All @@ -142,7 +137,6 @@ @implementation InactiveTabsMediator
CHECK(tabRestoreService);
self = [super init];
if (self) {
_consumer = consumer;
_webStateList = webStateList;

// Observe the web state list.
Expand All @@ -169,12 +163,6 @@ @implementation InactiveTabsMediator
_prefObserverBridge->ObserveChangesForPreference(
prefs::kInactiveTabsTimeThreshold, &_prefChangeRegistrar);

// Push the tabs to the consumer.
PopulateConsumerItems(_consumer, _webStateList);
// Push the info to the consumer.
NSInteger daysThreshold = InactiveTabsTimeThreshold().InDays();
[_consumer updateInactiveTabsDaysThreshold:daysThreshold];

_snapshotCache = snapshotAgent->snapshot_cache();
[_snapshotCache addObserver:self];

Expand All @@ -189,6 +177,21 @@ - (void)dealloc {
[_snapshotCache removeObserver:self];
}

- (void)setConsumer:
(id<TabCollectionConsumer, InactiveTabsInfoConsumer>)consumer {
if (_consumer == consumer) {
return;
}

_consumer = consumer;

// Push the tabs to the consumer.
PopulateConsumerItems(_consumer, _webStateList);
// Push the info to the consumer.
NSInteger daysThreshold = InactiveTabsTimeThreshold().InDays();
[_consumer updateInactiveTabsDaysThreshold:daysThreshold];
}

- (NSInteger)numberOfItems {
return _webStateList->count();
}
Expand Down

0 comments on commit 9f17187

Please sign in to comment.