Bug 178567

Summary: When destroying a resource, register "only" the clients who are losing their resource as having pending resources
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: SVGAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, simon.fraser, thorton, webkit-bug-importer, zimmermann
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Said Abou-Hallawa 2017-10-19 22:11:18 PDT
In <http://trac.webkit.org/changeset/95047>, we tried to fix this issue. But instead of registering the clients of the destroyed resource, we registered all the clients in the cache as having pending resources.

In SVGResourcesCache::resourceDestroyed(), we call resourcesCacheFromRenderer() which returns a cache that maps <RenderElement, SVGResources>.
We loop through all the elements in the cache and we call SVGResources::resourceDestroyed() which will remove the reference to the destroyed resource if it's one of the resources of SVGResources
Then we call SVGDocumentExtensions::addPendingResource() with the ID of the destroyed resource and the Element of the RenderElement.

This is wrong if the SVGResources does not have a reference to the destroyed resource. It is waste of time to register the Element of the RenderElement to have a pending resources in this caae.
Comment 1 Said Abou-Hallawa 2017-10-19 22:31:34 PDT
Created attachment 324358 [details]
Patch
Comment 2 Said Abou-Hallawa 2017-10-19 22:34:05 PDT
<rdar://problem/35064781>
Comment 3 Simon Fraser (smfr) 2017-10-20 11:01:40 PDT
Comment on attachment 324358 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324358&action=review

> Source/WebCore/rendering/svg/SVGResources.cpp:391
>              m_clipperFilterMaskerData->masker = 0;

These 0 should all be nullptr, right?

> Source/WebCore/rendering/svg/SVGResources.h:70
> +    bool resourceDestroyed(RenderSVGResourceContainer&);

Please add a comment saying what the return value means, or use an enum.
Comment 4 Said Abou-Hallawa 2017-10-20 12:02:19 PDT
Created attachment 324424 [details]
Patch
Comment 5 WebKit Commit Bot 2017-10-20 13:34:31 PDT
Comment on attachment 324424 [details]
Patch

Clearing flags on attachment: 324424

Committed r223789: <https://trac.webkit.org/changeset/223789>
Comment 6 WebKit Commit Bot 2017-10-20 13:34:33 PDT
All reviewed patches have been landed.  Closing bug.