1349457 - stylo: eagerly style generated content just after it is created

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P1)

Description

[Cameron McCormack (:heycam)]

When generated content is created, we need to eagerly style it.  Not doing this is the cause of layout/reftests/bugs/451168-1.html tripping the bug 1345695 assertion.

Comment 1

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=216200ed739462e305f44b955abfc5479fed363a

Comment 2

[Cameron McCormack (:heycam)]

Attachment: Bug 1349457 - stylo: Eagerly style generated content.

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

Comment 3

[Cameron McCormack (:heycam)]

Comment on attachment 8849891 [details]
Bug 1349457 - stylo: Eagerly style generated content.

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

Comment 4

[Bobby Holley (:bholley)]

Comment on attachment 8849891 [details]
Bug 1349457 - stylo: Eagerly style generated content.

https://reviewboard.mozilla.org/r/122636/#review125012

r=me with the |createdGeneratedContent| stuff removed, or a good reason why we should keep it.

::: layout/base/nsCSSFrameConstructor.cpp:1918
(Diff revision 2)
> +  if (createdGeneratedContent && aParentContent->IsStyledByServo()) {
> +    mPresShell->StyleSet()->AsServo()->StyleNewSubtree(container);
> +  }

Hm, shouldn't we style |container| unconditionally? I don't understand what case we're trying to optimize for with |createdGeneratedContent| and why it would be ok to skip |container| even if |createdGeneratedContent| were false.

Comment 5

[Bobby Holley (:bholley)]

Note that I'm assuming here that we only end up in this function if we are very likely to create actual generated content. Otherwise we'd be appending a lot of unnecessary XUL NAC to the DOM.

Comment 6

[Cameron McCormack (:heycam)]

You are right that we get in here only if we have ::before/::after rules that could produce generated content.  It is possible for nsCSSFrameConstructor::CreateGeneratedContent to return null, either (a) if the |content: url()| didn't resolve to an image, or (b) the |content: attr(x)| didn't refer to an existing attribute on the element.  I assume such cases are rare, so we could style |container| unconditionally if you think it's not worth checking whether CreateGeneratedContent succeeded.

Comment 7

[Bobby Holley (:bholley)]

Yeah, let's keep it simple and unconditional.

Comment 8

[Cameron McCormack (:heycam)]

Comment on attachment 8849891 [details]
Bug 1349457 - stylo: Eagerly style generated content.

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c15ca7e1ba7
stylo: Eagerly style generated content. r=bholley

Comment 10

[Carsten Book [:Tomcat]]

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