Closed Bug 1452170 Opened 2 years ago Closed 2 years ago
Logging .h, Restyle Tracker Inlines .h and related RESTYLE _LOGGING cruft
No description provided.
Comment on attachment 8965773 [details] [diff] [review] patch Is this just a "remove dead code" bug? These functions aren't exactly dead -- they have callers in the tree (behind an #ifdef guard) here: https://dxr.mozilla.org/mozilla-central/source/layout/base/RestyleLogging.h You should probably tag heycam to confirm that this really is dead code (since he's got hg blame on this code), and you should probably remove the callers before or at the same time as removing these APIs.
Attachment #8965773 - Flags: review?(dholbert) → review-
Looks like I should morph this into "Remove dead RESTYLE_LOGGING code".
RestyleTrackerInlines.h isn't included by any file. RESTYLE_LOGGING isn't defined by anything.
(In reply to Jonathan Watt [:jwatt] from comment #4) > RESTYLE_LOGGING isn't defined by anything. Sure, but I assume the idea is/was that a developer would define it locally if they wanted extra logging. > RestyleTrackerInlines.h isn't included by any file. Still: probably better to remove the (#include-severed) callers from RestyleLogging.h at the same time. (Hypothetically, if we want to keep those callers around, then the right solution here would be to sprinkle around some includes for RestyleTrackerInlines.h, rather than removing it.)
Right, sorry I wasn't clear. Those two points were meant as a summary (along with comment 3) for Emilio to consider whether he thinks this logging should be resurected or removed.
We have restyle logging with RUST_LOG=style, using the standard log mechanism. So I don't think it's worth resurrecting this code IMO.
Yes, RestyleLogging.h can be removed. (One slightly nice thing about the old logging code, compared to the RUST_LOG=style one, is that it indented the logging based on the depth of the restyle, which made it a bit easier to visually scan to see where restyles where being cut off and so on. That's probably too tricky to do with the standard Rust logging mechanism, not to mention how it would work given work stealing / parallelism. In any case, that wouldn't involve RestyleLogging.h.)
To be clear, it would be all the RESTYLE_LOGGING code I'd remove, not just RestyleLogging.h https://searchfox.org/mozilla-central/search?q=RESTYLE_LOGGING
Yeah, all that can go.
Summary: Remove RestyleTrackerInlines.h → Remove RestyleLogging.h, RestyleTrackerInlines.h and related RESTYLE_LOGGING cruft
Comment on attachment 8966116 [details] [diff] [review] patch Review of attachment 8966116 [details] [diff] [review]: ----------------------------------------------------------------- Please remove RestyleLogging.h from layout/base/moz.build too.
Attachment #8966116 - Flags: review?(cam) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f892607541eb Remove RestyleLogging.h, RestyleTrackerInlines.h and related RESTYLE_LOGGING cruft. r=heycam
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/5c3a7a21dedd follow-up - Fix Windows unified build bustage by adding missing AutoRestore.h includes. r=me on CLOSED TREE
You need to log in before you can comment on or make changes to this bug.