1296250 - Fix mask-html-xbl-bound-01.html | assertion count 1 is more than expected 0 assertions

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[u459114]

Get 1 assertion after enable mask-as-shorhand.
REFTEST TEST-UNEXPECTED-FAIL | file:///home/worker/workspace/build/tests/reftest/tests/layout/reftests/svg/svg-integration/mask-html-xbl-bound-01.html | assertion count 1 is more than expected 0 assertions

https://treeherder.mozilla.org/logviewer.html#?job_id=25918147&repo=try#L11623

Comment 1

[u459114]

Attachment: Bug 1296250 - Part 1. Correct the condition of an assertion in SetStyleImage.

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

Comment 2

[u459114]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a8ca131500d

Comment 3

[u459114]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ff17dfb3d27

Comment 4

[u459114]

(In reply to C.J. Ku[:cjku](UTC+8) from comment #3)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ff17dfb3d27

Try result of enable-mask-as-shorthand

Comment 5

[u459114]

Attachment: Bug 1296250 - Part 2. Promote NS_ASSERTION to MOZ_ASSERT.

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

Comment 6

[u459114]

Comment on attachment 8782387 [details]
Bug 1296250 - Part 1. Correct the condition of an assertion in SetStyleImage.

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

Comment 7

[Cameron McCormack (:heycam)]

Comment on attachment 8782387 [details]
Bug 1296250 - Part 1. Correct the condition of an assertion in SetStyleImage.

https://reviewboard.mozilla.org/r/72570/#review70280

::: layout/style/nsRuleNode.cpp:1284
(Diff revision 2)
> +      // 2. aValue is a not a local-ref URL, but it refers to an element in
> +      //    current document. For example, the url of the current document is

"an element in the current document"

::: layout/style/nsRuleNode.cpp:1290
(Diff revision 2)
> +      //    current document. For example, the url of the current document is
> +      //    "http://foo.html" and aValue is url(http://foo.html#foo)
> +      // 3. aValue is a local-ref URL, e.g. url(#foo)
> +      //
> +      // We skip image download in TryToStartImageLoadOnValue under #2 and #3,
> +      // and that why we get eCSSUnit_URL instead of eCSSUnit_Image here.

"that's why"

::: layout/style/nsRuleNode.cpp:1292
(Diff revision 2)
> +      // Check #2.
> +      nsIDocument* currentDoc = aStyleContext->PresContext()->Document();
> +      nsIURI* docURI = currentDoc->GetDocumentURI();
> +      nsIURI* imageURI = aValue.GetURLValue();
> +      bool isEqualExceptRef = false;
> +      imageURI->EqualsExceptRef(docURI, &isEqualExceptRef);
> +
> +      // Check #3.
>        mozilla::css::URLValueData *urlData = aValue.GetURLStructValue();
> +      bool isLocalRef = urlData->GetLocalURLFlag();

Can we factor this out so that we use the same check from TryToStartImageOnLoad and in here?

Also, it would be good if we could allow "isEqualExceptRef || isLocalRef" only if we're in SetStyleImage for mask-image, but I guess it might be a bit annoying to pass through the property or a boolean.  Up to you.

Comment 8

[Cameron McCormack (:heycam)]

Comment on attachment 8782396 [details]
Bug 1296250 - Part 2. Promote NS_ASSERTION to MOZ_ASSERT.

https://reviewboard.mozilla.org/r/72584/#review70284

Comment 9

[u459114]

Comment on attachment 8782387 [details]
Bug 1296250 - Part 1. Correct the condition of an assertion in SetStyleImage.

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

Comment 10

[u459114]

Comment on attachment 8782396 [details]
Bug 1296250 - Part 2. Promote NS_ASSERTION to MOZ_ASSERT.

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d17d3db2b52
Part 1. Correct the condition of an assertion in SetStyleImage. r=heycam
https://hg.mozilla.org/integration/autoland/rev/20885c842fd3
Part 2. Promote NS_ASSERTION to MOZ_ASSERT. r=heycam

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5d17d3db2b52
https://hg.mozilla.org/mozilla-central/rev/20885c842fd3