extend restyle logging with style struct information

RESOLVED FIXED in mozilla35

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: heycam, Assigned: heycam)

Tracking

Trunk
mozilla35
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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.
Created attachment 8494989 [details] [diff] [review]
patch
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8494989 - Flags: review?(dbaron)
Created attachment 8494991 [details]
sample output

Here's some sample output.  See the style context tree and the "swapping style structs" bit.
Ignore the stray printf I left in the patch.
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
Attachment #8494989 - Flags: review?(dbaron) → review+
(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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/298b1f34d02a
https://hg.mozilla.org/mozilla-central/rev/298b1f34d02a
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.