Bug 126069 – Start refactoring Filter code to reuse CachedSVGDocument for clipPath

• Component: CSS
• Status: RESOLVED FIXED

[Dirk Schulze]

The current code in StyleResolver needs some refactoring so that clipPath can use SVG document caching as well.

[Dirk Schulze]

Created attachment 219759 [details]
Patch

[Andreas Kling]

Comment on attachment 219759 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Smaller refactoring of the CSS filter style resolver code. Previously the code
> +        requested the FilterOperations list from RenderStyle and compared the content
> +        in this list with an internal map. Then the resource loading was triggered.
> +        With the refactoring we do not request the list from RenderStyle anymore but
> +        rely on the hash map data entirely.

Why is this not necessary anymore?

> Source/WebCore/css/StyleResolver.cpp:3418
> +    PendingSVGDocumentMap::iterator end = state.pendingSVGDocuments().end();
> +    for (PendingSVGDocumentMap::iterator it = state.pendingSVGDocuments().begin(); it != end; ++it) {

I'd shorten this down a bit with auto:

for (auto it = state.pendingSVGDocuments().begin(), end = state.pendingSVGDocuments().end(); it != end; ++it) {

[Dirk Schulze]

(In reply to comment #2)
> (From update of attachment 219759 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=219759&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        Smaller refactoring of the CSS filter style resolver code. Previously the code
> > +        requested the FilterOperations list from RenderStyle and compared the content
> > +        in this list with an internal map. Then the resource loading was triggered.
> > +        With the refactoring we do not request the list from RenderStyle anymore but
> > +        rely on the hash map data entirely.
> 
> Why is this not necessary anymore?

The HashMap has a pointer to the FilterOperation that needs the cached resource. The function currently has access to this pointer but instead uses RenderStyle to get the operator again. Even worst, we now need to iterate through the whole list of FilterOperations instead just taking the FilterOperations we know that they need resources.


> 
> > Source/WebCore/css/StyleResolver.cpp:3418
> > +    PendingSVGDocumentMap::iterator end = state.pendingSVGDocuments().end();
> > +    for (PendingSVGDocumentMap::iterator it = state.pendingSVGDocuments().begin(); it != end; ++it) {
> 
> I'd shorten this down a bit with auto:
> 
> for (auto it = state.pendingSVGDocuments().begin(), end = state.pendingSVGDocuments().end(); it != end; ++it) {


Will do.

[Dirk Schulze]

Created attachment 219769 [details]
Patch

[Andreas Kling]

Comment on attachment 219769 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 219769 [details]
Patch

Rejecting attachment 219769 [details] from commit-queue.

New failing tests:
editing/pasteboard/copy-image-with-alt-text.html
Full output: http://webkit-queues.appspot.com/results/50948252

[WebKit Commit Bot]

Created attachment 219780 [details]
Archive of layout-test-results from webkit-cq-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 219759 [details]
Patch

Attachment 219759 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/44438132

New failing tests:
editing/inserting/insert-br-009.html

[Build Bot]

Created attachment 219837 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Dirk Schulze]

Comment on attachment 219769 [details]
Patch

Looking at the tests, they seem unrelated to filters. Resubmitting.

[WebKit Commit Bot]

Comment on attachment 219769 [details]
Patch

Clearing flags on attachment: 219769

Committed r160973: <http://trac.webkit.org/changeset/160973>

[Ahmad Saleem]

Landed and didn't seem to back-out:

https://github.com/WebKit/WebKit/commit/2283bb6745b292b6c68784385b7284e724cae818

[Radar WebKit Bug Importer]

<rdar://problem/114791414>