Skip to content

Commit

Permalink
Revert "BackForwardCache: Add a flag parameter supported_features"
Browse files Browse the repository at this point in the history 
This reverts commit 2db43e7.

Reason for revert: Failure on the linux-bfcache-rel bot. See https://ci.chromium.org/ui/p/chromium/builders/ci/linux-bfcache-rel/12034/overview

Original change's description:
> BackForwardCache: Add a flag parameter supported_features
>
> This CL adds a new flag parameter supported_features to BackForwardCache
> feature. This parameter can specify the set of the features to be
> allowed with back-forward cache. This enables us to enable the feature
> remotely.
>
> Bug: 1155449
> Change-Id: Idb7eb55b114af688533ba2e069a9334714be725c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2573850
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Alexander Timin <altimin@chromium.org>
> Reviewed-by: Fergal Daly <fergal@chromium.org>
> Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#835571}

TBR=hajimehoshi@chromium.org,haraken@chromium.org,altimin@chromium.org,fergal@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: Ie806efc5caecaa1b66c125e9e90c1a1edc0bdd5f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1155449
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582122
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835587}
  • Loading branch information
@msramek
msramek authored and Chromium LUCI CQ committed Dec 10, 2020
1 parent 9274bb3 commit 832fa81
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 293 deletions.
122 changes: 0 additions & 122 deletions content/browser/back_forward_cache_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8702,126 +8702,4 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestWithFileSystemAPISupported,
FROM_HERE);
}

class BackForwardCacheBrowserTestWithSupportedFeatures
: public BackForwardCacheBrowserTest {
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
EnableFeatureAndSetParams(features::kBackForwardCache, "supported_features",
"WebFileSystem,WebLocks");
BackForwardCacheBrowserTest::SetUpCommandLine(command_line);
}
};

IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestWithSupportedFeatures,
CacheWithFileSystemAPI) {
ASSERT_TRUE(embedded_test_server()->Start());
GURL url_a(embedded_test_server()->GetURL("/fileapi/request_test.html"));
GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));

// 1) Navigate to the page A with WebFileSystem usage.
EXPECT_TRUE(NavigateToURL(shell(), url_a));
RenderFrameHostImpl* rfh_a = current_frame_host();
RenderFrameDeletedObserver deleted(rfh_a);

// 2) Navigate away.
EXPECT_TRUE(NavigateToURL(shell(), url_b));
EXPECT_FALSE(deleted.deleted());
EXPECT_TRUE(rfh_a->IsInBackForwardCache());

// 3) Go back to the page A
web_contents()->GetController().GoBack();
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
EXPECT_EQ(rfh_a, current_frame_host());
ExpectOutcome(BackForwardCacheMetrics::HistoryNavigationOutcome::kRestored,
FROM_HERE);

// 4) Use WebLock
EXPECT_TRUE(ExecJs(rfh_a, R"(
const never_resolved = new Promise(resolve => {});
new Promise(continue_test => {
navigator.locks.request('test', async () => {
continue_test();
await never_resolved;
});
})
)"));

// 5) Navigate away again.
EXPECT_TRUE(NavigateToURL(shell(), url_b));
EXPECT_FALSE(deleted.deleted());
EXPECT_TRUE(rfh_a->IsInBackForwardCache());

// 6) Go back to the page A again.
web_contents()->GetController().GoBack();
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
EXPECT_EQ(rfh_a, current_frame_host());
ExpectOutcome(BackForwardCacheMetrics::HistoryNavigationOutcome::kRestored,
FROM_HERE);
}

class BackForwardCacheBrowserTestWithNoSupportedFeatures
: public BackForwardCacheBrowserTest {
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
// Specify empty supported features explicitly.
EnableFeatureAndSetParams(features::kBackForwardCache, "supported_features",
"");
BackForwardCacheBrowserTest::SetUpCommandLine(command_line);
}
};

IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestWithNoSupportedFeatures,
DontCacheWithFileSystemAPI) {
ASSERT_TRUE(embedded_test_server()->Start());
GURL url_a(embedded_test_server()->GetURL("/fileapi/request_test.html"));
GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));

// 1) Navigate to the page A with WebFileSystem usage.
EXPECT_TRUE(NavigateToURL(shell(), url_a));
RenderFrameHostImpl* rfh_a1 = current_frame_host();
RenderFrameDeletedObserver deleted_a1(rfh_a1);

// 2) Navigate away.
EXPECT_TRUE(NavigateToURL(shell(), url_b));
EXPECT_TRUE(deleted_a1.deleted());

// 3) Go back to the page A
web_contents()->GetController().GoBack();
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
ExpectNotRestored(
{BackForwardCacheMetrics::NotRestoredReason::kBlocklistedFeatures},
FROM_HERE);
ExpectBlocklistedFeature(
blink::scheduler::WebSchedulerTrackedFeature::kWebFileSystem, FROM_HERE);

RenderFrameHostImpl* rfh_a2 = current_frame_host();
RenderFrameDeletedObserver deleted_a2(rfh_a2);

// 4) Use WebLock
EXPECT_TRUE(ExecJs(rfh_a2, R"(
const never_resolved = new Promise(resolve => {});
new Promise(continue_test => {
navigator.locks.request('test', async () => {
continue_test();
await never_resolved;
});
})
)"));

// 5) Navigate away again.
EXPECT_TRUE(NavigateToURL(shell(), url_b));
EXPECT_TRUE(deleted_a2.deleted());

// 6) Go back to the page A again.
web_contents()->GetController().GoBack();
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
ExpectNotRestored(
{BackForwardCacheMetrics::NotRestoredReason::kBlocklistedFeatures},
FROM_HERE);
ExpectBlocklistedFeatures(
{blink::scheduler::WebSchedulerTrackedFeature::kWebFileSystem,
blink::scheduler::WebSchedulerTrackedFeature::kWebLocks},
FROM_HERE);
}

} // namespace content
2 changes: 1 addition & 1 deletion content/browser/renderer_host/back_forward_cache_can_store_document_result.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ std::string DescribeFeatures(uint64_t blocklisted_features) {
for (size_t i = 0;
i <= static_cast<size_t>(WebSchedulerTrackedFeature::kMaxValue); ++i) {
if (blocklisted_features & (1 << i)) {
features.push_back(blink::scheduler::FeatureToHumanReadableString(
features.push_back(blink::scheduler::FeatureToString(
static_cast<WebSchedulerTrackedFeature>(i)));
}
}
Expand Down
27 changes: 0 additions & 27 deletions content/browser/renderer_host/back_forward_cache_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,31 +96,6 @@ bool IsFileSystemSupported() {
return file_system_api_supported.Get();
}

uint64_t SupportedFeaturesBitmaskImpl() {
if (!DeviceHasEnoughMemoryForBackForwardCache())
return 0;

static constexpr base::FeatureParam<std::string> supported_features(
&features::kBackForwardCache, "supported_features", "");
std::vector<std::string> tokens =
base::SplitString(supported_features.Get(), ",", base::TRIM_WHITESPACE,
base::SPLIT_WANT_NONEMPTY);
uint64_t mask = 0;
for (const std::string& token : tokens) {
auto feature = blink::scheduler::StringToFeature(token);
DCHECK(feature.has_value()) << "invalid feature string: " << token;
if (feature.has_value()) {
mask |= blink::scheduler::FeatureToBit(feature.value());
}
}
return mask;
}

uint64_t SupportedFeaturesBitmask() {
static uint64_t mask = SupportedFeaturesBitmaskImpl();
return mask;
}

bool IgnoresOutstandingNetworkRequestForTesting() {
if (!DeviceHasEnoughMemoryForBackForwardCache())
return false;
Expand Down Expand Up @@ -212,8 +187,6 @@ uint64_t GetDisallowedFeatures(RenderFrameHostImpl* rfh,
result &= blink::scheduler::StickyFeaturesBitmask();
}

result &= ~SupportedFeaturesBitmask();

return result;
}

Expand Down
1 change: 0 additions & 1 deletion third_party/blink/common/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ source_set("common_unittests_sources") {
"origin_trials/trial_token_validator_unittest.cc",
"page/content_to_visible_time_reporter_unittest.cc",
"page_state/page_state_serialization_unittest.cc",
"scheduler/web_scheduler_tracked_feature_unittest.cc",
"service_worker/service_worker_scope_match_unittest.cc",
"test/run_all_unittests.cc",
"tokens/multi_token_unittest.cc",
Expand Down
Loading

0 comments on commit 832fa81

Please sign in to comment.