Closed Bug 484898 Opened 11 years ago Closed 11 years ago

Remove unused *_TIMER macros in CNavDTD and nsViewSourceHTML

Categories

(Core :: DOM: HTML Parser, defect)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED

People

(Reporter: mozilla+ben, Unassigned)

References

()

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 1 obsolete file)

Given that bug 484874 removed the mParser members from our DTDs, START_TIMER and STOP_TIMER no longer have any timers to start or stop.  Those macros and related logging code should be removed.  We should perhaps also reconsider whether we need nsParser::mParseTime and nsParser::mDTDTime at all.

mrbkap advises that future profiling mechanisms should be implemented with DTRACE.
Severity: normal → trivial
Whiteboard: [good first bug]
Version: unspecified → Trunk
Hi I'm new to patching bugs and would like to familiarise myself with the process.  The TIMER macros and related loggers have been removed from both CNavDTD.cpp and nsViewSourceHTML.cpp files.
Attachment #391550 - Flags: review?(bnewman)
Comment on attachment 391550 [details] [diff] [review]
Removed *_TIMER macros in CNavDTD and nsViewSourceHTML

Changes look great.  Thanks for taking up the reins here, and welcome to the project!

It seems you're still using CVS.  Any good reason?  Mercurial makes it much easier to create and manage patches; see here for detailed info:

  https://developer.mozilla.org/en/Mercurial_FAQ

If you don't mind, I'd appreciate it if you'd reapply these changes to a fresh hg clone of mozilla-central, and regenerate the patch using hg diff.
Attachment #391550 - Flags: review?(bnewman) → review+
Attachment #391550 - Attachment is obsolete: true
Attachment #391995 - Flags: review?(bnewman)
Hi

I the new patch good with Mercurial?  

Regards
Tommy
Attachment #391995 - Flags: review?(bnewman) → review+
Comment on attachment 391995 [details] [diff] [review]
Removed unused *_TIMER macros in CNavDTD and nsViewSourceHTML.  Used Mercurial.

Looks great.  Sorry for the delay (you were right to bump me).  I ran the patch past the tryserver (https://wiki.mozilla.org/Build:TryServer) just to make absolutely sure there wasn't any code using these timers, and there were no problems.

As far as I'm concerned, this patch is ready to land; I just need to make sure my review is sufficient.
Comment on attachment 391995 [details] [diff] [review]
Removed unused *_TIMER macros in CNavDTD and nsViewSourceHTML.  Used Mercurial.

Since you're the module owner, jst, could you take a quick look at this?

Sorry for the delay, Tommy.
Attachment #391995 - Flags: superreview?(jst)
Attachment #391995 - Flags: superreview?(jst) → superreview+
Comment on attachment 391995 [details] [diff] [review]
Removed unused *_TIMER macros in CNavDTD and nsViewSourceHTML.  Used Mercurial.

Looks good!
no problem Ben
thanks I believe this is the start of a new hobby ;)  
May there be many more.

Regards
Tommy
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/f58cd3435c4d

Thanks, Tommy!  Welcome to the project.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.