Commit ce8869d8 authored by Guido Urdaneta's avatar Guido Urdaneta Committed by Commit Bot

Revert "Convert ConditionalCacheDeletionHelperBrowserTest to not depend on...

Revert "Convert ConditionalCacheDeletionHelperBrowserTest to not depend on URLRequestContext when network service is enabled."

This reverts commit 56a58d7d.

Reason for revert: FindIt determined that this CL makes ConditionalCacheDeletionHelperBrowserTest.TimeAndURL flaky.
FindIt's analysis looks correct.

Sample failed run:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win%207%20Tests%20x64%20%281%29/45771

Sample logs:
../../content/browser/browsing_data/conditional_cache_deletion_helper_browsertest.cc(107): error: Value of: TestCacheEntry(url)
  Actual: true
Expected: false
Stack trace:
Backtrace:
	testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop [0x00000001409F75B7+87]
	testing::internal::AssertHelper::operator= [0x00000001409F713E+78]
	content::ConditionalCacheDeletionHelperBrowserTest::CompareRemainingKeys [0x000000013FD458FE+510]
	content::ConditionalCacheDeletionHelperBrowserTest_TimeAndURL_Test::RunTestOnMainThread [0x000000013FD4634A+2410]
	content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x0000000141C5413D+445]
	content::ShellBrowserMainParts::PreMainMessageLoopRun [0x0000000142564E24+68]
	content::BrowserMainLoop::PreMainMessageLoopRun [0x0000000140CD4D3E+62]
	content::StartupTaskRunner::RunAllTasksNow [0x00000001410129BB+43]
	content::BrowserMainLoop::CreateStartupTasks [0x0000000140CD3B67+599]
	content::BrowserMainRunnerImpl::Initialize [0x0000000140CD6EDB+107]
	ShellBrowserMain [0x00000001447B04B5+21]
	content::ShellMainDelegate::RunProcess [0x00000001447AED2C+188]
	content::RunBrowserProcessMain [0x0000000140C020E9+89]
	content::ContentMainRunnerImpl::RunServiceManager [0x0000000140C02A1B+219]
	content::ContentMainRunnerImpl::Run [0x0000000140C0290E+238]
	service_manager::Main [0x00000001421C591A+554]
	content::ContentMain [0x0000000140C0202E+62]
	content::BrowserTestBase::SetUp [0x0000000141C53E44+1796]



Original change's description:
> Convert ConditionalCacheDeletionHelperBrowserTest to not depend on URLRequestContext when network service is enabled.
>
> Bug: 837753
> Change-Id: I35b75299bcea1cc2b93a3c47dcbea036cc4e2d03
> Reviewed-on: https://chromium-review.googlesource.com/c/1357567
> Reviewed-by: Clark DuVall <cduvall@chromium.org>
> Commit-Queue: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612858}

TBR=jam@chromium.org,cduvall@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 910891
Change-Id: I541e21663a02cdedae6aa4991d5e8633890ffbbd
Reviewed-on: https://chromium-review.googlesource.com/c/1356946
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: default avatarGuido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613119}
parent c2d9ce86
...@@ -19,28 +19,26 @@ ...@@ -19,28 +19,26 @@
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/cache_test_util.h" #include "content/public/test/cache_test_util.h"
#include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test.h"
#include "content/shell/browser/shell.h" #include "content/shell/browser/shell.h"
#include "net/disk_cache/disk_cache.h" #include "net/disk_cache/disk_cache.h"
#include "net/dns/mock_host_resolver.h"
#include "net/http/http_cache.h" #include "net/http/http_cache.h"
#include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_context_getter.h"
#include "services/network/public/cpp/features.h"
namespace content { namespace content {
namespace { namespace {
bool DeletionCondition(const std::set<GURL>& erase_urls, bool KeyIsEven(const disk_cache::Entry* entry) {
const disk_cache::Entry* entry) { int key_as_int = 0;
return !!erase_urls.count(GURL(entry->GetKey())); base::StringToInt(entry->GetKey().c_str(), &key_as_int);
return (key_as_int % 2) == 0;
} }
bool HasHttpExampleOrigin(const GURL& url) { bool HasHttpsExampleOrigin(const GURL& url) {
return url.host() == "example.com"; return url.GetOrigin() == "https://example.com/";
} }
} // namespace } // namespace
...@@ -48,11 +46,6 @@ bool HasHttpExampleOrigin(const GURL& url) { ...@@ -48,11 +46,6 @@ bool HasHttpExampleOrigin(const GURL& url) {
class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest { class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest {
public: public:
void SetUpOnMainThread() override { void SetUpOnMainThread() override {
host_resolver()->AddRule("*", "127.0.0.1");
ASSERT_TRUE(embedded_test_server()->Start());
if (base::FeatureList::IsEnabled(network::features::kNetworkService))
return;
cache_util_ = std::make_unique<CacheTestUtil>( cache_util_ = std::make_unique<CacheTestUtil>(
content::BrowserContext::GetDefaultStoragePartition( content::BrowserContext::GetDefaultStoragePartition(
shell()->web_contents()->GetBrowserContext())); shell()->web_contents()->GetBrowserContext()));
...@@ -67,18 +60,6 @@ class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest { ...@@ -67,18 +60,6 @@ class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest {
void TearDownOnMainThread() override { cache_util_.reset(); } void TearDownOnMainThread() override { cache_util_.reset(); }
void CreateCacheEntry(const std::set<GURL>& urls) {
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
for (auto& url : urls) {
ASSERT_EQ(net::OK, LoadBasicRequest(
storage_partition()->GetNetworkContext(), url));
}
} else {
for (auto& url : urls)
cache_util_->CreateCacheEntries({url.spec()});
}
}
void DeleteEntries( void DeleteEntries(
const base::Callback<bool(const disk_cache::Entry*)>& condition) { const base::Callback<bool(const disk_cache::Entry*)>& condition) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
...@@ -88,23 +69,12 @@ class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest { ...@@ -88,23 +69,12 @@ class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest {
helper->DeleteAndDestroySelfWhenFinished(done_callback_); helper->DeleteAndDestroySelfWhenFinished(done_callback_);
} }
bool TestCacheEntry(const GURL& url) { void CompareRemainingKeys(std::set<std::string> expected_set) {
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) { std::vector<std::string> remaining_keys = cache_util_->GetEntryKeys();
return LoadBasicRequest(storage_partition()->GetNetworkContext(), url, std::sort(remaining_keys.begin(), remaining_keys.end());
0 /* process_id */, 0 /* render_frame_id */, std::vector<std::string> expected;
net::LOAD_ONLY_FROM_CACHE | expected.assign(expected_set.begin(), expected_set.end());
net::LOAD_SKIP_CACHE_VALIDATION) == net::OK; EXPECT_EQ(expected, remaining_keys);
} else {
return base::ContainsValue(cache_util_->GetEntryKeys(), url.spec());
}
}
void CompareRemainingKeys(const std::set<GURL>& expected_urls,
const std::set<GURL>& erase_urls) {
for (auto& url : expected_urls)
EXPECT_TRUE(TestCacheEntry(url));
for (auto& url : erase_urls)
EXPECT_FALSE(TestCacheEntry(url));
} }
void DoneCallback(int value) { void DoneCallback(int value) {
...@@ -120,15 +90,7 @@ class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest { ...@@ -120,15 +90,7 @@ class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest {
CacheTestUtil* GetCacheTestUtil() { return cache_util_.get(); } CacheTestUtil* GetCacheTestUtil() { return cache_util_.get(); }
StoragePartition* storage_partition() {
return BrowserContext::GetDefaultStoragePartition(browser_context());
}
private: private:
BrowserContext* browser_context() {
return shell()->web_contents()->GetBrowserContext();
}
base::Callback<void(int)> done_callback_; base::Callback<void(int)> done_callback_;
std::unique_ptr<CacheTestUtil> cache_util_; std::unique_ptr<CacheTestUtil> cache_util_;
std::unique_ptr<base::WaitableEvent> waitable_event_; std::unique_ptr<base::WaitableEvent> waitable_event_;
...@@ -137,45 +99,22 @@ class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest { ...@@ -137,45 +99,22 @@ class ConditionalCacheDeletionHelperBrowserTest : public ContentBrowserTest {
// Tests that ConditionalCacheDeletionHelper only deletes those cache entries // Tests that ConditionalCacheDeletionHelper only deletes those cache entries
// that match the condition. // that match the condition.
IN_PROC_BROWSER_TEST_F(ConditionalCacheDeletionHelperBrowserTest, Condition) { IN_PROC_BROWSER_TEST_F(ConditionalCacheDeletionHelperBrowserTest, Condition) {
std::set<GURL> urls = { // Create 5 entries.
embedded_test_server()->GetURL("foo.com", "/title1.html"), std::set<std::string> keys = {"123", "47", "56", "81", "42"};
embedded_test_server()->GetURL("bar.com", "/title1.html"),
embedded_test_server()->GetURL("baz.com", "/title1.html"), GetCacheTestUtil()->CreateCacheEntries(keys);
embedded_test_server()->GetURL("qux.com", "/title1.html")};
// Delete the entries whose keys are even numbers.
CreateCacheEntry(urls); base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::IO},
std::set<GURL> erase_urls = { base::BindOnce(&ConditionalCacheDeletionHelperBrowserTest::DeleteEntries,
embedded_test_server()->GetURL("bar.com", "/title1.html"), base::Unretained(this), base::Bind(&KeyIsEven)));
embedded_test_server()->GetURL("baz.com", "/title1.html"), WaitForTasksOnIOThread();
};
// Expect that the keys with values 56 and 42 were deleted.
for (auto& url : erase_urls) keys.erase("56");
urls.erase(url); keys.erase("42");
CompareRemainingKeys(keys);
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
network::mojom::ClearDataFilterPtr filter =
network::mojom::ClearDataFilter::New();
filter->type = network::mojom::ClearDataFilter::Type::DELETE_MATCHES;
for (auto& url : erase_urls)
filter->origins.push_back(url::Origin::Create(url));
base::RunLoop run_loop;
storage_partition()->GetNetworkContext()->ClearHttpCache(
base::Time(), base::Time::Max(), std::move(filter),
run_loop.QuitClosure());
run_loop.Run();
} else {
base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::IO},
base::BindOnce(
&ConditionalCacheDeletionHelperBrowserTest::DeleteEntries,
base::Unretained(this),
base::Bind(&DeletionCondition, erase_urls)));
WaitForTasksOnIOThread();
}
CompareRemainingKeys(urls, erase_urls);
} }
// Tests that ConditionalCacheDeletionHelper correctly constructs a condition // Tests that ConditionalCacheDeletionHelper correctly constructs a condition
...@@ -195,12 +134,14 @@ IN_PROC_BROWSER_TEST_F(ConditionalCacheDeletionHelperBrowserTest, Condition) { ...@@ -195,12 +134,14 @@ IN_PROC_BROWSER_TEST_F(ConditionalCacheDeletionHelperBrowserTest, Condition) {
#endif #endif
IN_PROC_BROWSER_TEST_F(ConditionalCacheDeletionHelperBrowserTest, IN_PROC_BROWSER_TEST_F(ConditionalCacheDeletionHelperBrowserTest,
MAYBE_TimeAndURL) { MAYBE_TimeAndURL) {
// Create some entries. // Create some entries.
std::set<GURL> urls = { std::set<std::string> keys;
embedded_test_server()->GetURL("foo.com", "/title1.html"), keys.insert("https://google.com/index.html");
embedded_test_server()->GetURL("example.com", "/title1.html"), keys.insert("https://example.com/foo/bar/icon.png");
embedded_test_server()->GetURL("bar.com", "/title1.html")}; keys.insert("http://chrome.com");
CreateCacheEntry(urls);
GetCacheTestUtil()->CreateCacheEntries(keys);
// Wait some milliseconds for the cache to write the entries. // Wait some milliseconds for the cache to write the entries.
// This assures that future entries will have timestamps strictly greater than // This assures that future entries will have timestamps strictly greater than
...@@ -209,53 +150,32 @@ IN_PROC_BROWSER_TEST_F(ConditionalCacheDeletionHelperBrowserTest, ...@@ -209,53 +150,32 @@ IN_PROC_BROWSER_TEST_F(ConditionalCacheDeletionHelperBrowserTest,
base::Time now = base::Time::Now(); base::Time now = base::Time::Now();
// Create a few more entries with a later timestamp. // Create a few more entries with a later timestamp.
std::set<GURL> newer_urls = { std::set<std::string> newer_keys;
embedded_test_server()->GetURL("foo.com", "/simple_page.html"), newer_keys.insert("https://google.com/");
embedded_test_server()->GetURL("example.com", "/title2.html"), newer_keys.insert("https://example.com/foo/bar/icon2.png");
embedded_test_server()->GetURL("example.com", "/title3.html"), newer_keys.insert("https://example.com/foo/bar/icon3.png");
embedded_test_server()->GetURL("example2.com", "/simple_page.html")}; newer_keys.insert("http://example.com/foo/bar/icon4.png");
CreateCacheEntry(newer_urls);
// Create a condition for entries with the "http://example.com" origin
// created after waiting.
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
network::mojom::ClearDataFilterPtr filter =
network::mojom::ClearDataFilter::New();
filter->type = network::mojom::ClearDataFilter::Type::DELETE_MATCHES;
filter->origins.push_back(url::Origin::Create(
embedded_test_server()->GetURL("example.com", "/")));
base::RunLoop run_loop; GetCacheTestUtil()->CreateCacheEntries(newer_keys);
storage_partition()->GetNetworkContext()->ClearHttpCache(
now, base::Time::Max(), std::move(filter), run_loop.QuitClosure());
run_loop.Run();
} else {
base::Callback<bool(const disk_cache::Entry*)> condition =
ConditionalCacheDeletionHelper::CreateURLAndTimeCondition(
base::Bind(&HasHttpExampleOrigin), now, base::Time::Max());
// Delete the entries. // Create a condition for entries with the "https://example.com" origin
base::PostTaskWithTraits( // created after waiting.
FROM_HERE, {BrowserThread::IO}, base::Callback<bool(const disk_cache::Entry*)> condition =
base::BindOnce( ConditionalCacheDeletionHelper::CreateURLAndTimeCondition(
&ConditionalCacheDeletionHelperBrowserTest::DeleteEntries, base::Bind(&HasHttpsExampleOrigin), now, base::Time::Max());
base::Unretained(this), std::move(condition)));
WaitForTasksOnIOThread(); // Delete the entries.
} base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::IO},
// Expect that only "title2.html" and "title3.html" were deleted. base::BindOnce(&ConditionalCacheDeletionHelperBrowserTest::DeleteEntries,
std::set<GURL> erase_urls = { base::Unretained(this), std::move(condition)));
embedded_test_server()->GetURL("example.com", "/title2.html"), WaitForTasksOnIOThread();
embedded_test_server()->GetURL("example.com", "/title3.html"),
}; // Expect that only "icon2.png" and "icon3.png" were deleted.
keys.insert(newer_keys.begin(), newer_keys.end());
for (auto& url : newer_urls) keys.erase("https://example.com/foo/bar/icon2.png");
urls.insert(url); keys.erase("https://example.com/foo/bar/icon3.png");
CompareRemainingKeys(keys);
for (auto& url : erase_urls)
urls.erase(url);
CompareRemainingKeys(urls, erase_urls);
} }
} // namespace content } // namespace content
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment