Skip to content

Commit

Permalink
Fixed inactive tabs "flashing" issue.
Browse files Browse the repository at this point in the history
All inactive cells were "flashing" when switching from the tab view to a
pinned cell on the tab grid. This happened because snapshots weren't
preloaded for the inactive cell on the tab grid.

Previous logic expected that the active cell could be only in the tab
grid. Therefore, the snapshots were preloaded for the cells before and
after the active one. Currently, the active cell could be a pinned one.
Thus, the preloading logic stopped working. Without preloading the grid
cells are unable to load the snapshot fast enough to display it during
the contracting animation. So, the cells fade in an empty view which
from the user perspective looks like a flash.

This CL updates the snapshots preloading logic. Now the snapshots
preloading is done for visible items in the grid only.

Fixed: 1417139
Change-Id: Id71348d0d1454c9700c7cc98529b01513014fc1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4255770
Commit-Queue: Petro Akzhygitov <pakzhygitov@google.com>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1107416}
  • Loading branch information
pakzhygitov authored and Chromium LUCI CQ committed Feb 20, 2023
1 parent bfe320f commit b7e9ab9
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@
- (void)faviconForIdentifier:(NSString*)identifier
completion:(void (^)(UIImage*))completion;

// Asks the receiver to pre-fetch all of the snapshots for tabs that are likely
// to be visible, given that `gridSize` tabs can be displayed at once.
- (void)preloadSnapshotsForVisibleGridSize:(int)gridSize;
// Asks the receiver to pre-fetch all of the snapshots for tabs that are
// currently visible. Visible tabs identifiers are provided via
// `visibleGridItems` array.
- (void)preloadSnapshotsForVisibleGridItems:(NSSet<NSString*>*)visibleGridItems;

// Tells the receiver to dispose of any pre-loaded snapshots it may have cached.
- (void)clearPreloadedSnapshots;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@
@property(nonatomic, strong) UIView<GridEmptyView>* emptyStateView;
// Returns YES if the grid has no items.
@property(nonatomic, readonly, getter=isGridEmpty) BOOL gridEmpty;
// Currently visible items in the grid.
@property(nonatomic, readonly) NSSet<NSString*>* visibleGridItems;
// The visual look of the grid.
@property(nonatomic, assign) GridTheme theme;
// The current mode for the grid.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,12 @@ - (BOOL)isGridEmpty {
return self.items.count == 0;
}

- (NSSet<NSString*>*)visibleGridItems {
NSArray<NSIndexPath*>* visibleItemsIndexPaths =
[self.collectionView indexPathsForVisibleItems];
return [self itemIdentifiersFromIndexPaths:visibleItemsIndexPaths];
}

- (void)setMode:(TabGridMode)mode {
if (_mode == mode) {
return;
Expand Down Expand Up @@ -1799,6 +1805,22 @@ - (void)updateSearchResultsHeader {
base::SysNSStringToUTF16(resultsCount));
}

// Converts `indexPaths` into corresponding item identifiers.
- (NSSet<NSString*>*)itemIdentifiersFromIndexPaths:
(NSArray<NSIndexPath*>*)indexPaths {
NSMutableSet<NSString*>* itemIdentifiers = [NSMutableSet set];

[indexPaths enumerateObjectsUsingBlock:^(NSIndexPath* indexPath,
NSUInteger index, BOOL* stop) {
NSUInteger itemIndex = base::checked_cast<NSUInteger>(indexPath.item);
if (itemIndex < self.items.count) {
[itemIdentifiers addObject:self.items[itemIndex].identifier];
}
}];

return [itemIdentifiers copy];
}

#pragma mark Suggested Actions Section

- (void)updateSuggestedActionsSection {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,8 @@ - (void)faviconForIdentifier:(NSString*)identifier
}
}

- (void)preloadSnapshotsForVisibleGridSize:(int)gridSize {
- (void)preloadSnapshotsForVisibleGridItems:
(NSSet<NSString*>*)visibleGridItems {
// TODO (crbug.com/1406524): Implement or remove.
}

Expand Down
16 changes: 12 additions & 4 deletions ios/chrome/browser/ui/tab_switcher/tab_grid/tab_grid_mediator.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1005,16 +1005,24 @@ - (void)faviconForIdentifier:(NSString*)identifier
}
}

- (void)preloadSnapshotsForVisibleGridSize:(int)gridSize {
int startIndex = std::max(self.webStateList->active_index() - gridSize, 0);
int endIndex = std::min(self.webStateList->active_index() + gridSize,
self.webStateList->count() - 1);
- (void)preloadSnapshotsForVisibleGridItems:
(NSSet<NSString*>*)visibleGridItems {
int startIndex = self.webStateList->GetIndexOfFirstNonPinnedWebState();
int endIndex = self.webStateList->count() - 1;

for (int i = startIndex; i <= endIndex; i++) {
web::WebState* web_state = self.webStateList->GetWebStateAt(i);
NSString* identifier = web_state->GetStableIdentifier();

BOOL isWebStateHidden = ![visibleGridItems containsObject:identifier];
if (isWebStateHidden) {
continue;
}

auto cacheImage = ^(UIImage* image) {
self.appearanceCache[identifier] = image;
};

[self snapshotForIdentifier:identifier completion:cacheImage];
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -481,22 +481,30 @@ - (UIView*)animationViewsContainerBottomView {
#pragma mark - Public Methods

- (void)prepareForAppearance {
int gridSize = [self approximateVisibleGridCount];
NSSet<NSString*>* visibleGridItems = [self visibleGridItemsForActivePage];

switch (self.activePage) {
case TabGridPageIncognitoTabs:
[self.incognitoTabsImageDataSource
preloadSnapshotsForVisibleGridSize:gridSize];
preloadSnapshotsForVisibleGridItems:visibleGridItems];
break;
case TabGridPageRegularTabs:
[self.regularTabsImageDataSource
preloadSnapshotsForVisibleGridSize:gridSize];
preloadSnapshotsForVisibleGridItems:visibleGridItems];
break;
case TabGridPageRemoteTabs:
// Nothing to do.
break;
}
}

- (NSSet<NSString*>*)visibleGridItemsForActivePage {
GridViewController* activeGridViewController =
[self gridViewControllerForPage:self.activePage];

return [activeGridViewController visibleGridItems];
}

- (void)contentWillAppearAnimated:(BOOL)animated {
[self resetIdlePageStatus];
self.viewVisible = YES;
Expand Down Expand Up @@ -2116,21 +2124,6 @@ - (void)broadcastIncognitoContentVisibility {
[self.handler setIncognitoContentVisible:incognitoContentVisible];
}

// Returns the approximate number of grid cells that will be visible on this
// device.
- (int)approximateVisibleGridCount {
if (IsRegularXRegularSizeClass(self)) {
// A 12" iPad Pro can show 30 cells in the tab grid.
return 30;
}
if (IsCompactWidth(self)) {
// A portrait phone shows up to four rows of two cells.
return 8;
}
// A landscape phone shows up to three rows of four cells.
return 12;
}

- (void)setupSearchUI {
self.scrimView = [[UIControl alloc] init];
self.scrimView.backgroundColor =
Expand Down
3 changes: 2 additions & 1 deletion ios/showcase/tab_grid/sc_grid_coordinator.mm
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ - (void)faviconForIdentifier:(NSString*)identifier
completion(nil);
}

- (void)preloadSnapshotsForVisibleGridSize:(int)gridSize {
- (void)preloadSnapshotsForVisibleGridItems:
(NSSet<NSString*>*)visibleGridItems {
// No-op here.
}

Expand Down

0 comments on commit b7e9ab9

Please sign in to comment.