Skip to content

GitLab

T
tangled
Commit 7c08a1a3 authored by rajendrant's avatar rajendrant Committed by Commit Bot
Browse files
Options

Split placeholder image load checks and enables 

Refactor the MaybeAllowImagePlaceholder() and MaybeAllowLazyLoadingImage()
functions to checks and enable the placeholder request bit separately.

This will help with lazyloading of CSS background images.

Bug: 846170
Change-Id: I07dd50db59287c290ff0a7f0cc726dc23ed882e8
Reviewed-on: https://chromium-review.googlesource.com/1178637
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: default avatarScott Little <sclittle@chromium.org>
Reviewed-by: default avatarNate Chapin <japhet@chromium.org>
Reviewed-by: default avatarFredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#584252}
parent 57ffd0ad
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 223 additions and 251 deletions
third_party/WebKit/LayoutTests/http/tests/previews/client-lofi-sprite-expected.txt deleted 100644 → 0
View file @ 57ffd0ad
layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600
layer at (0,0) size 800x176
LayoutBlockFlow {HTML} at (0,0) size 800x176
LayoutBlockFlow {BODY} at (8,8) size 784x160
LayoutBlockFlow {DIV} at (0,0) size 40x40
LayoutBlockFlow {DIV} at (0,40) size 40x40
LayoutBlockFlow {DIV} at (0,80) size 40x40
LayoutBlockFlow {DIV} at (0,120) size 40x40
third_party/WebKit/LayoutTests/http/tests/previews/client-lofi-sprite.html deleted 100644 → 0
View file @ 57ffd0ad
<!DOCTYPE html>
<style>
div {
width: 40px;
height: 40px;
}
</style>
<body>
<!-- This test verifies if CSS sprite image is shown, instead of placeholder image. -->
<div id="sprite1"></div>
<div id="sprite2"></div>
<div id="sprite3"></div>
<div id="sprite4"></div>
<script>
internals.settings.setFetchImagePlaceholders(true);
document.getElementById("sprite1").style.cssText = 'background: url(https://127.0.0.1:8443/resources/square200.png) 0 0;';
document.getElementById("sprite2").style.cssText = 'background: url(https://127.0.0.1:8443/resources/square200.png) 0 10px;';
document.getElementById("sprite3").style.cssText = 'background: url(https://127.0.0.1:8443/resources/square200.png) 10px 0;';
document.getElementById("sprite4").style.cssText = 'background: none, url(https://127.0.0.1:8443/resources/square200.png) 10px 10px;';
</script>
</body>
third_party/blink/renderer/core/css/css_image_set_value.cc
View file @ 7c08a1a3
......@@ -127,8 +127,10 @@ StyleImage* CSSImageSetValue::CacheImage(
}
if (document.GetFrame() &&
placeholder_image_request_type == FetchParameters::kAllowPlaceholder)
document.GetFrame()->MaybeAllowImagePlaceholder(params);
placeholder_image_request_type == FetchParameters::kAllowPlaceholder &&
document.GetFrame()->IsClientLoFiAllowed(params.GetResourceRequest())) {
params.SetClientLoFiPlaceholder();
}
cached_image_ = StyleFetchedImageSet::Create(
ImageResourceContent::Fetch(params, document.Fetcher()),
......
third_party/blink/renderer/core/css/css_image_value.cc
View file @ 7c08a1a3
......@@ -74,8 +74,10 @@ StyleImage* CSSImageValue::CacheImage(
}
if (document.GetFrame() &&
placeholder_image_request_type == FetchParameters::kAllowPlaceholder)
document.GetFrame()->MaybeAllowImagePlaceholder(params);
placeholder_image_request_type == FetchParameters::kAllowPlaceholder &&
document.GetFrame()->IsClientLoFiAllowed(params.GetResourceRequest())) {
params.SetClientLoFiPlaceholder();
}
cached_image_ = StyleFetchedImage::Create(document, params);
}
......
third_party/blink/renderer/core/frame/local_frame.h
View file @ 7c08a1a3
......@@ -73,7 +73,6 @@ class Editor;
class Element;
class EventHandler;
class EventHandlerRegistry;
class FetchParameters;
class FloatSize;
class FrameConsole;
class FrameResourceCoordinator;
......@@ -95,6 +94,7 @@ class Node;
class NodeTraversal;
class PerformanceMonitor;
class PluginData;
class ResourceRequest;
class ScriptController;
class SharedBuffer;
class SmoothScrollSequencer;
......@@ -290,15 +290,11 @@ class CORE_EXPORT LocalFrame final : public Frame,
AdTracker* GetAdTracker() { return ad_tracker_; }
void SetAdTrackerForTesting(AdTracker* ad_tracker);
// Convenience function to allow loading image placeholders for the request if
// either the flag in Settings() for using image placeholders is set, or if
// the embedder decides that Client Lo-Fi should be used for this request.
void MaybeAllowImagePlaceholder(FetchParameters&) const;
// Returns true if Client Lo-Fi should be used for this request.
bool IsClientLoFiAllowed(const ResourceRequest&) const;
// Convenience function to allow loading image placeholders for the request if
// lazyloading the image is possible. Returns if lazyloading the image is
// possible.
bool MaybeAllowLazyLoadingImage(FetchParameters&) const;
// Returns true if lazyloading the image is possible.
bool IsLazyLoadingImageAllowed() const;
// The returned value is a off-heap raw-ptr and should not be stored.
WebURLLoaderFactory* GetURLLoaderFactory();
......
third_party/blink/renderer/core/frame/local_frame_test.cc
View file @ 7c08a1a3
......@@ -12,6 +12,16 @@
namespace blink {
namespace {
void MaybeAllowImagePlaceholder(DummyPageHolder* page_holder,
FetchParameters& params) {
if (page_holder->GetFrame().IsClientLoFiAllowed(params.GetResourceRequest()))
params.SetClientLoFiPlaceholder();
}
} // namespace
class TestLocalFrameClient : public EmptyLocalFrameClient {
public:
explicit TestLocalFrameClient(
......@@ -32,10 +42,10 @@ TEST(LocalFrameTest, MaybeAllowPlaceholderImageUsesSpecifiedRequestValue) {
request1.SetURL(KURL("http://insecure.com"));
request1.SetPreviewsState(WebURLRequest::kClientLoFiOn);
FetchParameters params1(request1);
DummyPageHolder::Create(IntSize(800, 600), nullptr,
new TestLocalFrameClient(WebURLRequest::kPreviewsOff))
->GetFrame()
.MaybeAllowImagePlaceholder(params1);
auto page_holder = DummyPageHolder::Create(
IntSize(800, 600), nullptr,
new TestLocalFrameClient(WebURLRequest::kPreviewsOff));
MaybeAllowImagePlaceholder(page_holder.get(), params1);
EXPECT_EQ(FetchParameters::kAllowPlaceholder,
params1.GetPlaceholderImageRequestType());
......@@ -43,11 +53,10 @@ TEST(LocalFrameTest, MaybeAllowPlaceholderImageUsesSpecifiedRequestValue) {
request2.SetURL(KURL("https://secure.com"));
request2.SetPreviewsState(WebURLRequest::kPreviewsOff);
FetchParameters params2(request2);
DummyPageHolder::Create(
auto page_holder2 = DummyPageHolder::Create(
IntSize(800, 600), nullptr,
new TestLocalFrameClient(WebURLRequest::kClientLoFiOn))
->GetFrame()
.MaybeAllowImagePlaceholder(params2);
new TestLocalFrameClient(WebURLRequest::kClientLoFiOn));
MaybeAllowImagePlaceholder(page_holder2.get(), params2);
EXPECT_EQ(FetchParameters::kDisallowPlaceholder,
params2.GetPlaceholderImageRequestType());
}
......@@ -60,7 +69,7 @@ TEST(LocalFrameTest, MaybeAllowPlaceholderImageUsesFramePreviewsState) {
std::unique_ptr<DummyPageHolder> page_holder = DummyPageHolder::Create(
IntSize(800, 600), nullptr,
new TestLocalFrameClient(WebURLRequest::kClientLoFiOn));
page_holder->GetFrame().MaybeAllowImagePlaceholder(params1);
MaybeAllowImagePlaceholder(page_holder.get(), params1);
EXPECT_EQ(FetchParameters::kAllowPlaceholder,
params1.GetPlaceholderImageRequestType());
EXPECT_TRUE(page_holder->GetFrame().IsUsingDataSavingPreview());
......@@ -72,7 +81,7 @@ TEST(LocalFrameTest, MaybeAllowPlaceholderImageUsesFramePreviewsState) {
std::unique_ptr<DummyPageHolder> page_holder2 = DummyPageHolder::Create(
IntSize(800, 600), nullptr,
new TestLocalFrameClient(WebURLRequest::kServerLitePageOn));
page_holder2->GetFrame().MaybeAllowImagePlaceholder(params2);
MaybeAllowImagePlaceholder(page_holder2.get(), params2);
EXPECT_EQ(FetchParameters::kDisallowPlaceholder,
params2.GetPlaceholderImageRequestType());
EXPECT_FALSE(page_holder2->GetFrame().IsUsingDataSavingPreview());
......@@ -84,12 +93,11 @@ TEST(LocalFrameTest,
request1.SetURL(KURL("https://secure.com"));
request1.SetPreviewsState(WebURLRequest::kPreviewsUnspecified);
FetchParameters params1(request1);
DummyPageHolder::Create(
auto page_holder = DummyPageHolder::Create(
IntSize(800, 600), nullptr,
new TestLocalFrameClient(WebURLRequest::kServerLoFiOn |
WebURLRequest::kClientLoFiOn))
->GetFrame()
.MaybeAllowImagePlaceholder(params1);
WebURLRequest::kClientLoFiOn));
MaybeAllowImagePlaceholder(page_holder.get(), params1);
EXPECT_EQ(FetchParameters::kAllowPlaceholder,
params1.GetPlaceholderImageRequestType());
......@@ -97,12 +105,11 @@ TEST(LocalFrameTest,
request2.SetURL(KURL("http://insecure.com"));
request2.SetPreviewsState(WebURLRequest::kPreviewsUnspecified);
FetchParameters params2(request2);
DummyPageHolder::Create(
auto page_holder2 = DummyPageHolder::Create(
IntSize(800, 600), nullptr,
new TestLocalFrameClient(WebURLRequest::kServerLoFiOn |
WebURLRequest::kClientLoFiOn))
->GetFrame()
.MaybeAllowImagePlaceholder(params2);
WebURLRequest::kClientLoFiOn));
MaybeAllowImagePlaceholder(page_holder2.get(), params2);
EXPECT_EQ(FetchParameters::kDisallowPlaceholder,
params2.GetPlaceholderImageRequestType());
}
......
third_party/blink/renderer/core/frame/settings.json5
View file @ 7c08a1a3
......@@ -893,12 +893,6 @@
initial: false,
},
// Whether or not to issue range requests for images and show placeholders.
{
name: "fetchImagePlaceholders",
initial: false,
},
// Whether the frame is a presentation receiver and should expose
// `navigator.presentation.receiver`.
{
......
third_party/blink/renderer/core/loader/document_loader.cc
View file @ 7c08a1a3
......@@ -218,8 +218,11 @@ Resource* DocumentLoader::StartPreload(Resource::Type type,
switch (type) {
case Resource::kImage:
if (frame_) {
frame_->MaybeAllowImagePlaceholder(params);
frame_->MaybeAllowLazyLoadingImage(params);
if (frame_->IsClientLoFiAllowed(params.GetResourceRequest())) {
params.SetClientLoFiPlaceholder();
} else if (frame_->IsLazyLoadingImageAllowed()) {
params.SetAllowImagePlaceholder();
}
}
resource = ImageResource::Fetch(params, Fetcher());
break;
......
third_party/blink/renderer/core/loader/image_loader.cc
View file @ 7c08a1a3
......@@ -443,11 +443,14 @@ void ImageLoader::DoUpdateFromElement(BypassMainWorldBehavior bypass_behavior,
if (update_behavior != kUpdateForcedReload &&
lazy_image_load_state_ == LazyImageLoadState::kNone) {
const auto* frame = document.GetFrame();
frame->MaybeAllowImagePlaceholder(params);
auto* html_image = ToHTMLImageElementOrNull(GetElement());
if (html_image && html_image->ElementCreatedByParser() &&
frame->MaybeAllowLazyLoadingImage(params)) {
lazy_image_load_state_ = LazyImageLoadState::kDeferred;
if (frame->IsClientLoFiAllowed(params.GetResourceRequest())) {
params.SetClientLoFiPlaceholder();
} else if (auto* html_image = ToHTMLImageElementOrNull(GetElement())) {
if (html_image->ElementCreatedByParser() &&
frame->IsLazyLoadingImageAllowed()) {
params.SetAllowImagePlaceholder();
lazy_image_load_state_ = LazyImageLoadState::kDeferred;
}
}
}
......
third_party/blink/renderer/platform/loader/fetch/fetch_parameters.cc
View file @ 7c08a1a3
......@@ -111,6 +111,12 @@ void FetchParameters::MakeSynchronous() {
options_.synchronous_policy = kRequestSynchronously;
}
void FetchParameters::SetClientLoFiPlaceholder() {
resource_request_.SetPreviewsState(resource_request_.GetPreviewsState() |
WebURLRequest::kClientLoFiOn);
SetAllowImagePlaceholder();
}
void FetchParameters::SetAllowImagePlaceholder() {
DCHECK_EQ(kDisallowPlaceholder, placeholder_image_request_type_);
if (!resource_request_.Url().ProtocolIsInHTTPFamily() ||
......
third_party/blink/renderer/platform/loader/fetch/fetch_parameters.h
View file @ 7c08a1a3
......@@ -178,9 +178,13 @@ class PLATFORM_EXPORT FetchParameters {
// Configures the request to load an image placeholder if the request is
// eligible (e.g. the url's protocol is HTTP, etc.). If this request is
// non-eligible, this method doesn't modify the ResourceRequest. Calling this
// method sets m_placeholderImageRequestType to the appropriate value.
// method sets placeholder_image_request_type_ to the appropriate value.
void SetAllowImagePlaceholder();
// Configures the request to load an image as a placeholder and sets the
// Client LoFi preview bit.
void SetClientLoFiPlaceholder();
private:
ResourceRequest resource_request_;
// |decoder_options_|'s ContentType is set to |kPlainTextContent| in
......
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