Closed Bug 880089 Opened 6 years ago Closed 6 years ago

Remove 'flags' parameter from nsIXPConnect.idl's setDefaultSecurityManager() method

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: dholbert, Assigned: Six)

References

Details

Attachments

(1 file, 2 obsolete files)

Per bug 880078 comment 3 and comment 4, these flags can be removed:
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/idl/nsIXPCSecurityManager.idl?mark=27-43#27
...and the method they're used in, setDefaultSecurityManager(), can lose its 'flags' parameter as well:
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/idl/nsIXPConnect.idl#443
Comment on attachment 759021 [details] [diff] [review]
Remove 'flags' parameter from setDefaultSecurityManager() methods

>+++ b/js/xpconnect/idl/nsIXPConnect.idl
>@@ -439,18 +439,17 @@ interface nsIXPConnect : nsISupports
>     [noscript, notxpcom] nsISupports
>     getNativeOfWrapper(in JSContextPtr aJSContext,
>                        in JSObjectPtr  aJSObj);
> 
>     /**
>     * The security manager to use when the current JSContext has no security
>     * manager.
>     */
>-    void setDefaultSecurityManager(in nsIXPCSecurityManager aManager,
>-                                   in uint16_t flags);
>+    void setDefaultSecurityManager(in nsIXPCSecurityManager aManager);

You need to update the IID of this interface, since it's got a method-signature that's changing.

>+++ b/js/xpconnect/src/nsXPConnect.cpp
> NS_IMETHODIMP
>-nsXPConnect::SetDefaultSecurityManager(nsIXPCSecurityManager *aManager,
>-                                       uint16_t flags)
>+nsXPConnect::SetDefaultSecurityManager(nsIXPCSecurityManager *aManager)
> {
>     NS_IF_ADDREF(aManager);
>     NS_IF_RELEASE(mDefaultSecurityManager);
>     mDefaultSecurityManager = aManager;
>     mDefaultSecurityManagerFlags = flags;

I don't think that "mDefaultSecurityManagerFlags = flags" line will compile anymore, now that 'flags' is gone, right?

I suspect bholley meant that mDefaultSecurityManagerFlags (and its accessor method) could go away, too.  (He should be the one to ultimately review this.)
Attachment #759021 - Flags: feedback+
Attachment #759021 - Flags: review?(dholbert)
New patch taking care of dholbert's comments
Attachment #759021 - Attachment is obsolete: true
Attachment #759031 - Flags: review?(bobbyholley+bmo)
Assignee: nobody → six.dsn
Status: NEW → ASSIGNED
Comment on attachment 759031 [details] [diff] [review]
Remove unused parameter 'flags' from nsIXPConnect.idl's in setDefaultSecurityManager() methods

Review of attachment 759031 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r=bholley if it compiles.
Attachment #759031 - Flags: review?(bobbyholley+bmo) → review+
Wrong version of the patch uploaded earlier, 
was missing modifications in js/xpconnect/src/XPCWrappedNative.cpp
build on my linux64, push to try:
https://tbpl.mozilla.org/?tree=Try&rev=539f9fa4b38c
Attachment #759031 - Attachment is obsolete: true
Attachment #759327 - Flags: review?(bobbyholley+bmo)
Didn't dholbert already make the XPCWrappedNative changes in bug 880078?
Attachment #759031 - Attachment is obsolete: false
Attachment #759327 - Attachment is obsolete: true
Attachment #759327 - Flags: review?(bobbyholley+bmo)
Yep this is why i didn't removed it first time...
i just did the changes to put the patch you reviewed as good
tbpl looks good too at this time
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5d33985da55b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.