Skip to content

Commit

Permalink
[Sheriff] Revert "[bfcache] Add base unit tests to cover default cach…
Browse files Browse the repository at this point in the history
…e sizes."

This reverts commit 98b9026.

Reason for revert: The new test is failing at linux-bfcache-rel

https://ci.chromium.org/ui/p/chromium/builders/ci/linux-bfcache-rel/40144/overview

Original change's description:
> [bfcache] Add base unit tests to cover default cache sizes.
>
> Bug: 1291435
> Change-Id: Ife59940b8e7b35fb62e02ce3500840dbaf5a55d9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4012518
> Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
> Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
> Auto-Submit: Ming-Ying Chung <mych@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1075704}

Bug: 1291435
Change-Id: I1d6f1d3b6a2c8e494c77aea48f935382cef08336
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4056218
Auto-Submit: Roman Sorokin <rsorokin@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Roman Sorokin <rsorokin@google.com>
Cr-Commit-Position: refs/heads/main@{#1075728}
  • Loading branch information
romasorokin authored and Chromium LUCI CQ committed Nov 25, 2022
1 parent b18ef66 commit 837e52f
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 93 deletions.
32 changes: 2 additions & 30 deletions content/browser/renderer_host/back_forward_cache_impl.h
Expand Up @@ -11,7 +11,6 @@
#include <unordered_set>

#include "base/feature_list.h"
#include "base/gtest_prod_util.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/memory/weak_ptr.h"
Expand Down Expand Up @@ -105,28 +104,6 @@ struct CONTENT_EXPORT BackForwardCacheCanStoreDocumentResultWithTree {
// frozen state and is kept in this object. They can potentially be reused
// after an history navigation. Reusing a document means swapping it back with
// the current_frame_host.
//
//
// BackForwardCache Size & Pruning Logic:
//
// 1. `EnforceCacheSizeLimit()` is called to prune the cache size down on
// storing a new cache entry, or when the renderer process's
// `IsProcessBackgrounded()` state changes.
// A. [Android-only] The number of entries where `HasForegroundedProcess()`
// is true is pruned to `GetForegroundedEntriesCacheSize()`.
// B. Prunes to `GetCacheSize()` entries no matter what kinds of tabs
// BackForwardCache is in.
// C. When a `RenderFrameHost` enters BackForwardCache, it schedules a task
// in `RenderFrameHostImpl::StartBackForwardCacheEvictionTimer()` to
// evicts the outermost frame after
// `kDefaultTimeToLiveInBackForwardCacheInSeconds` seconds.
// 2. In `performance_manager::policies::BFCachePolicy`:
// A. (To Launch) [Desktop-only] On moderate memory pressure, the number of
// entries in a visible tab's cache is pruned to
// `ForegroundCacheSizeOnModeratePressure()`. The number in a non-visible
// tab is pruned to `BackgroundCacheSizeOnModeratePressure()`.
// B. (To Launch) [Desktop-only] On critical memory pressure, the cache is
// cleared.
class CONTENT_EXPORT BackForwardCacheImpl
: public BackForwardCache,
public RenderProcessHostInternalObserver,
Expand Down Expand Up @@ -384,6 +361,8 @@ class CONTENT_EXPORT BackForwardCacheImpl
BackForwardCacheCanStoreDocumentResult& eviction_reason);

private:
FRIEND_TEST_ALL_PREFIXES(BackForwardCacheMetricsTest, AllFeaturesCovered);

// Destroys all evicted frames in the BackForwardCache.
void DestroyEvictedFrames();

Expand Down Expand Up @@ -572,13 +551,6 @@ class CONTENT_EXPORT BackForwardCacheImpl
};

base::WeakPtrFactory<BackForwardCacheImpl> weak_factory_;

// For testing:
FRIEND_TEST_ALL_PREFIXES(BackForwardCacheMetricsTest, AllFeaturesCovered);
FRIEND_TEST_ALL_PREFIXES(BackForwardCacheActiveSizeTest, ActiveCacheSize);
FRIEND_TEST_ALL_PREFIXES(BackForwardCacheOverwriteSizeTest,
OverwrittenCacheSize);
FRIEND_TEST_ALL_PREFIXES(BackForwardCacheDefaultSizeTest, DefaultCacheSize);
};

// Allow external code to be notified when back-forward cache is disabled for a
Expand Down
Expand Up @@ -3,13 +3,7 @@
// found in the LICENSE file.

#include "content/browser/renderer_host/back_forward_cache_impl.h"

#include "base/test/scoped_feature_list.h"
#include "content/test/test_render_frame_host.h"
#include "content/test/test_render_view_host.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/scheduler/web_scheduler_tracked_feature.h"

namespace content {

Expand Down Expand Up @@ -94,60 +88,4 @@ TEST_F(BackForwardCacheImplTest, SecondCrossOriginReachable) {
blink::mojom::BFCacheBlocked::kNo);
}

// Covers BackForwardCache's cache size-related values used in Stable.
// See docs/back_forward_cache_size.md for more details.
class BackForwardCacheActiveSizeTest : public ::testing::Test {
protected:
void SetUp() override {
feature_list_.InitWithFeaturesAndParameters(
{{features::kBackForwardCache,
{{"cache_size", "6"}, {"foreground_cache_size", "2"}}}},
{});
}

private:
base::test::ScopedFeatureList feature_list_;
};

TEST_F(BackForwardCacheActiveSizeTest, ActiveCacheSize) {
EXPECT_EQ(BackForwardCacheImpl::GetCacheSize(), 6u);
EXPECT_EQ(BackForwardCacheImpl::GetForegroundedEntriesCacheSize(), 2u);
EXPECT_TRUE(BackForwardCacheImpl::UsingForegroundBackgroundCacheSizeLimit());
}

// Covers overwriting BackForwardCache's cache size-related values.
// When "cache_size" or "foreground_cache_size" presents in both
// `kBackForwardCacheSize` and `features::kBackForwardCache`, the former should
// take precedence.
class BackForwardCacheOverwriteSizeTest : public ::testing::Test {
protected:
void SetUp() override {
feature_list_.InitWithFeaturesAndParameters(
{{kBackForwardCacheSize,
{{"cache_size", "8"}, {"foreground_cache_size", "4"}}},
{features::kBackForwardCache,
{{"cache_size", "6"}, {"foreground_cache_size", "2"}}}},
{});
}

private:
base::test::ScopedFeatureList feature_list_;
};

TEST_F(BackForwardCacheOverwriteSizeTest, OverwrittenCacheSize) {
EXPECT_EQ(BackForwardCacheImpl::GetCacheSize(), 8u);
EXPECT_EQ(BackForwardCacheImpl::GetForegroundedEntriesCacheSize(), 4u);
EXPECT_TRUE(BackForwardCacheImpl::UsingForegroundBackgroundCacheSizeLimit());
}

// Covers BackForwardCache's default cache size-related values.
// Note that these tests don't cover the values configured from Finch.
class BackForwardCacheDefaultSizeTest : public ::testing::Test {};

TEST_F(BackForwardCacheDefaultSizeTest, DefaultCacheSize) {
EXPECT_EQ(BackForwardCacheImpl::GetCacheSize(), 1u);
EXPECT_EQ(BackForwardCacheImpl::GetForegroundedEntriesCacheSize(), 0u);
EXPECT_FALSE(BackForwardCacheImpl::UsingForegroundBackgroundCacheSizeLimit());
}

} // namespace content
} // namespace content

0 comments on commit 837e52f

Please sign in to comment.