Commit bb924e18 authored by yhirano's avatar yhirano Committed by Commit bot

Separate preaload matching from MemoryCache

This change decouples preload matching from MemoryCache lookup. With this change,
preload matching is done for ResourceFetcher::prealods_.

This is the first step and part of the second step in the design document[1].

1: https://docs.google.com/document/d/1oq8ixPnaDxuAlKUTRQ3WoYHlJenVkzNU9xnkuZX_dWM/edit#

BUG=695939

Review-Url: https://codereview.chromium.org/2834733003
Cr-Commit-Position: refs/heads/master@{#469046}
parent 47f9431d
nick.jpg has MIME type image/jpeg nick.jpg has MIME type image/jpeg
nick.jpg has MIME type image/jpeg nick.jpg has MIME type image/jpeg
nick.jpg has MIME type image/jpeg
This test verifies that an image which is prefetched, and which is also contained as a subresource of the current document can be loaded correctly as a subresource. See bug 49236 in which this wasn't working. This test verifies that an image which is prefetched, and which is also contained as a subresource of the current document can be loaded correctly as a subresource. See bug 49236 in which this wasn't working.
When this test succeeds, you'll see an image of Nick on a sailboat immediately below this text. When this test fails, you will see no images at all. When this test succeeds, you'll see an image of Nick on a sailboat immediately below this text. When this test fails, you will see no images at all.
......
...@@ -17,8 +17,6 @@ window.addEventListener("load", t.step_func(function() { ...@@ -17,8 +17,6 @@ window.addEventListener("load", t.step_func(function() {
assert_false(internals.isPreloaded('../resources/Ahem.ttf'), "fonts should not be preloaded"); assert_false(internals.isPreloaded('../resources/Ahem.ttf'), "fonts should not be preloaded");
assert_false(internals.isPreloaded('../resources/dummy.css'), "css should not be preloaded"); assert_false(internals.isPreloaded('../resources/dummy.css'), "css should not be preloaded");
assert_false(internals.isPreloaded('../resources/square.png'), "imgs should not be preloaded"); assert_false(internals.isPreloaded('../resources/square.png'), "imgs should not be preloaded");
assert_true(internals.isPreloaded('../resources/testharness.js'));
assert_true(internals.isPreloaded('../resources/testharnessreport.js'));
t.done(); t.done();
} }
})); }));
......
...@@ -22,6 +22,9 @@ function runTest() { ...@@ -22,6 +22,9 @@ function runTest() {
// Right now there is a bug that srcset does not properly deal with dynamic changes to the scale factor, // Right now there is a bug that srcset does not properly deal with dynamic changes to the scale factor,
// so to work around that, we must reload the page to get the new image. // so to work around that, we must reload the page to get the new image.
sessionStorage.pageReloaded = true; sessionStorage.pageReloaded = true;
if (window.internals) {
internals.evictAllResources();
}
document.location.reload(true); document.location.reload(true);
} }
} }
......
...@@ -6,6 +6,7 @@ MockContentSettingsClient: allowImage((file test):permissionclient/resources/bos ...@@ -6,6 +6,7 @@ MockContentSettingsClient: allowImage((file test):permissionclient/resources/bos
MockContentSettingsClient: allowImage((file test):permissionclient/resources/boston.gif): false MockContentSettingsClient: allowImage((file test):permissionclient/resources/boston.gif): false
MockContentSettingsClient: allowImage((file test):permissionclient/resources/boston.gif): false MockContentSettingsClient: allowImage((file test):permissionclient/resources/boston.gif): false
MockContentSettingsClient: allowImage((file test):permissionclient/resources/boston.gif): false MockContentSettingsClient: allowImage((file test):permissionclient/resources/boston.gif): false
MockContentSettingsClient: allowImage((file test):permissionclient/resources/boston.gif): false
Blocked images can be reloaded, so neither onload nor onerror is called. Only check here that onload is never called when image is blocked. Blocked images can be reloaded, so neither onload nor onerror is called. Only check here that onload is never called when image is blocked.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
......
...@@ -217,11 +217,6 @@ Resource* DocumentLoader::StartPreload(Resource::Type type, ...@@ -217,11 +217,6 @@ Resource* DocumentLoader::StartPreload(Resource::Type type,
NOTREACHED(); NOTREACHED();
} }
// CSP layout tests verify that preloads are subject to access checks by
// seeing if they are in the `preload started` list. Therefore do not add
// them to the list if the load is immediately denied.
if (resource && !resource->GetResourceError().IsAccessCheck())
Fetcher()->PreloadStarted(resource);
return resource; return resource;
} }
......
...@@ -127,7 +127,7 @@ TEST_P(LinkLoaderPreloadTest, Preload) { ...@@ -127,7 +127,7 @@ TEST_P(LinkLoaderPreloadTest, Preload) {
ASSERT_EQ(1, fetcher->CountPreloads()); ASSERT_EQ(1, fetcher->CountPreloads());
Resource* resource = loader->LinkPreloadedResourceForTesting(); Resource* resource = loader->LinkPreloadedResourceForTesting();
ASSERT_NE(resource, nullptr); ASSERT_NE(resource, nullptr);
EXPECT_TRUE(fetcher->ContainsAsPreloadForTesting(resource)); EXPECT_TRUE(fetcher->ContainsAsPreload(resource));
EXPECT_EQ(test_case.priority, resource->GetResourceRequest().Priority()); EXPECT_EQ(test_case.priority, resource->GetResourceRequest().Priority());
EXPECT_EQ(test_case.context, EXPECT_EQ(test_case.context,
resource->GetResourceRequest().GetRequestContext()); resource->GetResourceRequest().GetRequestContext());
......
...@@ -40,6 +40,7 @@ source_set("loader") { ...@@ -40,6 +40,7 @@ source_set("loader") {
"fetch/IntegrityMetadata.h", "fetch/IntegrityMetadata.h",
"fetch/MemoryCache.cpp", "fetch/MemoryCache.cpp",
"fetch/MemoryCache.h", "fetch/MemoryCache.h",
"fetch/PreloadKey.h",
"fetch/RawResource.cpp", "fetch/RawResource.cpp",
"fetch/RawResource.h", "fetch/RawResource.h",
"fetch/Resource.cpp", "fetch/Resource.cpp",
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef PreloadKey_h
#define PreloadKey_h
#include "platform/loader/fetch/Resource.h"
#include "platform/weborigin/KURL.h"
#include "platform/weborigin/KURLHash.h"
#include "platform/wtf/HashTraits.h"
namespace blink {
// PreloadKey is a key type of the prealods map in a fetch group (a.k.a.
// blink::ResourceFetcher).
struct PreloadKey final {
public:
struct Hash {
STATIC_ONLY(Hash);
public:
static unsigned GetHash(const PreloadKey& key) {
return KURLHash::GetHash(key.url);
}
static bool Equal(const PreloadKey& x, const PreloadKey& y) {
return x == y;
}
static constexpr bool safe_to_compare_to_empty_or_deleted = false;
};
PreloadKey() = default;
PreloadKey(WTF::HashTableDeletedValueType)
: url(WTF::kHashTableDeletedValue) {}
PreloadKey(const KURL& url, Resource::Type type)
: url(RemoveFragmentFromUrl(url)), type(type) {}
bool IsHashTableDeletedValue() const { return url.IsHashTableDeletedValue(); }
bool operator==(const PreloadKey& x) const {
return url == x.url && type == x.type;
}
static KURL RemoveFragmentFromUrl(const KURL& src) {
if (!src.HasFragmentIdentifier())
return src;
KURL url = src;
url.RemoveFragmentIdentifier();
return url;
}
KURL url;
Resource::Type type = Resource::kMainResource;
};
} // namespace blink
namespace WTF {
template <>
struct DefaultHash<blink::PreloadKey> {
using Hash = blink::PreloadKey::Hash;
};
template <>
struct HashTraits<blink::PreloadKey>
: public SimpleClassHashTraits<blink::PreloadKey> {};
} // namespace WTF
#endif // PreloadKey_h
...@@ -33,6 +33,7 @@ ...@@ -33,6 +33,7 @@
#include "platform/loader/fetch/FetchContext.h" #include "platform/loader/fetch/FetchContext.h"
#include "platform/loader/fetch/FetchInitiatorInfo.h" #include "platform/loader/fetch/FetchInitiatorInfo.h"
#include "platform/loader/fetch/FetchParameters.h" #include "platform/loader/fetch/FetchParameters.h"
#include "platform/loader/fetch/PreloadKey.h"
#include "platform/loader/fetch/Resource.h" #include "platform/loader/fetch/Resource.h"
#include "platform/loader/fetch/ResourceError.h" #include "platform/loader/fetch/ResourceError.h"
#include "platform/loader/fetch/ResourceLoadPriority.h" #include "platform/loader/fetch/ResourceLoadPriority.h"
...@@ -106,7 +107,6 @@ class PLATFORM_EXPORT ResourceFetcher ...@@ -106,7 +107,6 @@ class PLATFORM_EXPORT ResourceFetcher
int CountPreloads() const { return preloads_.size(); } int CountPreloads() const { return preloads_.size(); }
void ClearPreloads(ClearPreloadsPolicy = kClearAllPreloads); void ClearPreloads(ClearPreloadsPolicy = kClearAllPreloads);
void PreloadStarted(Resource*);
void LogPreloadStats(ClearPreloadsPolicy); void LogPreloadStats(ClearPreloadsPolicy);
void WarnUnusedPreloads(); void WarnUnusedPreloads();
...@@ -142,9 +142,10 @@ class PLATFORM_EXPORT ResourceFetcher ...@@ -142,9 +142,10 @@ class PLATFORM_EXPORT ResourceFetcher
// Calling this method before main document resource is fetched is invalid. // Calling this method before main document resource is fetched is invalid.
ResourceTimingInfo* GetNavigationTimingInfo(); ResourceTimingInfo* GetNavigationTimingInfo();
bool ContainsAsPreloadForTesting(Resource* resource) const { // Returns whether the given resource is contained as a preloaded resource.
return preloads_.Contains(resource); bool ContainsAsPreload(Resource*) const;
}
void RemovePreload(Resource*);
// Workaround for https://crbug.com/666214. // Workaround for https://crbug.com/666214.
// TODO(hiroshige): Remove this hack. // TODO(hiroshige): Remove this hack.
...@@ -187,6 +188,14 @@ class PLATFORM_EXPORT ResourceFetcher ...@@ -187,6 +188,14 @@ class PLATFORM_EXPORT ResourceFetcher
const ResourceFactory&, const ResourceFactory&,
ResourceRequestBlockedReason); ResourceRequestBlockedReason);
Resource* MatchPreload(const FetchParameters& params, Resource::Type);
void InsertAsPreloadIfNecessary(Resource*,
const FetchParameters& params,
Resource::Type);
bool IsReusableForPreloading(const FetchParameters&,
Resource*,
bool is_static_data) const;
// RevalidationPolicy enum values are used in UMAs https://crbug.com/579496. // RevalidationPolicy enum values are used in UMAs https://crbug.com/579496.
enum RevalidationPolicy { kUse, kRevalidate, kReload, kLoad }; enum RevalidationPolicy { kUse, kRevalidate, kReload, kLoad };
RevalidationPolicy DetermineRevalidationPolicy(Resource::Type, RevalidationPolicy DetermineRevalidationPolicy(Resource::Type,
...@@ -230,7 +239,7 @@ class PLATFORM_EXPORT ResourceFetcher ...@@ -230,7 +239,7 @@ class PLATFORM_EXPORT ResourceFetcher
HashSet<String> validated_urls_; HashSet<String> validated_urls_;
mutable DocumentResourceMap document_resources_; mutable DocumentResourceMap document_resources_;
HeapListHashSet<Member<Resource>> preloads_; HeapHashMap<PreloadKey, Member<Resource>> preloads_;
Member<MHTMLArchive> archive_; Member<MHTMLArchive> archive_;
TaskRunnerTimer<ResourceFetcher> resource_timing_report_timer_; TaskRunnerTimer<ResourceFetcher> resource_timing_report_timer_;
......
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