Introduce Inline Script Cache [chromium/src : main]
Takashi Nakayama (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Hiroshige Hayashizaki
- Kouhei Ueno
- Leszek Swirski
- Rakina Zata Amni
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroshige Hayashizaki (Gerrit)
Hiroshige Hayashizaki added 6 comments![]( "Open in Gerrit")
if (auto* cache_host = loader->GetCodeCacheHost(); cache_host) {nit: `if (auto* cache_host = loader->GetCodeCacheHost()) {`
code_cache = cache_host->FetchInlineScriptCacheSync(Probably "empty `mojo_base::BigBuffer`" instead of "valid null"?
(assuming this means https://source.chromium.org/chromium/chromium/src/+/main:mojo/public/cpp/base/big_buffer.h;drc=c409e7c1320a2ab19e4820a19d1378bf0306ddcc;l=94)
---
Not a strong opinion, but I suspect we should return `std::nullopt` (in other words `cached_metadata_handler->SetSerializedCachedMetadata()` shouldn't be called).
Anyway the behavior is the same (its `cached_metadata_` should be nullptr anyway, because `CachedMetadata::CreateFromSerializedData()` returns nullptr for empty vector), but perhaps not relying the validation at `CreateFromSerializedData ` and directly distinguishing non-existing cache entry (`std::nullopt`) and existing but invalid empty cache entry (empty `BigBuffer`) is a little clearer.
uint8_t source_hash_[kStringDigestSize] = {};Can this be `SecureStringDigest`?
DCHECK(!cached_metadata_);`CHECK()` instead of `DCHECK()`. ditto below and other CLs.
return (cached_metadata_) ? cached_metadata_->SerializedData().size() : 0;nit: `(cached_metadata_)` -> `cached_metadata_`
Related details
- Kouhei Ueno
- Leszek Swirski
- Rakina Zata Amni
- Takashi Nakayama
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Takashi Nakayama (Gerrit)
Takashi Nakayama added 5 comments![]( "Open in Gerrit")
if (auto* cache_host = loader->GetCodeCacheHost(); cache_host) {nit: `if (auto* cache_host = loader->GetCodeCacheHost()) {`
Done
code_cache = cache_host->FetchInlineScriptCacheSync(Probably "empty `mojo_base::BigBuffer`" instead of "valid null"?
(assuming this means https://source.chromium.org/chromium/chromium/src/+/main:mojo/public/cpp/base/big_buffer.h;drc=c409e7c1320a2ab19e4820a19d1378bf0306ddcc;l=94)---
Not a strong opinion, but I suspect we should return `std::nullopt` (in other words `cached_metadata_handler->SetSerializedCachedMetadata()` shouldn't be called).Anyway the behavior is the same (its `cached_metadata_` should be nullptr anyway, because `CachedMetadata::CreateFromSerializedData()` returns nullptr for empty vector), but perhaps not relying the validation at `CreateFromSerializedData ` and directly distinguishing non-existing cache entry (`std::nullopt`) and existing but invalid empty cache entry (empty `BigBuffer`) is a little clearer.
Done; Changed `FetchInlineScriptCacheSync()` to return `std::nullopt` when cache fetch timed out in the parent CL while keeping an empty `BigBuffer` is still passed to `SetSerializedCachedMetadata()` to align with other caches. PTAL
Can this be `SecureStringDigest`?
Done
`CHECK()` instead of `DCHECK()`. ditto below and other CLs.
Done
return (cached_metadata_) ? cached_metadata_->SerializedData().size() : 0;Takashi Nakayamanit: `(cached_metadata_)` -> `cached_metadata_`
Done
Related details
- Hiroshige Hayashizaki
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |