The default bug view has changed. See this FAQ.

Remove windows.h from JS headers, added in bug 588537

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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.
Blocks: 588537
(Assignee)

Comment 1

6 years ago
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.)
Attachment #545839 - Flags: review?(gal)
(Assignee)

Updated

6 years ago
Blocks: 673631
(Assignee)

Comment 2

6 years ago
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.
Attachment #545839 - Flags: review?(gal) → review?(dmandelin)
Attachment #545839 - Flags: review?(dmandelin) → review+
(Assignee)

Comment 3

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/eb2a432bdf1a
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/eb2a432bdf1a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.