1386864 - stylo: Assertion failure: IsResolved() (Resolve() must be called first), at /home/ikezoe/autoland/layout/style/nsStyleStruct.h:355

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Hiroyuki Ikezoe (:hiro)]

Happens while running test_transitions_per_property.html

Comment 1

[Boris Chiou [:boris]]

This is a regression, but I'm not sure where is this from. Unfortunately, if we don't fix this, we cannot enable test_transitions_per_property.html.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Yeah, it's really unfortunate.  We definitely should run test_transitions_per_property.html to avoid this kind of regressions.

Comment 3

[Boris Chiou [:boris]]

call stack


GECKO(15097) | Assertion failure: IsResolved() (Resolve() must be called first), at ./layout/style/nsStyleStruct.h:355
GECKO(15097) | #01: AddAndRemoveImageAssociations(nsFrame*, nsStyleImageLayers const*, nsStyleImageLayers const*) (nsStyleStruct.h:543, in XUL)
GECKO(15097) | #02: nsFrame::DidSetStyleContext(nsStyleContext*) (nsFrame.cpp:922, in XUL)
GECKO(15097) | #03: nsIFrame::SetStyleContext(nsStyleContext*) (nsStyleContext.h:81, in XUL)
GECKO(15097) | #04: mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoTraversalFlags) (ServoRestyleManager.cpp:657, in XUL)
GECKO(15097) | #05: mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoTraversalFlags) (ServoRestyleManager.cpp:714, in XUL)
GECKO(15097) | #06: mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoTraversalFlags) (ServoRestyleManager.cpp:714, in XUL)
GECKO(15097) | #07: mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoTraversalFlags) (ServoRestyleManager.cpp:714, in XUL)
GECKO(15097) | #08: mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) (ServoRestyleManager.cpp:925, in XUL)
GECKO(15097) | #09: mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) (ServoRestyleManager.cpp:996, in XUL)
GECKO(15097) | #10: nsDocument::FlushPendingNotifications(mozilla::FlushType) (nsDocument.h:1025, in XUL)
GECKO(15097) | #11: nsComputedDOMStyle::UpdateCurrentStyleSources(bool) (nsComputedDOMStyle.cpp:807, in XUL)
GECKO(15097) | #12: nsComputedDOMStyle::GetPropertyCSSValue(nsAString const&, mozilla::ErrorResult&) (nsComputedDOMStyle.cpp:1011, in XUL)
GECKO(15097) | #13: nsComputedDOMStyle::GetPropertyValue(nsAString const&, nsAString&) (AlreadyAddRefed.h:121, in XUL)
GECKO(15097) | #14: mozilla::dom::CSSStyleDeclarationBinding::getPropertyValue(JSContext*, JS::Handle<JSObject*>, nsICSSDeclaration*, JSJitMethodCallArgs const&) (nsICSSDeclaration.h:126, in XUL)
GECKO(15097) | #15: mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (BindingUtils.cpp:3055, in XUL)

Comment 4

[Boris Chiou [:boris]]

This happened while running "transitions not supported for property mask-image"

CJ, do you have any suggestion for this assertion, e.g. any possible idea?

Comment 5

[Boris Chiou [:boris]]

Hi, CJ,

And this assertion happened in this line [1], the |prop| is "mask-image", |val| is 
"url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==), none"


[1] http://searchfox.org/mozilla-central/rev/30a47c4339bd397b937abdb2305f99d3bb537ba6/layout/style/test/test_transitions_per_property.html#987

Comment 6

[Boris Chiou [:boris]]

Looks like you know how to fix it, so assign to you. Thanks, CJ. :)

Comment 7

[u459114]

Attachment: Bug 1386864 - Part 1. Compare image-data of two style-image objects only if both of them are resolved.

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

Comment 8

[u459114]

Attachment: Bug 1386864 - Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations.

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

Comment 9

[Boris Chiou [:boris]]

Thanks you ~~~~~~~~

Comment 10

[u459114]

Comment on attachment 8893713 [details]
Bug 1386864 - Part 1. Compare image-data of two style-image objects only if both of them are resolved.

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

Comment 11

[u459114]

Comment on attachment 8893714 [details]
Bug 1386864 - Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations.

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

Comment 12

[Boris Chiou [:boris]]

I had verified these patches by test_transitions_per_property.html

Comment 13

[Cameron McCormack (:heycam)]

Comment on attachment 8893713 [details]
Bug 1386864 - Part 1. Compare image-data of two style-image objects only if both of them are resolved.

https://reviewboard.mozilla.org/r/164826/#review170484

Comment 14

[Cameron McCormack (:heycam)]

Comment on attachment 8893714 [details]
Bug 1386864 - Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations.

https://reviewboard.mozilla.org/r/164828/#review170486

::: layout/generic/nsFrame.cpp:831
(Diff revision 2)
>  static
>  void
> +CompareLayers(const nsStyleImageLayers* aFirstLayers,

Nit: "static void" on one line, here and below.

::: layout/generic/nsFrame.cpp:845
(Diff revision 2)
> +      continue;
> +    }
> +
> +    // aCallback is called when the style image in aFirstLayers is thought to
> +    // be different with the corresponded one in aSecondLayers
> +    if (!aSecondLayers || i >= aSecondLayers->mImageCount ||

I think aSecondLayers is always non-null?  If so, please remove the null check.

Comment 15

[u459114]

Comment on attachment 8893714 [details]
Bug 1386864 - Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations.

https://reviewboard.mozilla.org/r/164828/#review170486

> I think aSecondLayers is always non-null?  If so, please remove the null check.

Depdend on the caller side, tt can be null 
https://hg.mozilla.org/mozilla-central/file/c48b603e7650/layout/generic/nsFrame.cpp#l911

Comment 16

[u459114]

Comment on attachment 8893714 [details]
Bug 1386864 - Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations.

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

Comment 17

[u459114]

Comment on attachment 8893713 [details]
Bug 1386864 - Part 1. Compare image-data of two style-image objects only if both of them are resolved.

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

Comment 18

[u459114]

Comment on attachment 8893714 [details]
Bug 1386864 - Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations.

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

Comment 19

[u459114]

Comment on attachment 8893714 [details]
Bug 1386864 - Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations.

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

Comment 20

[u459114]

Comment on attachment 8893714 [details]
Bug 1386864 - Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations.

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

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2dc62e1b19a9
Part 1. Compare image-data of two style-image objects only if both of them are resolved. r=heycam
https://hg.mozilla.org/integration/autoland/rev/7f21a31de9fa
Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations. r=heycam

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2dc62e1b19a9
https://hg.mozilla.org/mozilla-central/rev/7f21a31de9fa

Comment 23

[Aki Sasaki (not active)]

https://hg.mozilla.org/projects/date/rev/2dc62e1b19a9802ce958066629522b46f6f4d19a
Bug 1386864 - Part 1. Compare image-data of two style-image objects only if both of them are resolved. r=heycam

https://hg.mozilla.org/projects/date/rev/7f21a31de9faa0ba282c541efdf1017e86a74c69
Bug 1386864 - Part 2. Implement CompareLayers to reuse the code in AddAndRemoveImageAssociations. r=heycam