[Webkit-unassigned] [Bug 220727] Implement efficient way to collect images that are likely visible in the viewport

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 20 18:18:44 PST 2021


https://bugs.webkit.org/show_bug.cgi?id=220727

--- Comment #7 from Sachit Partap <sachit092partapv at gmail.com> ---
Comment on attachment 417871
  --> https://bugs.webkit.org/attachment.cgi?id=417871
patch

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 2aaf2c489ad6..60dc23d04ade 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,45 @@
>+2021-01-19  Antti Koivisto  <antti at apple.com>
>+
>+        Implement efficient way to collect images that are likely visible in the viewport
>+        https://bugs.webkit.org/show_bug.cgi?id=220727
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        To implement visibility based resource load scheduling we need to know what is visible.
>+
>+        This patch expands the exisiting visibility test mechanism to support collecting all visible, loading resources in a document.
>+        The code is not used yet.
>+
>+        * loader/ImageLoader.cpp:
>+        (WebCore::ImageLoader::imageVisibleInViewport const):
>+        * loader/ImageLoader.h:
>+        * loader/cache/CachedImage.cpp:
>+        (WebCore::CachedImage::isVisibleInViewport const):
>+        * loader/cache/CachedImage.h:
>+        * loader/cache/CachedImageClient.h:
>+        (WebCore::CachedImageClient::imageVisibleInViewport const):
>+
>+        Add a virtual interface. It is implemented by ImageLoader (for regular images) and RenderElement (for CSS images like backgrounds).
>+
>+        * loader/cache/CachedResourceLoader.cpp:
>+        (WebCore::CachedResourceLoader::CachedResourceLoader):
>+        
>+        * loader/cache/CachedResourceLoader.h:
>+        * rendering/RenderElement.cpp:
>+        (WebCore::RenderElement::isVisibleIgnoringGeometry const):
>+
>+        Factor geometry-independent test to a function.
>+
>+        (WebCore::RenderElement::isVisibleInDocumentRect const):
>+        (WebCore::RenderElement::imageVisibleInViewport const):
>+        * rendering/RenderElement.h:
>+        * rendering/RenderReplaced.cpp:
>+        (WebCore::RenderReplaced::isContentLikelyVisibleInViewport):
>+
>+        This also allows intrisically sized images that currently have zero size.
>+
>+        * rendering/RenderReplaced.h:
>+
> 2021-01-18  Andres Gonzalez  <andresg_22 at apple.com>
> 
>         Apple Mail AX, VoiceOver: When composing a new email message, moving VO focus to the message body field does not bring keyboard focus along.
>diff --git a/Source/WebCore/loader/ImageLoader.cpp b/Source/WebCore/loader/ImageLoader.cpp
>index 6c065d7813b1..0acf5f489351 100644
>--- a/Source/WebCore/loader/ImageLoader.cpp
>+++ b/Source/WebCore/loader/ImageLoader.cpp
>@@ -602,4 +602,16 @@ void ImageLoader::resetLazyImageLoading(Document& document)
>     m_lazyImageLoadState = LazyImageLoadState::None;
> }
> 
>+VisibleInViewportState ImageLoader::imageVisibleInViewport(const Document& document) const
>+{
>+    if (&element().document() != &document)
>+        return VisibleInViewportState::No;
>+
>+    auto* renderer = element().renderer();
>+    if (!is<RenderReplaced>(renderer))
>+        return VisibleInViewportState::No;
>+
>+    return downcast<RenderReplaced>(*renderer).isContentLikelyVisibleInViewport() ? VisibleInViewportState::Yes : VisibleInViewportState::No;
>+}
>+
> }
>diff --git a/Source/WebCore/loader/ImageLoader.h b/Source/WebCore/loader/ImageLoader.h
>index bda032d55af7..457e5a481a78 100644
>--- a/Source/WebCore/loader/ImageLoader.h
>+++ b/Source/WebCore/loader/ImageLoader.h
>@@ -115,6 +115,8 @@ private:
>     
>     void timerFired();
> 
>+    VisibleInViewportState imageVisibleInViewport(const Document&) const override;
>+
>     Element& m_element;
>     CachedResourceHandle<CachedImage> m_image;
>     Timer m_derefElementTimer;
>diff --git a/Source/WebCore/loader/cache/CachedImage.cpp b/Source/WebCore/loader/cache/CachedImage.cpp
>index 5cc933832574..5c72b988faf2 100644
>--- a/Source/WebCore/loader/cache/CachedImage.cpp
>+++ b/Source/WebCore/loader/cache/CachedImage.cpp
>@@ -743,4 +743,14 @@ bool CachedImage::canSkipRevalidation(const CachedResourceLoader& loader, const
>     return m_skippingRevalidationDocument && loader.document() == m_skippingRevalidationDocument;
> }
> 
>+bool CachedImage::isVisibleInViewport(const Document& document) const
>+{
>+    CachedResourceClientWalker<CachedImageClient> walker(m_clients);
>+    while (auto* client = walker.next()) {
>+        if (client->imageVisibleInViewport(document) == VisibleInViewportState::Yes)
>+            return true;
>+    }
>+    return false;
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/loader/cache/CachedImage.h b/Source/WebCore/loader/cache/CachedImage.h
>index 72d59c1ff484..d01ff9d5523c 100644
>--- a/Source/WebCore/loader/cache/CachedImage.h
>+++ b/Source/WebCore/loader/cache/CachedImage.h
>@@ -97,6 +97,8 @@ public:
>     bool stillNeedsLoad() const override { return !errorOccurred() && status() == Unknown && !isLoading(); }
>     bool canSkipRevalidation(const CachedResourceLoader&, const CachedResourceRequest&) const;
> 
>+    bool isVisibleInViewport(const Document&) const;
>+
> private:
>     void clear();
> 
>diff --git a/Source/WebCore/loader/cache/CachedImageClient.h b/Source/WebCore/loader/cache/CachedImageClient.h
>index ba6445b7ea7e..fd558f2f28d7 100644
>--- a/Source/WebCore/loader/cache/CachedImageClient.h
>+++ b/Source/WebCore/loader/cache/CachedImageClient.h
>@@ -28,6 +28,7 @@
> namespace WebCore {
> 
> class CachedImage;
>+class Document;
> class IntRect;
> 
> enum class VisibleInViewportState { Unknown, Yes, No };
>@@ -46,6 +47,7 @@ public:
> 
>     // Called when a new decoded frame for a large image is available or when an animated image is ready to advance to the next frame.
>     virtual VisibleInViewportState imageFrameAvailable(CachedImage& image, ImageAnimatingState, const IntRect* changeRect) { imageChanged(&image, changeRect); return VisibleInViewportState::No; }
>+    virtual VisibleInViewportState imageVisibleInViewport(const Document&) const { return VisibleInViewportState::No; }
> 
>     virtual void didRemoveCachedImageClient(CachedImage&) { }
> 
>diff --git a/Source/WebCore/loader/cache/CachedResourceLoader.cpp b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
>index d1d64dcb7798..1358656ef5d3 100644
>--- a/Source/WebCore/loader/cache/CachedResourceLoader.cpp
>+++ b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
>@@ -192,14 +192,9 @@ static CachedResourceHandle<CachedResource> createResource(CachedResourceRequest
> }
> 
> CachedResourceLoader::CachedResourceLoader(DocumentLoader* documentLoader)
>-    : m_document(nullptr)
>-    , m_documentLoader(documentLoader)
>-    , m_requestCount(0)
>+    : m_documentLoader(documentLoader)
>     , m_unusedPreloadsTimer(*this, &CachedResourceLoader::warnUnusedPreloads)
>     , m_garbageCollectDocumentResourcesTimer(*this, &CachedResourceLoader::garbageCollectDocumentResources)
>-    , m_autoLoadImages(true)
>-    , m_imagesEnabled(true)
>-    , m_allowStaleResources(false)
> {
> }
> 
>@@ -1526,6 +1521,32 @@ void CachedResourceLoader::clearPreloads(ClearPreloadsMode mode)
>     m_preloads = WTFMove(remainingLinkPreloads);
> }
> 
>+Vector<CachedResource*> CachedResourceLoader::visibleResourcesToPrioritize()
>+{
>+    if (!document())
>+        return { };
>+
>+    Vector<CachedResource*> toPrioritize;
>+
>+    for (auto& resource : m_documentResources.values()) {
>+        if (!is<CachedImage>(resource.get()))
>+            continue;
>+        auto& cachedImage = downcast<CachedImage>(*resource);
>+        auto url = cachedImage.url().string();
>+        if (!cachedImage.isLoading())
>+            continue;
>+        if (!cachedImage.url().protocolIsInHTTPFamily())
>+            continue;
>+        if (!cachedImage.loader())
>+            continue;
>+        if (!cachedImage.isVisibleInViewport(*document()))
>+            continue;
>+        toPrioritize.append(&cachedImage);
>+    }
>+
>+    return toPrioritize;
>+}
>+
> const ResourceLoaderOptions& CachedResourceLoader::defaultCachedResourceOptions()
> {
>     static NeverDestroyed<ResourceLoaderOptions> options(
>diff --git a/Source/WebCore/loader/cache/CachedResourceLoader.h b/Source/WebCore/loader/cache/CachedResourceLoader.h
>index 2d7fdc1e2feb..6029707487ea 100644
>--- a/Source/WebCore/loader/cache/CachedResourceLoader.h
>+++ b/Source/WebCore/loader/cache/CachedResourceLoader.h
>@@ -165,6 +165,8 @@ public:
> 
>     KeepaliveRequestTracker& keepaliveRequestTracker() { return m_keepaliveRequestTracker; }
> 
>+    Vector<CachedResource*> visibleResourcesToPrioritize();
>+
> private:
>     explicit CachedResourceLoader(DocumentLoader*);
> 
>@@ -202,7 +204,7 @@ private:
>     WeakPtr<Document> m_document;
>     DocumentLoader* m_documentLoader;
> 
>-    int m_requestCount;
>+    int m_requestCount { 0 };
> 
>     std::unique_ptr<ListHashSet<CachedResource*>> m_preloads;
>     Timer m_unusedPreloadsTimer;
>@@ -212,10 +214,9 @@ private:
>     ResourceTimingInformation m_resourceTimingInfo;
>     KeepaliveRequestTracker m_keepaliveRequestTracker;
> 
>-    // 29 bits left
>-    bool m_autoLoadImages : 1;
>-    bool m_imagesEnabled : 1;
>-    bool m_allowStaleResources : 1;
>+    bool m_autoLoadImages { true };
>+    bool m_imagesEnabled { true };
>+    bool m_allowStaleResources { false };
> };
> 
> class ResourceCacheValidationSuppressor {
>diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
>index 0f8fb3938134..e871f052abcc 100644
>--- a/Source/WebCore/rendering/RenderElement.cpp
>+++ b/Source/WebCore/rendering/RenderElement.cpp
>@@ -1308,7 +1308,7 @@ bool RenderElement::mayCauseRepaintInsideViewport(const IntRect* optionalViewpor
>     return visibleRect.intersects(enclosingIntRect(absoluteClippedOverflowRect()));
> }
> 
>-bool RenderElement::isVisibleInDocumentRect(const IntRect& documentRect) const
>+bool RenderElement::isVisibleIgnoringGeometry() const
> {
>     if (document().activeDOMObjectsAreSuspended())
>         return false;
>@@ -1317,6 +1317,14 @@ bool RenderElement::isVisibleInDocumentRect(const IntRect& documentRect) const
>     if (view().frameView().isOffscreen())
>         return false;
> 
>+    return true;
>+}
>+
>+bool RenderElement::isVisibleInDocumentRect(const IntRect& documentRect) const
>+{
>+    if (!isVisibleIgnoringGeometry())
>+        return false;
>+
>     // Use background rect if we are the root or if we are the body and the background is propagated to the root.
>     // FIXME: This is overly conservative as the image may not be a background-image, in which case it will not
>     // be propagated to the root. At this point, we unfortunately don't have access to the image anymore so we
>@@ -1328,7 +1336,6 @@ bool RenderElement::isVisibleInDocumentRect(const IntRect& documentRect) const
>         ASSERT(is<HTMLHtmlElement>(rootRenderer.element()));
>         // FIXME: Should share body background propagation code.
>         backgroundIsPaintedByRoot = !rootRenderer.hasBackground();
>-
>     }
> 
>     LayoutRect backgroundPaintingRect = backgroundIsPaintedByRoot ? view().backgroundRect() : absoluteClippedOverflowRect();
>@@ -1394,6 +1401,14 @@ VisibleInViewportState RenderElement::imageFrameAvailable(CachedImage& image, Im
>     return isVisible ? VisibleInViewportState::Yes : VisibleInViewportState::No;
> }
> 
>+VisibleInViewportState RenderElement::imageVisibleInViewport(const Document& document) const
>+{
>+    if (&this->document() != &document)
>+        return VisibleInViewportState::No;
>+
>+    return isVisibleInViewport() ? VisibleInViewportState::Yes : VisibleInViewportState::No;
>+}
>+
> void RenderElement::notifyFinished(CachedResource& resource, const NetworkLoadMetrics&)
> {
>     document().cachedResourceLoader().notifyFinished(resource);
>diff --git a/Source/WebCore/rendering/RenderElement.h b/Source/WebCore/rendering/RenderElement.h
>index 763c617f1a63..d0c247cdd24e 100644
>--- a/Source/WebCore/rendering/RenderElement.h
>+++ b/Source/WebCore/rendering/RenderElement.h
>@@ -131,6 +131,7 @@ public:
>     bool repaintAfterLayoutIfNeeded(const RenderLayerModelObject* repaintContainer, const LayoutRect& oldBounds, const LayoutRect& oldOutlineBox, const LayoutRect* newBoundsPtr = nullptr, const LayoutRect* newOutlineBoxPtr = nullptr);
> 
>     bool borderImageIsLoadedAndCanBeRendered() const;
>+    bool isVisibleIgnoringGeometry() const;
>     bool mayCauseRepaintInsideViewport(const IntRect* visibleRect = nullptr) const;
>     bool isVisibleInDocumentRect(const IntRect& documentRect) const;
> 
>@@ -295,7 +296,7 @@ protected:
> 
>     void removeFromRenderFragmentedFlowIncludingDescendants(bool shouldUpdateState);
>     void adjustFragmentedFlowStateOnContainingBlockChangeIfNeeded();
>-    
>+
>     bool isVisibleInViewport() const;
> 
> private:
>@@ -327,6 +328,7 @@ private:
> 
>     bool canDestroyDecodedData() final { return !isVisibleInViewport(); }
>     VisibleInViewportState imageFrameAvailable(CachedImage&, ImageAnimatingState, const IntRect* changeRect) final;
>+    VisibleInViewportState imageVisibleInViewport(const Document&) const final;
>     void didRemoveCachedImageClient(CachedImage&) final;
>     void scheduleRenderingUpdateForImage(CachedImage&) final;
> 
>diff --git a/Source/WebCore/rendering/RenderReplaced.cpp b/Source/WebCore/rendering/RenderReplaced.cpp
>index cf9158e55b36..43bd53708e82 100644
>--- a/Source/WebCore/rendering/RenderReplaced.cpp
>+++ b/Source/WebCore/rendering/RenderReplaced.cpp
>@@ -776,4 +776,20 @@ LayoutRect RenderReplaced::clippedOverflowRectForRepaint(const RenderLayerModelO
>     return computeRectForRepaint(r, repaintContainer);
> }
> 
>+bool RenderReplaced::isContentLikelyVisibleInViewport()
>+{
>+    if (!isVisibleIgnoringGeometry())
>+        return false;
>+
>+    auto& frameView = view().frameView();
>+    auto visibleRect = LayoutRect(frameView.windowToContents(frameView.windowClipRect()));
>+    auto contentRect = computeRectForRepaint(replacedContentRect(), nullptr);
>+
>+    // Content rectangle may be empty because it is intrinsically sized and the content has not loaded yet.
>+    if (contentRect.isEmpty() && (style().logicalWidth().isAuto() || style().logicalHeight().isAuto()))
>+        return visibleRect.contains(contentRect.location());
>+
>+    return visibleRect.intersects(contentRect);
>+}
>+
> }
>diff --git a/Source/WebCore/rendering/RenderReplaced.h b/Source/WebCore/rendering/RenderReplaced.h
>index 7a7412a9d5e1..9b479420a555 100644
>--- a/Source/WebCore/rendering/RenderReplaced.h
>+++ b/Source/WebCore/rendering/RenderReplaced.h
>@@ -43,6 +43,8 @@ public:
>     LayoutSize intrinsicSize() const final { return m_intrinsicSize; }
>     
>     RoundedRect roundedContentBoxRect() const;
>+    
>+    bool isContentLikelyVisibleInViewport();
> 
> protected:
>     RenderReplaced(Element&, RenderStyle&&);

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210121/66e9d517/attachment-0001.htm>


More information about the webkit-unassigned mailing list