Skip to content

Commit

Permalink
Rename loading distance threshold to margin
Browse files Browse the repository at this point in the history 
This change aligns the lazy loading image observer implementation with
the spec language, reduces confusion as threshold has an entirely
different meaning in the context of intersection observers, and paves
the way for the followup change to use scroll-margin instead of
root-margin for the lazy loading image intersection observer.

Specs:
https://html.spec.whatwg.org/#lazy-load-root-margin

https://www.w3.org/TR/intersection-observer/#dom-intersectionobserverinit-rootmargin

https://www.w3.org/TR/intersection-observer/#dom-intersectionobserverinit-threshold

R=pdr

Bug: 1391989
Low-Coverage-Reason: TRIVIAL_CHANGE no effective coverage change.
Change-Id: Ic1faf75042b20c46f0cea8ee1a9bc7afcc06ee15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5028435
Commit-Queue: Traian Captan <tcaptan@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225081}
  • Loading branch information
@tcaptan-cr
tcaptan-cr authored and Chromium LUCI CQ committed Nov 15, 2023
1 parent ece7f2a commit bacaa80
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 88 deletions.
12 changes: 6 additions & 6 deletions third_party/blink/public/web/web_settings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,12 @@ class WebSettings {
virtual void SetLazyFrameLoadingDistanceThresholdPx2G(int) = 0;
virtual void SetLazyFrameLoadingDistanceThresholdPx3G(int) = 0;
virtual void SetLazyFrameLoadingDistanceThresholdPx4G(int) = 0;
virtual void SetLazyImageLoadingDistanceThresholdPxUnknown(int) = 0;
virtual void SetLazyImageLoadingDistanceThresholdPxOffline(int) = 0;
virtual void SetLazyImageLoadingDistanceThresholdPxSlow2G(int) = 0;
virtual void SetLazyImageLoadingDistanceThresholdPx2G(int) = 0;
virtual void SetLazyImageLoadingDistanceThresholdPx3G(int) = 0;
virtual void SetLazyImageLoadingDistanceThresholdPx4G(int) = 0;
virtual void SetLazyLoadingImageMarginPxUnknown(int) = 0;
virtual void SetLazyLoadingImageMarginPxOffline(int) = 0;
virtual void SetLazyLoadingImageMarginPxSlow2G(int) = 0;
virtual void SetLazyLoadingImageMarginPx2G(int) = 0;
virtual void SetLazyLoadingImageMarginPx3G(int) = 0;
virtual void SetLazyLoadingImageMarginPx4G(int) = 0;
virtual void SetForceDarkModeEnabled(bool) = 0;
virtual void SetPreferredColorScheme(blink::mojom::PreferredColorScheme) = 0;
virtual void SetPreferredContrast(mojom::PreferredContrast) = 0;
Expand Down
30 changes: 12 additions & 18 deletions third_party/blink/renderer/core/exported/web_settings_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -728,34 +728,28 @@ void WebSettingsImpl::SetLazyFrameLoadingDistanceThresholdPx4G(
settings_->SetLazyFrameLoadingDistanceThresholdPx4G(distance_px);
}

void WebSettingsImpl::SetLazyImageLoadingDistanceThresholdPxUnknown(
int distance_px) {
settings_->SetLazyImageLoadingDistanceThresholdPxUnknown(distance_px);
void WebSettingsImpl::SetLazyLoadingImageMarginPxUnknown(int distance_px) {
settings_->SetLazyLoadingImageMarginPxUnknown(distance_px);
}

void WebSettingsImpl::SetLazyImageLoadingDistanceThresholdPxOffline(
int distance_px) {
settings_->SetLazyImageLoadingDistanceThresholdPxOffline(distance_px);
void WebSettingsImpl::SetLazyLoadingImageMarginPxOffline(int distance_px) {
settings_->SetLazyLoadingImageMarginPxOffline(distance_px);
}

void WebSettingsImpl::SetLazyImageLoadingDistanceThresholdPxSlow2G(
int distance_px) {
settings_->SetLazyImageLoadingDistanceThresholdPxSlow2G(distance_px);
void WebSettingsImpl::SetLazyLoadingImageMarginPxSlow2G(int distance_px) {
settings_->SetLazyLoadingImageMarginPxSlow2G(distance_px);
}

void WebSettingsImpl::SetLazyImageLoadingDistanceThresholdPx2G(
int distance_px) {
settings_->SetLazyImageLoadingDistanceThresholdPx2G(distance_px);
void WebSettingsImpl::SetLazyLoadingImageMarginPx2G(int distance_px) {
settings_->SetLazyLoadingImageMarginPx2G(distance_px);
}

void WebSettingsImpl::SetLazyImageLoadingDistanceThresholdPx3G(
int distance_px) {
settings_->SetLazyImageLoadingDistanceThresholdPx3G(distance_px);
void WebSettingsImpl::SetLazyLoadingImageMarginPx3G(int distance_px) {
settings_->SetLazyLoadingImageMarginPx3G(distance_px);
}

void WebSettingsImpl::SetLazyImageLoadingDistanceThresholdPx4G(
int distance_px) {
settings_->SetLazyImageLoadingDistanceThresholdPx4G(distance_px);
void WebSettingsImpl::SetLazyLoadingImageMarginPx4G(int distance_px) {
settings_->SetLazyLoadingImageMarginPx4G(distance_px);
}

void WebSettingsImpl::SetForceDarkModeEnabled(bool enabled) {
Expand Down
12 changes: 6 additions & 6 deletions third_party/blink/renderer/core/exported/web_settings_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,12 @@ class CORE_EXPORT WebSettingsImpl final : public WebSettings {
void SetLazyFrameLoadingDistanceThresholdPx2G(int) override;
void SetLazyFrameLoadingDistanceThresholdPx3G(int) override;
void SetLazyFrameLoadingDistanceThresholdPx4G(int) override;
void SetLazyImageLoadingDistanceThresholdPxUnknown(int) override;
void SetLazyImageLoadingDistanceThresholdPxOffline(int) override;
void SetLazyImageLoadingDistanceThresholdPxSlow2G(int) override;
void SetLazyImageLoadingDistanceThresholdPx2G(int) override;
void SetLazyImageLoadingDistanceThresholdPx3G(int) override;
void SetLazyImageLoadingDistanceThresholdPx4G(int) override;
void SetLazyLoadingImageMarginPxUnknown(int) override;
void SetLazyLoadingImageMarginPxOffline(int) override;
void SetLazyLoadingImageMarginPxSlow2G(int) override;
void SetLazyLoadingImageMarginPx2G(int) override;
void SetLazyLoadingImageMarginPx3G(int) override;
void SetLazyLoadingImageMarginPx4G(int) override;

void SetForceDarkModeEnabled(bool) override;
void SetPreferredColorScheme(mojom::blink::PreferredColorScheme) override;
Expand Down
14 changes: 7 additions & 7 deletions third_party/blink/renderer/core/frame/settings.json5
View file
Original file line number Diff line number Diff line change
Expand Up @@ -977,35 +977,35 @@
},

//
// Lazy image loading distance-from-viewport thresholds for different effective connection types.
// Lazy loading image margins for different effective connection types.
//
{
name: "lazyImageLoadingDistanceThresholdPxUnknown",
name: "lazyLoadingImageMarginPxUnknown",
initial: 3000,
type: "int",
},
{
name: "lazyImageLoadingDistanceThresholdPxOffline",
name: "lazyLoadingImageMarginPxOffline",
initial: 8000,
type: "int",
},
{
name: "lazyImageLoadingDistanceThresholdPxSlow2G",
name: "lazyLoadingImageMarginPxSlow2G",
initial: 8000,
type: "int",
},
{
name: "lazyImageLoadingDistanceThresholdPx2G",
name: "lazyLoadingImageMarginPx2G",
initial: 6000,
type: "int",
},
{
name: "lazyImageLoadingDistanceThresholdPx3G",
name: "lazyLoadingImageMarginPx3G",
initial: 2500,
type: "int",
},
{
name: "lazyImageLoadingDistanceThresholdPx4G",
name: "lazyLoadingImageMarginPx4G",
initial: 1250,
type: "int",
},
Expand Down
71 changes: 39 additions & 32 deletions third_party/blink/renderer/core/html/lazy_load_image_observer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,6 @@ namespace blink {

namespace {

int GetLazyImageLoadingViewportDistanceThresholdPx(const Document& document) {
const Settings* settings = document.GetSettings();
if (!settings)
return 0;

switch (GetNetworkStateNotifier().EffectiveType()) {
case WebEffectiveConnectionType::kTypeUnknown:
return settings->GetLazyImageLoadingDistanceThresholdPxUnknown();
case WebEffectiveConnectionType::kTypeOffline:
return settings->GetLazyImageLoadingDistanceThresholdPxOffline();
case WebEffectiveConnectionType::kTypeSlow2G:
return settings->GetLazyImageLoadingDistanceThresholdPxSlow2G();
case WebEffectiveConnectionType::kType2G:
return settings->GetLazyImageLoadingDistanceThresholdPx2G();
case WebEffectiveConnectionType::kType3G:
return settings->GetLazyImageLoadingDistanceThresholdPx3G();
case WebEffectiveConnectionType::kType4G:
return settings->GetLazyImageLoadingDistanceThresholdPx4G();
}
NOTREACHED();
return 0;
}

// Returns if the element or its ancestors are invisible, due to their style or
// attribute or due to themselves not connected to the main document tree.
bool IsElementInInvisibleSubTree(const Element& element) {
Expand Down Expand Up @@ -147,7 +124,7 @@ bool IsDescendantOrSameDocument(Document& subject, Document& root) {
} // namespace

LazyLoadImageObserver::LazyLoadImageObserver(const Document& root_document) {
use_viewport_distance_threshold_ =
use_margin_ =
!RuntimeEnabledFeatures::DelayOutOfViewportLazyImagesEnabled() ||
root_document.LoadEventFinished();
}
Expand Down Expand Up @@ -330,11 +307,11 @@ void LazyLoadImageObserver::DocumentOnLoadFinished(Document* root_document) {
if (!RuntimeEnabledFeatures::DelayOutOfViewportLazyImagesEnabled()) {
return;
}
if (use_viewport_distance_threshold_) {
if (use_margin_) {
return;
}

use_viewport_distance_threshold_ = true;
use_margin_ = true;

if (lazy_load_intersection_observer_) {
// Intersection observer doesn't support dynamic margin changes so we just
Expand All @@ -345,15 +322,15 @@ void LazyLoadImageObserver::DocumentOnLoadFinished(Document* root_document) {

void LazyLoadImageObserver::CreateLazyLoadIntersectionObserver(
Document* root_document) {
int viewport_threshold =
use_viewport_distance_threshold_
? GetLazyImageLoadingViewportDistanceThresholdPx(*root_document)
: 0;
int margin = GetLazyLoadingImageMarginPx(*root_document);
IntersectionObserver* new_observer = IntersectionObserver::Create(
{Length::Fixed(viewport_threshold)}, {std::numeric_limits<float>::min()},
root_document,
/* (root) margin */ {Length::Fixed(margin)},
/* thresholds */ {std::numeric_limits<float>::min()},
/* document */ root_document,
/* callback */
WTF::BindRepeating(&LazyLoadImageObserver::LoadIfNearViewport,
WrapWeakPersistent(this)),
/* ukm_metric_id */
LocalFrameUkmAggregator::kLazyLoadIntersectionObserver);

if (lazy_load_intersection_observer_) {
Expand All @@ -372,4 +349,34 @@ void LazyLoadImageObserver::Trace(Visitor* visitor) const {
visitor->Trace(visibility_metrics_observer_);
}

int LazyLoadImageObserver::GetLazyLoadingImageMarginPx(
const Document& document) {
if (!use_margin_) {
return 0;
}

const Settings* settings = document.GetSettings();
if (!settings) {
return 0;
}

switch (GetNetworkStateNotifier().EffectiveType()) {
case WebEffectiveConnectionType::kTypeUnknown:
return settings->GetLazyLoadingImageMarginPxUnknown();
case WebEffectiveConnectionType::kTypeOffline:
return settings->GetLazyLoadingImageMarginPxOffline();
case WebEffectiveConnectionType::kTypeSlow2G:
return settings->GetLazyLoadingImageMarginPxSlow2G();
case WebEffectiveConnectionType::kType2G:
return settings->GetLazyLoadingImageMarginPx2G();
case WebEffectiveConnectionType::kType3G:
return settings->GetLazyLoadingImageMarginPx3G();
case WebEffectiveConnectionType::kType4G:
return settings->GetLazyLoadingImageMarginPx4G();
default:
NOTREACHED();
return 0;
}
}

} // namespace blink
10 changes: 6 additions & 4 deletions third_party/blink/renderer/core/html/lazy_load_image_observer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,12 @@ class LazyLoadImageObserver final

void CreateLazyLoadIntersectionObserver(Document* root_document);

// True if `lazy_load_intersection_observer_` should use a non-zero threshold
// for the viewport. True by default and used by DelayOutOfViewportLazyImages
// to not use a threshold while loading.
bool use_viewport_distance_threshold_;
int GetLazyLoadingImageMarginPx(const Document& document);

// True if `lazy_load_intersection_observer_` should use a non-zero margin
// for it's intersection observer. True by default and used by
// DelayOutOfViewportLazyImages to not use a margin while loading.
bool use_margin_;

// The intersection observer responsible for loading the image once it's near
// the viewport.
Expand Down
29 changes: 14 additions & 15 deletions third_party/blink/renderer/core/html/lazy_load_image_observer_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,19 +178,19 @@ class LazyLoadImagesParamsTest
Settings& settings = WebView().GetPage()->GetSettings();

// These should match the values that would be returned by
// GetLoadingDistanceThreshold().
settings.SetLazyImageLoadingDistanceThresholdPxUnknown(200);
settings.SetLazyImageLoadingDistanceThresholdPxOffline(300);
settings.SetLazyImageLoadingDistanceThresholdPxSlow2G(400);
settings.SetLazyImageLoadingDistanceThresholdPx2G(500);
settings.SetLazyImageLoadingDistanceThresholdPx3G(600);
settings.SetLazyImageLoadingDistanceThresholdPx4G(700);
// GetMargin().
settings.SetLazyLoadingImageMarginPxUnknown(200);
settings.SetLazyLoadingImageMarginPxOffline(300);
settings.SetLazyLoadingImageMarginPxSlow2G(400);
settings.SetLazyLoadingImageMarginPx2G(500);
settings.SetLazyLoadingImageMarginPx3G(600);
settings.SetLazyLoadingImageMarginPx4G(700);
}

// When DelayOutOfViewportLazyImages is enabled, this returns the threshold
// that will be used after the document has finished loading, as a threshold
// When DelayOutOfViewportLazyImages is enabled, this returns the margin
// that will be used after the document has finished loading, as a margin
// of zero is used during loading.
int GetLoadingDistanceThreshold() const {
int GetMargin() const {
static constexpr int kDistanceThresholdByEffectiveConnectionType[] = {
200, 300, 400, 500, 600, 700};
return kDistanceThresholdByEffectiveConnectionType[static_cast<int>(
Expand Down Expand Up @@ -231,7 +231,7 @@ TEST_P(LazyLoadImagesParamsTest, NearViewport) {
<img src='https://example.com/unset.png'
onload='console.log("unset onload");' />
</body>)HTML",
kViewportHeight + GetLoadingDistanceThreshold() - 100));
kViewportHeight + GetMargin() - 100));

css_resource.Complete("img { width: 50px; height: 50px; }");
test::RunPendingTasks();
Expand Down Expand Up @@ -316,7 +316,7 @@ TEST_P(LazyLoadImagesParamsTest, FarFromViewport) {
<img src='https://example.com/unset.png'
onload='console.log("unset onload");' />
</body>)HTML",
kViewportHeight + GetLoadingDistanceThreshold() + 100));
kViewportHeight + GetMargin() + 100));

css_resource.Complete("img { width: 50px; height: 50px; }");
test::RunPendingTasks();
Expand Down Expand Up @@ -399,8 +399,7 @@ class LazyLoadImagesTest : public SimTest {
gfx::Size(kViewportWidth, kViewportHeight));

Settings& settings = WebView().GetPage()->GetSettings();
settings.SetLazyImageLoadingDistanceThresholdPx4G(
kLoadingDistanceThreshold);
settings.SetLazyLoadingImageMarginPx4G(kLoadingDistanceThreshold);
settings.SetLazyFrameLoadingDistanceThresholdPx4G(
kLoadingDistanceThreshold);
}
Expand Down Expand Up @@ -1143,7 +1142,7 @@ class DelayOutOfViewportLazyImagesTest : public SimTest {
gfx::Size(kViewportWidth, kViewportHeight));

Settings& settings = WebView().GetPage()->GetSettings();
settings.SetLazyImageLoadingDistanceThresholdPx4G(kDistanceThresholdPx);
settings.SetLazyLoadingImageMarginPx4G(kDistanceThresholdPx);
}

private:
Expand Down

0 comments on commit bacaa80

Please sign in to comment.