484692 - Set/clear cached wrappers from within XPConnect

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P1)

Description

[Peter Van der Beken [:peterv]]

Currently we do that from various PostCreate/Finalize hooks in nsDOMClassInfo, but we should really do it inside XPConnect. For setting we already have a pointer to the nsWrapperCache and for clearing the additional QI shouldn't cost much more than the virtual call to the Finalize hook. This will probably also help with slimwrappers, since we could then avoid setting the wrapper again after morphing.

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

With this, you can get rid of the recently added DocumentFragmentSH, which exists for the sole purpose of doing the wrapper dance.

Comment 3

[Peter Van der Beken [:peterv]]

Ah, yes. Jst: when reviewing please assume the patch includes a backout of http://hg.mozilla.org/mozilla-central/rev/825869c4798a :-).

Comment 4

[Peter Van der Beken [:peterv]]

I think we should actually take this patch on branch to fix bug 480975, instead of the patch in that bug. I don't think there are any other classes that have the same problem, but this patch fixes them if we do.

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 368800 [details] [diff] [review]
v1

r+sr=jst

Comment 6

[Johnny Stenback (:jst)]

Blocking (and unblocked on bug 480975).

Comment 7

[Peter Van der Beken [:peterv]]

http://hg.mozilla.org/mozilla-central/rev/1e484f30d821

Comment 8

[[e-mail]]

Attachment: Error check

QI isn't required to zero its outparam on failure, is it?

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 369252 [details] [diff] [review]
Error check

No, most implementations do. I'll land this on branch with the other patch.

Comment 10

[[e-mail]]

Comment on attachment 369252 [details] [diff] [review]
Error check

Pushed changeset bff47eac99af to mozilla-central.

Comment 11

[Peter Van der Beken [:peterv]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6642289b94f0
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e89cfcdf66d1

Comment 12

[Peter Van der Beken [:peterv]]

I backed this out on branch again to try to resolve some sporadic orange, see bug 486271. No clue what's going on.

Comment 13

[Peter Van der Beken [:peterv]]

Tried to reproduce the orange locally, but can't. Running under purify didn't give any clues either.

Comment 14

[Peter Van der Beken [:peterv]]

The problem turned out to be removal of the WANT_FINALIZE flag from NODE_SCRIPTABLE_FLAGS. Because the fix for bug 480389 wasn't landed on branch we still have a Finalize hook in nsDocumentSH, and so we need to add WANT_FINALIZE to DOCUMENT_SCRIPTABLE_FLAGS.