1383791 - If the URL value of a css mask property does not consist of fragment, do not trigger svg resource donwload

Status: RESOLVED FIXED

(Core :: SVG, enhancement)

Description

[u459114]

Split out from bug 1301245, since there are still several uncerntain things in that bug

Comment 1

[u459114]

Attachment: Bug 1383791 - Download an SVG resource only if the mask URL consists of a fragment.

Review commit: https://reviewboard.mozilla.org/r/160530/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/160530/

Comment 2

[u459114]

Comment on attachment 8889498 [details]
Bug 1383791 -  Download an SVG resource only if the mask URL consists of a fragment.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/160530/diff/1-2/

Comment 3

[u459114]

Comment on attachment 8889498 [details]
Bug 1383791 -  Download an SVG resource only if the mask URL consists of a fragment.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/160530/diff/2-3/

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8889498 [details]
Bug 1383791 -  Download an SVG resource only if the mask URL consists of a fragment.

https://reviewboard.mozilla.org/r/160530/#review166038

r=me if not appending nullptr works.  Otherwise I'd like to understand why.

::: layout/svg/nsSVGEffects.cpp:408
(Diff revision 3)
> +    }
> +
> +    // This URL can not refer to an SVG mask resource if it dose not consist
> +    // of a fragment.
> +    if (!hasRef) {
> +      mProperties.AppendElement(nullptr);

Can we skip appending something, rather than appending nullptr?  Then you wouldn't need the null checks in the users of nsSVGMaskProperty::GetProps.

::: layout/svg/nsSVGEffects.cpp:410
(Diff revision 3)
> +      // nsSVGPaintingProperty's ctor tiggers an external SVG resource
> +      // download, use it only if maskUri has chance pointing to an SVG mask
> +      // resource.

Nit: "has a chance of pointing"

Comment 5

[u459114]

Comment on attachment 8889498 [details]
Bug 1383791 -  Download an SVG resource only if the mask URL consists of a fragment.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/160530/diff/3-4/

Comment 6

[u459114]

Comment on attachment 8889498 [details]
Bug 1383791 -  Download an SVG resource only if the mask URL consists of a fragment.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/160530/diff/4-5/

Comment 7

[Robert Longson [:longsonr]]

Comment on attachment 8889498 [details]
Bug 1383791 -  Download an SVG resource only if the mask URL consists of a fragment.

https://reviewboard.mozilla.org/r/160530/#review166054

::: layout/svg/nsSVGEffects.cpp:405
(Diff revision 5)
> -    nsSVGPaintingProperty* prop = new nsSVGPaintingProperty(maskUri, aFrame,
> -                                                            false);
> +    bool hasRef = false;
> +    if (maskUri) {
> +      maskUri->GetHasRef(&hasRef);
> +    }
> +
> +    // Accrording to maskUri, nsSVGPaintingProperty's ctor may tigger an

trigger (rather than tigger).

Comment 8

[Robert Longson [:longsonr]]

Comment on attachment 8889498 [details]
Bug 1383791 -  Download an SVG resource only if the mask URL consists of a fragment.

https://reviewboard.mozilla.org/r/160530/#review166056

Comment 9

[u459114]

Comment on attachment 8889498 [details]
Bug 1383791 -  Download an SVG resource only if the mask URL consists of a fragment.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/160530/diff/5-6/

Comment 10

[u459114]

Comment on attachment 8889498 [details]
Bug 1383791 -  Download an SVG resource only if the mask URL consists of a fragment.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/160530/diff/6-7/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e7ce13991ca
Download an SVG resource only if the mask URL consists of a fragment. r=heycam

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0e7ce13991ca