Last Comment Bug 671035 - Remove windows.h from JS headers, added in bug 588537
: Remove windows.h from JS headers, added in bug 588537
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Steve Fink [:sfink] [:s:] (PTO Sep23-28)
:
Mentors:
Depends on:
Blocks: 588537 673631
  Show dependency treegraph
 
Reported: 2011-07-12 12:30 PDT by Steve Fink [:sfink] [:s:] (PTO Sep23-28)
Modified: 2011-08-02 03:22 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Move ETW implementations into jsprobes.cpp, eliminate windows.h include from header (27.95 KB, patch)
2011-07-13 23:17 PDT, Steve Fink [:sfink] [:s:] (PTO Sep23-28)
dmandelin: review+
Details | Diff | Splinter Review

Description Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-07-12 12:30:14 PDT
In bug 588537, I added an #include of "jswin.h", which includes <windows.h>, to jsprobes.h. Brendan mentioned to me that that's not such a great idea, which I agree with. I have a simple followup fix that I'm still trying to get to compile.
Comment 1 Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-07-13 23:17:00 PDT
Created attachment 545839 [details] [diff] [review]
Move ETW implementations into jsprobes.cpp, eliminate windows.h include from header

This patch removes all ETW code from the jsprobes.h header and moves it into jsprobes.cpp. This removes windows.h pollution in JS header files, and the loss of inlining is unlikely to matter given ETW's overhead. (As in, it's already doing non-inlined function calls.)

As part of this, I added 3 new probe types, still lacking dtrace etc implementations: runtime create/destroy and final shutdown.

This also fixes a build failure I was getting with the bug 588537 patches, which mysteriously did not seem to affect my try pushes for some reason. The fix is to make only jsprobes.obj depend on the generated ETWProvider.h instead of all object files. (Without this patch, many of those files really did depend on it, but the rule was wrong so it also added in some *.cc files.)
Comment 2 Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-07-26 14:18:59 PDT
Comment on attachment 545839 [details] [diff] [review]
Move ETW implementations into jsprobes.cpp, eliminate windows.h include from header

I should stop dumping all probe-related reviews on gal.
Comment 3 Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-08-01 16:08:30 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/eb2a432bdf1a
Comment 4 Marco Bonardo [::mak] 2011-08-02 03:22:12 PDT
http://hg.mozilla.org/mozilla-central/rev/eb2a432bdf1a

Note You need to log in before you can comment on or make changes to this bug.