Closed Bug 1072724 Opened 5 years ago Closed 5 years ago

extend restyle logging with style struct information

Categories

(Core :: CSS Parsing and Computation, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: heycam, Assigned: heycam)

References

Details

Attachments

(2 files)

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.
Attached patch patchSplinter Review
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8494989 - Flags: review?(dbaron)
Attached file 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/mozilla-central/rev/298b1f34d02a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.