Skip to content

Commit

Permalink
[Thumbnail Cache] Eagerly prune cache when not tabs not visible
Browse files Browse the repository at this point in the history
Make TabContentManager#updateVisibleIds() a reliable signal for which
thumbnails are needed in the cache. Prune any entries not required by
the current Android Layout. Layout and its subclasses are the primary
users of updateVisibleIds.

The primary tab id is considered to have a live layer and not require
a thumbnail in the cache. All other requested tab ids will be added
to the cache up to the configured maximum size. Any tab ids still in
the cache that are not required will be pruned when the
kThumbnailCacheRefactor flag is enabled.

This CL also does a bit of tidying up of callers of updateVisibleIds
in particular StaticLayout which now releases its visible tab ids upon
a live layer being available.

Bug: 1402843
Change-Id: Ibb82db18d2410214a1cc504f41a8a4fe8212525a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4226062
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1108437}
  • Loading branch information
ckitagawa-work authored and Chromium LUCI CQ committed Feb 22, 2023
1 parent 4d31782 commit 204d640
Show file tree
Hide file tree
Showing 24 changed files with 495 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ protected void updateCacheVisibleIdsAndPrimary(List<Integer> visible, int primar
* is no primary screen-filling tab.
*/
protected void updateCacheVisibleIds(List<Integer> visible) {
if (mTabContentManager != null) mTabContentManager.updateVisibleIds(visible, -1);
updateCacheVisibleIdsAndPrimary(visible, Tab.INVALID_TAB_ID);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package org.chromium.chrome.browser.compositor.layouts;

import android.animation.Animator;
import android.content.Context;
import android.content.res.Resources;
import android.graphics.RectF;
Expand Down Expand Up @@ -31,14 +32,15 @@
import org.chromium.chrome.browser.theme.ThemeUtils;
import org.chromium.chrome.browser.theme.TopUiThemeColorProvider;
import org.chromium.chrome.browser.ui.native_page.NativePage;
import org.chromium.components.browser_ui.widget.animation.CancelAwareAnimatorListener;
import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.resources.ResourceManager;
import org.chromium.url.GURL;

import java.util.Arrays;
import java.util.LinkedList;
import java.util.Collections;
import java.util.List;

// TODO(meiliang): Rename to StaticLayoutMediator.
/**
Expand All @@ -62,11 +64,16 @@ public void run() {
.ofWritableFloatPropertyKey(mAnimationHandler, mModel, LayoutTab.SATURATION,
mModel.get(LayoutTab.SATURATION), 1.0f, HIDE_DURATION_MS)
.start();
CompositorAnimator
.ofWritableFloatPropertyKey(mAnimationHandler, mModel,
LayoutTab.STATIC_TO_VIEW_BLEND,
mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0.0f, HIDE_DURATION_MS)
.start();
CompositorAnimator animator = CompositorAnimator.ofWritableFloatPropertyKey(
mAnimationHandler, mModel, LayoutTab.STATIC_TO_VIEW_BLEND,
mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0.0f, HIDE_DURATION_MS);
animator.addListener(new CancelAwareAnimatorListener() {
@Override
public void onEnd(Animator animation) {
updateVisibleIdsLiveLayerOnly();
}
});
animator.start();
mModel.set(LayoutTab.SHOULD_STALL, false);
}
}
Expand Down Expand Up @@ -308,19 +315,35 @@ private void requestFocus(Tab tab) {
if (mIsActive && tab.getView() != null) tab.getView().requestFocus();
}

private void updateVisibleIdsLiveLayerOnly() {
// Check if we can use the live texture as frozen or native pages don't support live layer.
if (mModel.get(LayoutTab.CAN_USE_LIVE_TEXTURE)) {
updateVisibleIds(Collections.emptyList(), mModel.get(LayoutTab.TAB_ID));
} else {
updateVisibleIds();
}
}

private void updateVisibleIds() {
final int tabId = mModel.get(LayoutTab.TAB_ID);
updateVisibleIds(Collections.singletonList(tabId), tabId);
}

private void updateVisibleIds(List<Integer> tabIds, int primaryTabId) {
if (mTabContentManager != null) {
mTabContentManager.updateVisibleIds(tabIds, primaryTabId);
}
}

private void setStaticTab(Tab tab) {
assert tab != null;

if (mModel.get(LayoutTab.TAB_ID) == tab.getId() && !mModel.get(LayoutTab.SHOULD_STALL)) {
setPostHideState();
updateVisibleIdsLiveLayerOnly();
return;
}

if (mTabContentManager != null) {
mTabContentManager.updateVisibleIds(
new LinkedList<Integer>(Arrays.asList(tab.getId())), tab.getId());
}

mModel.set(LayoutTab.TAB_ID, tab.getId());
mModel.set(LayoutTab.IS_INCOGNITO, tab.isIncognito());
mModel.set(LayoutTab.ORIGINAL_CONTENT_WIDTH_IN_DP, mViewHost.getWidth() * mPxToDp);
Expand Down Expand Up @@ -354,6 +377,16 @@ private void updateStaticTab(Tab tab) {
boolean canUseLiveTexture =
tab.getWebContents() != null && !SadTab.isShowing(tab) && !isNativePage;
mModel.set(LayoutTab.CAN_USE_LIVE_TEXTURE, canUseLiveTexture);

// TODO(crbug/1402843): Move SHOULD_STALL checks inside the updateVisibleId* methods.
if (mModel.get(LayoutTab.SHOULD_STALL)) {
// TODO(crbug/1402843): if canUseLiveTexture is true it should be possible to use
// updateVisibleIdsLiveLayerOnly(). However, this was causing previous content
// to show when undoing a tab closure originating from the tab group bottom bar.
updateVisibleIds();
} else {
updateVisibleIdsLiveLayerOnly();
}
}

private int getToolbarTextBoxBackgroundColor(Tab tab) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,13 @@ public class TabContentManager {
private final Set<Integer> mRefectchedTabIds = new HashSet<>();

private float mThumbnailScale;
/**
* The limit on the number of fullsized or ETC1 compressed thumbnails in the in-memory cache.
* If in future there is a need for more bitmaps to be visible on the screen at once this value
* can be increased.
*/
private int mFullResThumbnailsMaxSize;
private final ContentOffsetProvider mContentOffsetProvider;
private int[] mPriorityTabIds;
private long mNativeTabContentManager;

private final ArrayList<ThumbnailChangeListener> mListeners =
Expand Down Expand Up @@ -175,8 +179,6 @@ public TabContentManager(Context context, ContentOffsetProvider contentOffsetPro
}
}
mThumbnailScale = thumbnailScale;

mPriorityTabIds = new int[mFullResThumbnailsMaxSize];
}

/**
Expand Down Expand Up @@ -574,21 +576,21 @@ public void invalidateIfChanged(int tabId, GURL url) {

/**
* Update the priority-ordered list of visible tabs.
* @param priority The list of tab ids ordered in terms of priority.
* @param priority The list of tab ids to load cached thumbnails for. Only the first
* {@link mFullResThumbnailsMaxSize} thumbnails will be loaded.
* @param primaryTabId The id of the current tab this is not loaded under the assumption it will
* have a live layer. If this is not the case it should be the first tab in
* the priority list.
*/
public void updateVisibleIds(List<Integer> priority, int primaryTabId) {
if (mNativeTabContentManager != 0) {
int idsSize = min(mFullResThumbnailsMaxSize, priority.size());

if (idsSize != mPriorityTabIds.length) {
mPriorityTabIds = new int[idsSize];
}

int[] priorityIds = new int[idsSize];
for (int i = 0; i < idsSize; i++) {
mPriorityTabIds[i] = priority.get(i);
priorityIds[i] = priority.get(i);
}
TabContentManagerJni.get().updateVisibleIds(
mNativeTabContentManager, mPriorityTabIds, primaryTabId);
mNativeTabContentManager, priorityIds, primaryTabId);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -53,7 +54,7 @@
import org.chromium.url.JUnitTestGURLs;

import java.util.Arrays;
import java.util.LinkedList;
import java.util.Collections;

/**
* Unit tests for {@link StaticLayout}.
Expand Down Expand Up @@ -177,6 +178,8 @@ public void setUp() {

mStaticLayout.show(System.currentTimeMillis(), false);
initAndAssertAllProperties();
// Reset calls to the mock as it will have been called during init.
reset(mTabContentManager);
}

@After
Expand Down Expand Up @@ -250,6 +253,25 @@ public void testTabSelection() {
assertFalse(mModel.get(LayoutTab.SHOULD_STALL));
assertEquals(0.0f, mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0);
assertEquals(1.0f, mModel.get(LayoutTab.SATURATION), 0);
assertTrue(mModel.get(LayoutTab.CAN_USE_LIVE_TEXTURE));
verify(mTabContentManager).updateVisibleIds(eq(Collections.emptyList()), eq(TAB2_ID));
}

@Test
public void testTabSelectionNativeTab() {
assertNotEquals(mTab2.getId(), mModel.get(LayoutTab.TAB_ID));
doReturn(true).when(mTab2).isNativePage();

getTabModelSelectorTabModelObserverFromCaptor().didSelectTab(
mTab2, TabSelectionType.FROM_USER, TAB1_ID);

assertEquals(mTab2.getId(), mModel.get(LayoutTab.TAB_ID));
assertFalse(mModel.get(LayoutTab.SHOULD_STALL));
assertEquals(0.0f, mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0);
assertEquals(1.0f, mModel.get(LayoutTab.SATURATION), 0);
assertFalse(mModel.get(LayoutTab.CAN_USE_LIVE_TEXTURE));
verify(mTabContentManager)
.updateVisibleIds(eq(Collections.singletonList(TAB2_ID)), eq(TAB2_ID));
}

@Test
Expand All @@ -262,6 +284,8 @@ public void testTabSelection_Stall() {
assertTrue(mModel.get(LayoutTab.SHOULD_STALL));
assertEquals(1.0f, mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0);
assertEquals(0.0f, mModel.get(LayoutTab.SATURATION), 0);
verify(mTabContentManager)
.updateVisibleIds(eq(Collections.singletonList(TAB2_ID)), eq(TAB2_ID));
}

@Test
Expand All @@ -272,6 +296,7 @@ public void testTabSelection_SameTab() {
assertFalse(mModel.get(LayoutTab.SHOULD_STALL));
assertEquals(0.0f, mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0);
assertEquals(1.0f, mModel.get(LayoutTab.SATURATION), 0);
verify(mTabContentManager).updateVisibleIds(eq(Collections.emptyList()), eq(TAB1_ID));
}

@Test
Expand All @@ -282,6 +307,8 @@ public void testOnPageLoadFinished() {
assertTrue(mModel.get(LayoutTab.SHOULD_STALL));
assertEquals(1.0f, mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0);
assertEquals(0.0f, mModel.get(LayoutTab.SATURATION), 0);
verify(mTabContentManager)
.updateVisibleIds(eq(Collections.singletonList(TAB2_ID)), eq(TAB2_ID));

// Index 1 is the TabObserver for mTab2.
mTabObserverCaptor.getAllValues().get(1).onPageLoadFinished(
Expand All @@ -290,6 +317,7 @@ public void testOnPageLoadFinished() {
assertFalse(mModel.get(LayoutTab.SHOULD_STALL));
assertEquals(0.0f, mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0);
assertEquals(1.0f, mModel.get(LayoutTab.SATURATION), 0);
verify(mTabContentManager).updateVisibleIds(eq(Collections.emptyList()), eq(TAB2_ID));
}

@Test
Expand All @@ -300,10 +328,10 @@ public void testTabOnShown() {
mTabObserverCaptor.getAllValues().get(1).onShown(mTab2, TabSelectionType.FROM_USER);
assertEquals(TAB2_ID, mModel.get(LayoutTab.TAB_ID));
assertTrue(mModel.get(LayoutTab.CAN_USE_LIVE_TEXTURE));
assertFalse(mModel.get(LayoutTab.SHOULD_STALL));
assertEquals(0.0f, mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0);

LinkedList visibleIdList = new LinkedList<Integer>(Arrays.asList(TAB2_ID));
verify(mTabContentManager).updateVisibleIds(eq(visibleIdList), eq(TAB2_ID));
verify(mTabContentManager).updateVisibleIds(eq(Collections.emptyList()), eq(TAB2_ID));
}

@Test
Expand All @@ -312,6 +340,8 @@ public void testTabOnContentChanged() {
mTabObserverCaptor.getAllValues().get(0).onContentChanged(mTab1);
assertTrue(mModel.get(LayoutTab.CAN_USE_LIVE_TEXTURE));
assertEquals(0.0f, mModel.get(LayoutTab.STATIC_TO_VIEW_BLEND), 0);

verify(mTabContentManager).updateVisibleIds(eq(Collections.emptyList()), eq(TAB1_ID));
}

@Test
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#include "chrome/browser/sharing_hub/sharing_hub_features.h"
#include "chrome/browser/signin/signin_features.h"
#include "chrome/browser/site_isolation/about_flags.h"
#include "chrome/browser/thumbnail/cc/features.h"
#include "chrome/browser/ui/browser_navigator_params.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/unexpire_flags.h"
Expand Down Expand Up @@ -9367,7 +9368,7 @@ const FeatureEntry kFeatureEntries[] = {
#if BUILDFLAG(IS_ANDROID)
{"thumbnail-cache-refactor", flag_descriptions::kThumbnailCacheRefactorName,
flag_descriptions::kThumbnailCacheRefactorDescription, kOsAndroid,
FEATURE_VALUE_TYPE(chrome::android::kThumbnailCacheRefactor)},
FEATURE_VALUE_TYPE(thumbnail::kThumbnailCacheRefactor)},
#endif // BUILDFLAG(IS_ANDROID)

{"autofill-enable-page-load-metadata-integration",
Expand Down
15 changes: 8 additions & 7 deletions chrome/browser/android/compositor/layer/thumbnail_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ scoped_refptr<ThumbnailLayer> ThumbnailLayer::Create() {
return base::WrapRefCounted(new ThumbnailLayer());
}

void ThumbnailLayer::SetThumbnail(Thumbnail* thumbnail) {
void ThumbnailLayer::SetThumbnail(thumbnail::Thumbnail* thumbnail) {
layer_->SetUIResourceId(thumbnail->ui_resource_id());
UpdateSizes(thumbnail->scaled_content_size(), thumbnail->scaled_data_size());
}
Expand Down Expand Up @@ -46,10 +46,11 @@ void ThumbnailLayer::ClearClip() {
void ThumbnailLayer::AddSelfToParentOrReplaceAt(
scoped_refptr<cc::slim::Layer> parent,
size_t index) {
if (index >= parent->children().size())
if (index >= parent->children().size()) {
parent->AddChild(layer_);
else if (parent->children()[index]->id() != layer_->id())
} else if (parent->children()[index]->id() != layer_->id()) {
parent->ReplaceChild(parent->children()[index].get(), layer_);
}
}

scoped_refptr<cc::slim::Layer> ThumbnailLayer::layer() {
Expand All @@ -60,18 +61,18 @@ ThumbnailLayer::ThumbnailLayer() : layer_(cc::slim::UIResourceLayer::Create()) {
layer_->SetIsDrawable(true);
}

ThumbnailLayer::~ThumbnailLayer() {
}
ThumbnailLayer::~ThumbnailLayer() = default;

void ThumbnailLayer::UpdateSizes(const gfx::SizeF& content_size,
const gfx::SizeF& resource_size) {
if (content_size != content_size_ || resource_size != resource_size_) {
content_size_ = content_size;
resource_size_ = resource_size;
if (clipped_)
if (clipped_) {
Clip(last_clipping_);
else
} else {
ClearClip();
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/android/compositor/layer/thumbnail_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/size_f.h"

namespace thumbnail {
class Thumbnail;
} // namespace thumbnail

namespace cc::slim {
class Layer;
Expand All @@ -32,7 +34,7 @@ class ThumbnailLayer : public Layer {
ThumbnailLayer& operator=(const ThumbnailLayer&) = delete;

// Sets thumbnail that will be shown. |thumbnail| should not be nullptr.
void SetThumbnail(Thumbnail* thumbnail);
void SetThumbnail(thumbnail::Thumbnail* thumbnail);
// Clip the thumbnail to the given |clipping|.
void Clip(const gfx::Rect& clipping);
void ClearClip();
Expand Down
10 changes: 8 additions & 2 deletions chrome/browser/android/compositor/tab_content_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ TabContentManager::TabContentManager(JNIEnv* env,
jboolean save_jpeg_thumbnails,
jdouble jpeg_aspect_ratio)
: weak_java_tab_content_manager_(env, obj) {
thumbnail_cache_ = std::make_unique<ThumbnailCache>(
thumbnail_cache_ = std::make_unique<thumbnail::ThumbnailCache>(
static_cast<size_t>(default_cache_size),
static_cast<size_t>(approximation_cache_size),
static_cast<size_t>(compression_queue_max_size),
Expand Down Expand Up @@ -159,10 +159,16 @@ scoped_refptr<ThumbnailLayer> TabContentManager::GetStaticLayer(int tab_id) {
return static_layer_cache_[tab_id];
}

// TODO(crbug.com/1402843): ThumbnailCache::PruneCache() shouldn't cause issues
// with `static_layer_cache_` as Thumbnails referenced by this cache may already
// expire even without eager pruning. Investigate whether entries in
// `static_layer_cache_` can and should be removed when their thumbnail is
// dropped from the ThumbnailCache.
scoped_refptr<ThumbnailLayer> TabContentManager::GetOrCreateStaticLayer(
int tab_id,
bool force_disk_read) {
Thumbnail* thumbnail = thumbnail_cache_->Get(tab_id, force_disk_read, true);
thumbnail::Thumbnail* thumbnail =
thumbnail_cache_->Get(tab_id, force_disk_read, true);
scoped_refptr<ThumbnailLayer> static_layer = static_layer_cache_[tab_id];

if (!thumbnail || !thumbnail->ui_resource_id()) {
Expand Down

0 comments on commit 204d640

Please sign in to comment.