Changeset 273764 in webkit

Timestamp:

Mar 2, 2021, 3:22:20 PM (6 years ago)

Author:

Said Abou-Hallawa

Message:

The layout of SVGImage should force the layout for its clients
​https://bugs.webkit.org/show_bug.cgi?id=221253
<rdar://problem/74138641>

Reviewed by Simon Fraser.

Source/WebCore:

Unlike the bitmap image, the intrinsic size of SVGImage can only be known
after running its layout. Because SVGImage can be used by multiple clients,
CachedImage maintains an SVGImageCache which maps a client to an
SVGImageForContainer. SVGImageForContainer is just a wrapper of SVGImage
with the intrinsic size of the client. Because we may set an entry for
the renderer in SVGImageCache early before running a layout for SVGImage
and because the renderer intrinsic size depends on the intrinsic size of
SVGImage, SVGImageForContainer may have an empty intrinsic size.

So basically it is a race condition: knowing the intrinsic size of the
SVGImage client depends on the intrinsic size of SVGImage itself. And the
intrinsic size of SVGImageForContainer depends on the intrinsic size of
the client. This may lead to not rendering the clients because their sizes
are empty. To fix this issue we need to force calculateBackgroundImageGeometry()
once we finish the layout of the SVGImage which happens after receiving all
its data and calling RenderBox::imageChanged().

Test: fast/css/background-svg-image-loading.html

• loader/cache/CachedImage.cpp:

(WebCore::CachedImage::finishLoading):

• rendering/RenderBox.cpp:

(WebCore::RenderBox::repaintLayerRectsForImage):

LayoutTests:

• fast/css/background-svg-image-loading-expected.html: Added.
• fast/css/background-svg-image-loading.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/background-svg-image-loading-expected.html (added)
• LayoutTests/fast/css/background-svg-image-loading.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/cache/CachedImage.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-03-02  Said Abou-Hallawa  <said@apple.com>
+
+        The layout of SVGImage should force the layout for its clients
+        https://bugs.webkit.org/show_bug.cgi?id=221253
+        <rdar://problem/74138641>
+
+        Reviewed by Simon Fraser.
+
+        * fast/css/background-svg-image-loading-expected.html: Added.
+        * fast/css/background-svg-image-loading.html: Added.
+
 2021-03-02  Lauro Moura  <lmoura@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-03-02  Said Abou-Hallawa  <said@apple.com>
+
+        The layout of SVGImage should force the layout for its clients
+        https://bugs.webkit.org/show_bug.cgi?id=221253
+        <rdar://problem/74138641>
+
+        Reviewed by Simon Fraser.
+
+        Unlike the bitmap image, the intrinsic size of SVGImage can only be known
+        after running its layout. Because SVGImage can be used by multiple clients,
+        CachedImage maintains an SVGImageCache which maps a client to an
+        SVGImageForContainer. SVGImageForContainer is just a wrapper of SVGImage
+        with the intrinsic size of the client. Because we may set an entry for
+        the renderer in SVGImageCache early before running a layout for SVGImage
+        and because the renderer intrinsic size depends on the intrinsic size of 
+        SVGImage, SVGImageForContainer may have an empty intrinsic size.
+
+        So basically it is a race condition: knowing the intrinsic size of the
+        SVGImage client depends on the intrinsic size of SVGImage itself. And the
+        intrinsic size of SVGImageForContainer depends on the intrinsic size of 
+        the client. This may lead to not rendering the clients because their sizes
+        are empty. To fix this issue we need to force calculateBackgroundImageGeometry()
+        once we finish the layout of the SVGImage which happens after receiving all
+        its data and calling RenderBox::imageChanged().
+
+        Test: fast/css/background-svg-image-loading.html
+
+        * loader/cache/CachedImage.cpp:
+        (WebCore::CachedImage::finishLoading):
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::repaintLayerRectsForImage):
+
 2021-03-02  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/loader/cache/CachedImage.cpp

@@ -585 +585 @@
     }
 
+    setLoading(false);
     notifyObservers();
     CachedResource::finishLoading(data, metrics);

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -1851 +1851 @@
 
     for (auto* layer = &layers; layer; layer = layer->next()) {
-        if (layer->image() && image == layer->image()->data() && layer->image()->canRender(this, style().effectiveZoom())) {
+        if (layer->image() && image == layer->image()->data() && (layer->image()->isLoaded() || layer->image()->canRender(this, style().effectiveZoom()))) {
             // Now that we know this image is being used, compute the renderer and the rect if we haven't already.
             bool drawingRootBackground = drawingBackground && (isDocumentElementRenderer() || (isBody() && !document().documentElement()->renderer()->hasBackground()));