197281 - Introduction of XPCConvert::GetNativeInterfaceFromJSObject is causing problems

Status: VERIFIED FIXED

(Core :: XPConnect, defect)

Description

[David Bradley]

I created this and used it early on in the IDispatch code, but no longer use it.
Unfortunately this refactoring changed the exit paths to the
JSObject2NativeInterface function. So I'm going to back this out.

Comment 1

[David Bradley]

Attachment: Back out GetNativeInterfaceFromJSObject

Comment 2

[David Bradley]

Oh, and I should have stated the behavior this resulted in. This was causing the
JSObject wrapping the native to get wrapped itself instead of bailing. For
instance in the case of document.appendChild(window);

The key is the identity->QueryInterface(*iid, dest)); call. In the current code,
this continues on into the next part of JSObject2NativeInterface. In the old
code, it returned regardless of success or failure from
JSObject2NativeInterface. That's not the only place either.

Comment 3

[David Bradley]

Comment on attachment 117151 [details] [diff] [review]
Back out GetNativeInterfaceFromJSObject

This patch is a pure diff between 1.78 and 1.79. But if there's anything a miss
in the old code I'll be happy to fix it.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 117151 [details] [diff] [review]
Back out GetNativeInterfaceFromJSObject

Just found a few nits:

+	     if(wrappedNative->GetIID().Equals(*iid))
+	     {
+		 NS_ADDREF(iface);
+		 *dest = iface;
+		 return JS_TRUE;
+	     }
+	     else
+		 return NS_SUCCEEDED(iface->QueryInterface(*iid, dest));

You could remove that else-after-return.

+	 if(GetISupportsFromJSObject(cx, src, &iface))
+	 {
+	     if(iface)
+		 return NS_SUCCEEDED(iface->QueryInterface(*iid, dest));
+	     return JS_FALSE;
+	 
+	 }

Remove that empty line after return JS_FALSE, or move the empty line above the
return.

r=jst

Comment 5

[David Bradley]

Comment on attachment 117151 [details] [diff] [review]
Back out GetNativeInterfaceFromJSObject

Alec, could you sr this. This is just a simple backout of part of my IDispatch
code patch. The patch is just the diff between 1.78 and 1.79 for this file.
I'll address jst's points on check in.

Comment 6

[Alec Flett]

Comment on attachment 117151 [details] [diff] [review]
Back out GetNativeInterfaceFromJSObject

sr=alecf

Comment 7

[David Bradley]

Patch checked in, with jst's nits addressed

Comment 8

[Phil Schwartau]

Checkin verified -