Commit 664b6639 authored by Dominic Farolino's avatar Dominic Farolino Committed by Commit Bot

Keep Resource::link_preload_ true even when served from the preload

cache

When a blink::Resource is put into the preload cache by a link preload
request, its `link_preload_` member is set to true.

Before this CL, when a blink::Resource in the preload cache is matched
with a request that itself is not a link preload, we reset the
`link_preload_` member to false, obscuring the fact that the Resource
was served from the preload cache to the consumer.

After this CL, we stop resetting this member, so that when a
non-link-preload request is matched with a blink::Resource from the
preload cache, it can determine whether it was originally a preloaded
resource or not. This is a preparation CL for implementing Preload + SRI
to work with each other. The reason that a non-link-preload benefits
from knowing whether it was served from the preload cache can be found
here [1].

[1]: https://docs.google.com/document/d/1l8y_T3z-nb0smR9EVoZ3TeEI6rGzLw74vvzWzODsnDQ/edit#heading=h.rocm6cm98uhw

R=kouhei@chromium.org, yhirano@chromium.org

Bug: 677022
Change-Id: I890ee44cdd650b8459e5220f10a48d1d1192210b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1661314
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: default avatarYoav Weiss <yoavweiss@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669969}
parent ba64a0c7
...@@ -1213,10 +1213,6 @@ blink::mojom::CodeCacheType Resource::ResourceTypeToCodeCacheType( ...@@ -1213,10 +1213,6 @@ blink::mojom::CodeCacheType Resource::ResourceTypeToCodeCacheType(
return ToCodeCacheType(resource_type); return ToCodeCacheType(resource_type);
} }
bool Resource::ShouldBlockLoadEvent() const {
return !link_preload_ && IsLoadEventBlockingResourceType();
}
bool Resource::IsLoadEventBlockingResourceType() const { bool Resource::IsLoadEventBlockingResourceType() const {
switch (type_) { switch (type_) {
case ResourceType::kImage: case ResourceType::kImage:
......
...@@ -247,7 +247,6 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>, ...@@ -247,7 +247,6 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
void SetLoader(ResourceLoader*); void SetLoader(ResourceLoader*);
ResourceLoader* Loader() const { return loader_.Get(); } ResourceLoader* Loader() const { return loader_.Get(); }
bool ShouldBlockLoadEvent() const;
bool IsLoadEventBlockingResourceType() const; bool IsLoadEventBlockingResourceType() const;
// Computes the status of an object after loading. Updates the expire date on // Computes the status of an object after loading. Updates the expire date on
......
...@@ -1028,8 +1028,6 @@ Resource* ResourceFetcher::RequestResource(FetchParameters& params, ...@@ -1028,8 +1028,6 @@ Resource* ResourceFetcher::RequestResource(FetchParameters& params,
ScheduleStaleRevalidate(resource); ScheduleStaleRevalidate(resource);
} }
if (resource->IsLinkPreload() && !params.IsLinkPreload())
resource->SetLinkPreload(false);
break; break;
} }
DCHECK(resource); DCHECK(resource);
...@@ -1910,10 +1908,17 @@ bool ResourceFetcher::StartLoad(Resource* resource) { ...@@ -1910,10 +1908,17 @@ bool ResourceFetcher::StartLoad(Resource* resource) {
loader = loader =
MakeGarbageCollected<ResourceLoader>(this, scheduler_, resource, size); MakeGarbageCollected<ResourceLoader>(this, scheduler_, resource, size);
if (resource->ShouldBlockLoadEvent()) // Preload requests should not block the load event. IsLinkPreload()
// actually continues to return true for Resources matched from the preload
// cache that must block the load event, but that is OK because this method
// is not responsible for promoting matched preloads to load-blocking. This
// is handled by MakePreloadedResourceBlockOnloadIfNeeded().
if (!resource->IsLinkPreload() &&
resource->IsLoadEventBlockingResourceType()) {
loaders_.insert(loader); loaders_.insert(loader);
else } else {
non_blocking_loaders_.insert(loader); non_blocking_loaders_.insert(loader);
}
StorePerformanceTimingInitiatorInformation(resource); StorePerformanceTimingInitiatorInformation(resource);
} }
......
...@@ -661,7 +661,7 @@ TEST_F(ResourceFetcherTest, LinkPreloadResourceAndUse) { ...@@ -661,7 +661,7 @@ TEST_F(ResourceFetcherTest, LinkPreloadResourceAndUse) {
Resource* preload_scanner_resource = Resource* preload_scanner_resource =
MockResource::Fetch(fetch_params_preload_scanner, fetcher, nullptr); MockResource::Fetch(fetch_params_preload_scanner, fetcher, nullptr);
EXPECT_EQ(resource, preload_scanner_resource); EXPECT_EQ(resource, preload_scanner_resource);
EXPECT_FALSE(resource->IsLinkPreload()); EXPECT_TRUE(resource->IsLinkPreload());
// Resource created by parser // Resource created by parser
FetchParameters fetch_params{ResourceRequest(url)}; FetchParameters fetch_params{ResourceRequest(url)};
...@@ -669,7 +669,7 @@ TEST_F(ResourceFetcherTest, LinkPreloadResourceAndUse) { ...@@ -669,7 +669,7 @@ TEST_F(ResourceFetcherTest, LinkPreloadResourceAndUse) {
MakeGarbageCollected<MockResourceClient>(); MakeGarbageCollected<MockResourceClient>();
Resource* new_resource = MockResource::Fetch(fetch_params, fetcher, client); Resource* new_resource = MockResource::Fetch(fetch_params, fetcher, client);
EXPECT_EQ(resource, new_resource); EXPECT_EQ(resource, new_resource);
EXPECT_FALSE(resource->IsLinkPreload()); EXPECT_TRUE(resource->IsLinkPreload());
// DCL reached // DCL reached
fetcher->ClearPreloads(ResourceFetcher::kClearSpeculativeMarkupPreloads); fetcher->ClearPreloads(ResourceFetcher::kClearSpeculativeMarkupPreloads);
...@@ -696,7 +696,7 @@ TEST_F(ResourceFetcherTest, PreloadMatchWithBypassingCache) { ...@@ -696,7 +696,7 @@ TEST_F(ResourceFetcherTest, PreloadMatchWithBypassingCache) {
Resource* second_resource = Resource* second_resource =
MockResource::Fetch(fetch_params_second, fetcher, nullptr); MockResource::Fetch(fetch_params_second, fetcher, nullptr);
EXPECT_EQ(resource, second_resource); EXPECT_EQ(resource, second_resource);
EXPECT_FALSE(resource->IsLinkPreload()); EXPECT_TRUE(resource->IsLinkPreload());
} }
TEST_F(ResourceFetcherTest, CrossFramePreloadMatchIsNotAllowed) { TEST_F(ResourceFetcherTest, CrossFramePreloadMatchIsNotAllowed) {
...@@ -789,7 +789,7 @@ TEST_F(ResourceFetcherTest, RepetitiveSpeculativePreloadShouldBeMerged) { ...@@ -789,7 +789,7 @@ TEST_F(ResourceFetcherTest, RepetitiveSpeculativePreloadShouldBeMerged) {
EXPECT_FALSE(resource1->IsUnusedPreload()); EXPECT_FALSE(resource1->IsUnusedPreload());
} }
TEST_F(ResourceFetcherTest, SpeculativePreloadShouldBePromotedToLinkePreload) { TEST_F(ResourceFetcherTest, SpeculativePreloadShouldBePromotedToLinkPreload) {
auto* fetcher = CreateFetcher(); auto* fetcher = CreateFetcher();
KURL url("http://127.0.0.1:8000/foo.png"); KURL url("http://127.0.0.1:8000/foo.png");
...@@ -824,7 +824,7 @@ TEST_F(ResourceFetcherTest, SpeculativePreloadShouldBePromotedToLinkePreload) { ...@@ -824,7 +824,7 @@ TEST_F(ResourceFetcherTest, SpeculativePreloadShouldBePromotedToLinkePreload) {
EXPECT_EQ(resource1, resource3); EXPECT_EQ(resource1, resource3);
EXPECT_FALSE(fetcher->ContainsAsPreload(resource1)); EXPECT_FALSE(fetcher->ContainsAsPreload(resource1));
EXPECT_FALSE(resource1->IsUnusedPreload()); EXPECT_FALSE(resource1->IsUnusedPreload());
EXPECT_FALSE(resource1->IsLinkPreload()); EXPECT_TRUE(resource1->IsLinkPreload());
} }
TEST_F(ResourceFetcherTest, Revalidate304) { TEST_F(ResourceFetcherTest, Revalidate304) {
......
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