1072724 - extend restyle logging with style struct information

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

Something that would also be useful to see in restyle logging is what the state of the style context tree is, including its cached struct pointers, before each individual restyle is processed.

Comment 1

[Cameron McCormack (:heycam)]

Attachment: patch

Comment 2

[Cameron McCormack (:heycam)]

Attachment: sample output

Here's some sample output.  See the style context tree and the "swapping style structs" bit.

Comment 3

[Cameron McCormack (:heycam)]

Ignore the stray printf I left in the patch.

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 8494989 [details] [diff] [review]
patch

Could you comment above nsStyleContext::ShouldLogRestyle() that it's
called by macros that are used only in functions that are called only
when PresContext()->RestyleManager()->ShouldLogRestyle() already returns
true?  (If that's not true, then perhaps it should call through to it?)

RestyleManager::StructsToLog:

>+      nsDependentCString v(value);
>+      nsCString s(v);

No need for v; just construct s from value.

Remove RestyleManager::ShouldLogStyleContextTrees(), which is unused.

But maybe, in RestyleTracker, you should avoid calling LogStyleContextTree
if StructsToLog() is 0.

>+  for (nsStyleStructID i = nsStyleStructID_Inherited_Start;
>+       i < nsStyleStructID_Length;
>+       i = nsStyleStructID(i + 1)) {

Initialize to nsStyleStructID(0) rather than
nsStyleStructID_Inherited_Start (which avoids assuming whether the
inherited or reset IDs come first).

>+    pseudo.Append(NS_ConvertUTF16toUTF8(pseudoTag).get());

No ".get()", please.  You're just forcing a recomputation of
a length you already have.

Although, really, you could just use AppendUTF16toUTF8.

r=dbaron with that

Comment 5

[Cameron McCormack (:heycam)]

(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #4)
> Remove RestyleManager::ShouldLogStyleContextTrees(), which is unused.
>
> But maybe, in RestyleTracker, you should avoid calling LogStyleContextTree
> if StructsToLog() is 0.

That's where I meant to include the ShouldLogStyleContextTrees call.  I'll just use StructsToLog() == 0.

Comment 6

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/298b1f34d02a

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/298b1f34d02a