725867 - Optimize anon content in BBP

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: wip

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 595898 [details] [diff] [review]
wip

This is still reasonable conservative, but helps with XUL based chrome UI
which has plenty of elements in anonymous content.

I'd like to find better way to ensure that anonymous elements in a document
are certainly alive, but haven't found that yet.
(document keeps non-anonymous elements alive)

Comment 2

[Andrew McCreight [:mccr8]]

Comment on attachment 595898 [details] [diff] [review]
wip

Review of attachment 595898 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsGenericElement.cpp
@@ +4537,4 @@
>    nsIDocument* currentDoc = aNode->GetCurrentDoc();
>    if (currentDoc &&
> +      nsCCUncollectableMarker::InGeneration(currentDoc->GetMarkedCCGeneration()) &&
> +      // If aNode is not optimizable, but it is still an element

"but it is still" should probably be "but is"

@@ +4542,5 @@
> +      // we can act as if it was optimizable. When the primary frame dies, node
> +      // will end up to the purple buffer because of the refcount change.
> +      (!unoptimizable ||
> +       (currentDoc->GetShell() && aNode->IsElement() &&
> +        aNode->AsElement()->GetPrimaryFrame()))) {

Maybe turn these two lines into a separate function with a descriptive name, then move the comment up there?  I don't understand any of the unoptimizable stuff so I can't really comment on that.

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 595898 [details] [diff] [review]
wip

+      // If aNode is not optimizable, but it is still an element
+      // with a frame in a document which has currently active presshell,
+      // we can act as if it was optimizable. When the primary frame dies, node

"the node", or "aNode".

+      // will end up to the purple buffer because of the refcount change.
+      (!unoptimizable ||
+       (currentDoc->GetShell() && aNode->IsElement() &&
+        aNode->AsElement()->GetPrimaryFrame()))) {

What Andrew said. Maybe put the code after || in a static inlined helper named NodeHasFrame() or somesuch?

r=jst with that.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/b3073c517d21