484898 - Remove unused *_TIMER macros in CNavDTD and nsViewSourceHTML

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Ben Newman (:bnewman) (:benjamn)]

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.

Comment 1

[Tommy]

Attachment: Removed *_TIMER macros in CNavDTD and nsViewSourceHTML

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.

Comment 2

[Ben Newman (:bnewman) (:benjamn)]

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.

Comment 3

[Tommy]

Attachment: Removed unused *_TIMER macros in CNavDTD and nsViewSourceHTML. Used Mercurial.

Comment 4

[Tommy]

Hi

I the new patch good with Mercurial?  

Regards
Tommy

Comment 5

[Ben Newman (:bnewman) (:benjamn)]

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 6

[Ben Newman (:bnewman) (:benjamn)]

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.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 391995 [details] [diff] [review]
Removed unused *_TIMER macros in CNavDTD and nsViewSourceHTML.  Used Mercurial.

Looks good!

Comment 8

[Tommy]

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

Regards
Tommy

Comment 9

[Ben Newman (:bnewman) (:benjamn)]

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

Thanks, Tommy!  Welcome to the project.