Closed Bug 677529 Opened 8 years ago Closed 8 years ago

Remove manual jspubtd.h inclusions from IDL files

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: Ms2ger, Assigned: matjk7)

References

Details

Attachments

(2 files, 2 obsolete files)

No description provided.
Attached patch wip (obsolete) — Splinter Review
This should work, except it doesn't. Am I missing something here or does Bug 676649 not work as I understand it (i.e automatically including jspubtd.h if jsval is used as a type in an IDL)?
Attachment #552295 - Flags: feedback?(Ms2ger)
What went wrong? It's entirely possible I missed things like [ptr] native JSValPtr(jsval); in nsIJSON.idl.
Attached patch patch (obsolete) — Splinter Review
This one works.

(In reply to Ms2ger from comment #2)
> What went wrong? It's entirely possible I missed things like [ptr] native
> JSValPtr(jsval); in nsIJSON.idl.

nsIJSON is fine, possibly because no one uses it anymore IIRC. Other interfaces had problems though:

http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/nsIXPConnect.idl#69 doesn't work.

http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/nsIXPCSecurityManager.idl doesn't work, even though it doesn't use jsval.

http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsIVariant.idl#118 doesn't work.
Attachment #552295 - Attachment is obsolete: true
Attachment #552295 - Flags: feedback?(Ms2ger)
Attachment #552481 - Flags: review?(Ms2ger)
(In reply to Matheus Kerschbaum from comment #3)
> Created attachment 552481 [details] [diff] [review]
> patch
> 
> This one works.
> 
> (In reply to Ms2ger from comment #2)
> > What went wrong? It's entirely possible I missed things like [ptr] native
> > JSValPtr(jsval); in nsIJSON.idl.
> 
> nsIJSON is fine, possibly because no one uses it anymore IIRC. Other
> interfaces had problems though:
> 
> http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/
> nsIXPConnect.idl#69 doesn't work.
> 
> http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/
> nsIXPCSecurityManager.idl doesn't work, even though it doesn't use jsval.

I think it's file to leave these two with manual inclusions.

> http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsIVariant.idl#118
> doesn't work.

Looks like I didn't check return values; patch coming up.
Could you check if this fixes nsIVariant for you?
Attachment #552863 - Flags: feedback?
Attachment #552863 - Flags: feedback? → feedback?(matjk7)
Comment on attachment 552481 [details] [diff] [review]
patch

LGTM, but don't get this landed before comm-central uses pyidl.
Attachment #552481 - Flags: review?(Ms2ger) → review+
comm-central is using pyidl for interfaces in mozilla-central.  It's only interfaces in comm-central that are being built with xpidlc.
(In reply to Ms2ger from comment #5)
> Created attachment 552863 [details] [diff] [review]
> Patch to check method return values
> 
> Could you check if this fixes nsIVariant for you?

I don't have bugzilla rights to mark this as f+, but the patch does work.
Attached patch patchSplinter Review
Attachment #552481 - Attachment is obsolete: true
Attachment #552907 - Flags: review+
Comment on attachment 552863 [details] [diff] [review]
Patch to check method return values

Great, thanks.
Attachment #552863 - Flags: review?(khuey)
Attachment #552863 - Flags: feedback?(matjk7)
Attachment #552863 - Flags: feedback+
http://hg.mozilla.org/try/rev/2b7f988f5a6a
http://hg.mozilla.org/try/rev/e0634a052db4
Assignee: nobody → matjk7
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.