Skip to content

Commit a7536de

Browse files
trflynn89trflynn89
committed
LibHTTP+LibWebView+RequestServer: Remove the partitioned disk cache mode
This was added specifically for WebDriver browsing sessions, but it is now effectively unused since profiles were added: 257d355 We now instead create a unique profile for each browser process launched by WebDriver. The profiles provide the isolation we need for multiple RequestServer processes.
1 parent 41ebdb3 commit a7536de

9 files changed

Lines changed: 30 additions & 54 deletions

File tree

‎Libraries/LibHTTP/Cache/DiskCache.cpp‎

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -44,37 +44,35 @@ static constexpr StringView cache_directory_for_mode(DiskCache::Mode mode)
4444
switch (mode) {
4545
case DiskCache::Mode::Normal:
4646
return "Cache"sv;
47-
case DiskCache::Mode::Partitioned:
48-
// FIXME: Ideally, we could support multiple RequestServer processes using the same database by setting a
49-
// reasonable busy timeout. We would also have to prevent multiple processes writing to the same cache
50-
// entry file at the same time with some interprocess locking mechanism.
51-
return "PartitionedCache"sv;
5247
case DiskCache::Mode::Testing:
5348
return "TestCache"sv;
5449
}
5550
VERIFY_NOT_REACHED();
5651
}
5752

58-
ErrorOr<DiskCache> DiskCache::create(Mode mode, LexicalPath cache_root)
53+
ErrorOr<Optional<DiskCache>> DiskCache::create(Mode mode, LexicalPath const& cache_root)
5954
{
6055
auto cache_directory = cache_root.append(cache_directory_for_mode(mode));
56+
57+
// Start with a clean slate in test mode. The index is in-memory, but the cache files themselves are leftover.
58+
if (mode == Mode::Testing)
59+
(void)FileSystem::remove(cache_directory.string(), FileSystem::RecursionMode::Allowed);
60+
6161
TRY(Core::Directory::create(cache_directory, Core::Directory::CreateDirectories::Yes));
6262

6363
auto database = mode == DiskCache::Mode::Normal
6464
? TRY(Database::Database::create(cache_directory.string(), INDEX_DATABASE))
6565
: TRY(Database::Database::create_memory_backed());
6666

6767
if (TRY(CacheIndex::migrate_schema(*database)) == Database::MigrationOutcome::DatabaseTooNew) {
68-
// Entry file names are derived from the cache key, so writing into the existing cache
69-
// directory without its index would corrupt entries the newer build's index refers to.
70-
// Partitioned mode is already a per-process transient cache in its own directory.
71-
// (A fresh memory-backed database always migrates, so this is necessarily Mode::Normal.)
72-
dbgln("Disk cache index was created by a newer Ladybird version; using a transient cache this session");
73-
return create(Mode::Partitioned, move(cache_root));
68+
// Entry file names are derived from the cache key, so writing into the existing cache directory without its
69+
// index would corrupt entries the newer build's index refers to. (A fresh memory-backed database always
70+
// migrates, so this is necessarily Mode::Normal.)
71+
dbgln("Disk cache index was created by a newer Ladybird version; disabling disk cache for this session");
72+
return OptionalNone {};
7473
}
7574

7675
auto index = TRY(CacheIndex::create(database, cache_directory));
77-
7876
return DiskCache { mode, move(database), move(cache_directory), move(index) };
7977
}
8078

@@ -84,19 +82,12 @@ DiskCache::DiskCache(Mode mode, NonnullRefPtr<Database::Database> database, Lexi
8482
, m_cache_directory(move(cache_directory))
8583
, m_index(move(index))
8684
{
87-
if (m_mode == Mode::Partitioned)
88-
m_partitioned_cache_key = String::number(Core::System::getpid());
8985
}
9086

9187
DiskCache::DiskCache(DiskCache&&) = default;
9288
DiskCache& DiskCache::operator=(DiskCache&&) = default;
9389

94-
DiskCache::~DiskCache()
95-
{
96-
// Clean up cache directories in testing modes to prevent endless growth of disk usage.
97-
if (m_mode != Mode::Normal)
98-
remove_entries_accessed_since(UnixDateTime::earliest());
99-
}
90+
DiskCache::~DiskCache() = default;
10091

10192
Variant<Optional<CacheEntryWriter&>, DiskCache::CacheHasOpenEntry> DiskCache::create_entry(CacheRequest& request, URL::URL const& url, StringView method, HeaderList const& request_headers, UnixDateTime request_start_time)
10293
{
@@ -109,7 +100,7 @@ Variant<Optional<CacheEntryWriter&>, DiskCache::CacheHasOpenEntry> DiskCache::cr
109100
}
110101

111102
auto serialized_url = serialize_url_for_cache_storage(url);
112-
auto cache_key = create_cache_key(serialized_url, method, m_partitioned_cache_key);
103+
auto cache_key = create_cache_key(serialized_url, method);
113104

114105
if (check_if_cache_has_open_entry(request, cache_key, url, CheckReaderEntries::Yes))
115106
return CacheHasOpenEntry {};
@@ -139,7 +130,7 @@ Variant<Optional<CacheEntryReader&>, DiskCache::CacheHasOpenEntry> DiskCache::op
139130
return Optional<CacheEntryReader&> {};
140131

141132
auto serialized_url = serialize_url_for_cache_storage(url);
142-
auto cache_key = create_cache_key(serialized_url, method, m_partitioned_cache_key);
133+
auto cache_key = create_cache_key(serialized_url, method);
143134

144135
if (check_if_cache_has_open_entry(request, cache_key, url, open_mode == OpenMode::Read ? CheckReaderEntries::No : CheckReaderEntries::Yes))
145136
return CacheHasOpenEntry {};
@@ -238,7 +229,7 @@ ErrorOr<bool> DiskCache::create_synthetic_entry(URL::URL const& url, StringView
238229
return false;
239230

240231
auto serialized_url = serialize_url_for_cache_storage(url);
241-
auto cache_key = create_cache_key(serialized_url, method, m_partitioned_cache_key);
232+
auto cache_key = create_cache_key(serialized_url, method);
242233
constexpr u64 synthetic_vary_key = 0;
243234

244235
if (m_index.has_entry(cache_key, synthetic_vary_key))
@@ -256,7 +247,7 @@ ErrorOr<bool> DiskCache::store_associated_data(URL::URL const& url, StringView m
256247
return false;
257248

258249
auto serialized_url = serialize_url_for_cache_storage(url);
259-
auto cache_key = create_cache_key(serialized_url, method, m_partitioned_cache_key);
250+
auto cache_key = create_cache_key(serialized_url, method);
260251
if (!vary_key.has_value()) {
261252
auto index_entry = m_index.find_entry(cache_key, request_headers);
262253
if (!index_entry.has_value())
@@ -291,7 +282,7 @@ ErrorOr<Optional<ByteBuffer>> DiskCache::retrieve_associated_data(URL::URL const
291282
return Optional<ByteBuffer> {};
292283

293284
auto serialized_url = serialize_url_for_cache_storage(url);
294-
auto cache_key = create_cache_key(serialized_url, method, m_partitioned_cache_key);
285+
auto cache_key = create_cache_key(serialized_url, method);
295286
if (!vary_key.has_value()) {
296287
auto index_entry = m_index.find_entry(cache_key, request_headers);
297288
if (!index_entry.has_value())
@@ -319,7 +310,7 @@ ErrorOr<Optional<CacheEntryBodyFile>> DiskCache::retrieve_associated_data_file(U
319310
return Optional<CacheEntryBodyFile> {};
320311

321312
auto serialized_url = serialize_url_for_cache_storage(url);
322-
auto cache_key = create_cache_key(serialized_url, method, m_partitioned_cache_key);
313+
auto cache_key = create_cache_key(serialized_url, method);
323314
if (!vary_key.has_value()) {
324315
auto index_entry = m_index.find_entry(cache_key, request_headers);
325316
if (!index_entry.has_value())

‎Libraries/LibHTTP/Cache/DiskCache.h‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,11 @@ class DiskCache {
2828
enum class Mode {
2929
Normal,
3030

31-
// In partitioned mode, the cache is enabled as normal, but each RequestServer process operates with a unique
32-
// disk cache database.
33-
Partitioned,
34-
3531
// In test mode, we only enable caching of responses on a per-request basis, signified by a request header. The
3632
// response headers will include some status on how the request was handled.
3733
Testing,
3834
};
39-
static ErrorOr<DiskCache> create(Mode, LexicalPath cache_root);
35+
static ErrorOr<Optional<DiskCache>> create(Mode, LexicalPath const& cache_root);
4036

4137
DiskCache(DiskCache&&);
4238
DiskCache& operator=(DiskCache&&);
@@ -83,8 +79,6 @@ class DiskCache {
8379
void delete_entry(u64 cache_key, u64 vary_key);
8480

8581
Mode m_mode;
86-
Optional<String> m_partitioned_cache_key;
87-
8882
NonnullRefPtr<Database::Database> m_database;
8983

9084
struct OpenCacheEntry {

‎Libraries/LibHTTP/Cache/Utilities.cpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,12 @@ static u64 serialize_hash(Crypto::Hash::SHA1& hasher)
7272
return result;
7373
}
7474

75-
u64 create_cache_key(StringView url, StringView method, Optional<String const&> extra_cache_key)
75+
u64 create_cache_key(StringView url, StringView method)
7676
{
7777
auto hasher = Crypto::Hash::SHA1::create();
7878
hasher->update(url);
7979
hasher->update(method);
8080

81-
if (extra_cache_key.has_value())
82-
hasher->update(*extra_cache_key);
83-
8481
return serialize_hash(*hasher);
8582
}
8683

‎Libraries/LibHTTP/Cache/Utilities.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ u64 compute_maximum_disk_cache_size(u64 free_bytes, u64 limit_maximum_disk_cache
3737
u64 compute_maximum_disk_cache_entry_size(u64 maximum_disk_cache_size);
3838

3939
String serialize_url_for_cache_storage(URL::URL const&);
40-
u64 create_cache_key(StringView url, StringView method, Optional<String const&> extra_cache_key = {});
40+
u64 create_cache_key(StringView url, StringView method);
4141
u64 create_vary_key(HeaderList const& request_headers, HeaderList const& response_headers);
4242
LexicalPath path_for_cache_entry(LexicalPath const& cache_directory, u64 cache_key, u64 vary_key);
4343
LexicalPath path_for_cache_entry_associated_data(LexicalPath const& cache_directory, u64 cache_key, u64 vary_key, CacheEntryAssociatedData);

‎Libraries/LibWebView/HelperProcess.cpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,6 @@ ErrorOr<NonnullRefPtr<Requests::RequestClient>> launch_request_server_process()
268268
case HTTPDiskCacheMode::Enabled:
269269
arguments.append("enabled"sv);
270270
break;
271-
case HTTPDiskCacheMode::Partitioned:
272-
arguments.append("partitioned"sv);
273-
break;
274271
case HTTPDiskCacheMode::Testing:
275272
arguments.append("testing"sv);
276273
break;

‎Libraries/LibWebView/Options.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ struct BrowserOptions {
100100
enum class HTTPDiskCacheMode {
101101
Disabled,
102102
Enabled,
103-
Partitioned,
104103
Testing,
105104
};
106105

‎Services/RequestServer/main.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,6 @@ ErrorOr<int> ladybird_main(Main::Arguments arguments)
9090
auto mode = TRY([&]() -> ErrorOr<HTTP::DiskCache::Mode> {
9191
if (http_disk_cache_mode == "enabled"sv)
9292
return HTTP::DiskCache::Mode::Normal;
93-
if (http_disk_cache_mode == "partitioned"sv)
94-
return HTTP::DiskCache::Mode::Partitioned;
9593
if (http_disk_cache_mode == "testing"sv)
9694
return HTTP::DiskCache::Mode::Testing;
9795
return Error::from_string_literal("Unrecognized disk cache mode");

‎Tests/LibHTTP/TestDiskCache.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ static HTTP::CacheEntryWriter& create_cache_entry(HTTP::DiskCache& disk_cache, T
6262

6363
TEST_CASE(associated_data_round_trips_with_cache_entry)
6464
{
65-
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root()));
65+
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root())).release_value();
6666
TestCacheRequest request;
6767

6868
auto url = parse_url("https://example.com/script.js"sv);
@@ -94,7 +94,7 @@ TEST_CASE(associated_data_round_trips_with_cache_entry)
9494

9595
TEST_CASE(replacing_cache_entry_removes_associated_data)
9696
{
97-
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root()));
97+
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root())).release_value();
9898
TestCacheRequest request;
9999

100100
auto url = parse_url("https://example.com/script.js"sv);
@@ -125,7 +125,7 @@ TEST_CASE(replacing_cache_entry_removes_associated_data)
125125

126126
TEST_CASE(flush_returns_mappable_body_file)
127127
{
128-
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root()));
128+
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root())).release_value();
129129
TestCacheRequest request;
130130

131131
auto url = parse_url("https://example.com/script.js"sv);
@@ -143,7 +143,7 @@ TEST_CASE(flush_returns_mappable_body_file)
143143

144144
TEST_CASE(replacing_cache_entry_keeps_existing_body_mapping_stable)
145145
{
146-
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root()));
146+
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root())).release_value();
147147
TestCacheRequest request;
148148

149149
auto url = parse_url("https://example.com/script.js"sv);
@@ -170,7 +170,7 @@ TEST_CASE(replacing_cache_entry_keeps_existing_body_mapping_stable)
170170

171171
TEST_CASE(associated_data_round_trips_with_explicit_vary_key)
172172
{
173-
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root()));
173+
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root())).release_value();
174174
TestCacheRequest request;
175175

176176
auto url = parse_url("https://example.com/script.js"sv);
@@ -201,7 +201,7 @@ TEST_CASE(associated_data_round_trips_with_explicit_vary_key)
201201

202202
TEST_CASE(associated_data_participates_in_cache_eviction)
203203
{
204-
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root()));
204+
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing, test_cache_root())).release_value();
205205
TestCacheRequest request;
206206

207207
auto url = parse_url("https://example.com/script.js"sv);

‎Tests/LibWebView/TestProfile.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,18 +262,18 @@ TEST_CASE(profile_caches_are_isolated)
262262
auto bytecode = TRY_OR_FAIL(ByteBuffer::copy("first-profile-bytecode"sv.bytes()));
263263

264264
{
265-
auto cache = TRY_OR_FAIL(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Normal, LexicalPath { first_profile.paths().cache }));
265+
auto cache = TRY_OR_FAIL(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Normal, LexicalPath { first_profile.paths().cache })).release_value();
266266
EXPECT(TRY_OR_FAIL(cache.create_synthetic_entry(url, "GET"sv)));
267267
EXPECT(TRY_OR_FAIL(cache.store_associated_data(url, "GET"sv, *request_headers, {}, HTTP::CacheEntryAssociatedData::JavaScriptBytecode, bytecode.bytes())));
268268
}
269269

270270
{
271-
auto cache = TRY_OR_FAIL(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Normal, LexicalPath { second_profile.paths().cache }));
271+
auto cache = TRY_OR_FAIL(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Normal, LexicalPath { second_profile.paths().cache })).release_value();
272272
EXPECT(!TRY_OR_FAIL(cache.retrieve_associated_data(url, "GET"sv, *request_headers, {}, HTTP::CacheEntryAssociatedData::JavaScriptBytecode)).has_value());
273273
}
274274

275275
{
276-
auto cache = TRY_OR_FAIL(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Normal, LexicalPath { first_profile.paths().cache }));
276+
auto cache = TRY_OR_FAIL(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Normal, LexicalPath { first_profile.paths().cache })).release_value();
277277
auto retrieved_bytecode = TRY_OR_FAIL(cache.retrieve_associated_data(url, "GET"sv, *request_headers, {}, HTTP::CacheEntryAssociatedData::JavaScriptBytecode));
278278
VERIFY(retrieved_bytecode.has_value());
279279
EXPECT_EQ(retrieved_bytecode->bytes(), bytecode.bytes());

0 commit comments

Comments
 (0)