Remove unused *_TIMER macros in CNavDTD and nsViewSourceHTML

RESOLVED FIXED

Status

()

defect
--
trivial
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: mozilla+ben, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug], )

Attachments

(1 attachment, 1 obsolete attachment)

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

Comment 1

10 years ago
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)
Reporter

Comment 2

10 years ago
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+

Comment 4

10 years ago
Hi

I the new patch good with Mercurial?  

Regards
Tommy
Reporter

Updated

10 years ago
Attachment #391995 - Flags: review?(bnewman) → review+
Reporter

Comment 5

10 years ago
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.
Reporter

Comment 6

10 years ago
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!

Comment 8

10 years ago
no problem Ben
thanks I believe this is the start of a new hobby ;)  
May there be many more.

Regards
Tommy
Reporter

Comment 9

10 years ago
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/f58cd3435c4d

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