Now we can see what is really happening in Hash: first hash the
interfaces from the base set, if any, then hash the additional
interface, if any.
Note that this function is wrong when mBaseSet is null. This will be
fixed in bug 1290239.
MozReview-Commit-ID: KaxQ57ofO1D
--HG--
extra : rebase_source : 42012878839adb2e36580480abce7d9708288db4
Both cases first hash together all of the existing interfaces.
MozReview-Commit-ID: AnUF5uPSPpN
--HG--
extra : rebase_source : 43ac016974d3ee4dfbd92361348aeeae5b6a793c
Now I take advantage of knowing that any new interface is always being
added to the end of the set.
Further cleanup of Hash() will happen in the next patch.
MozReview-Commit-ID: EoESTOfIOr
--HG--
extra : rebase_source : 8471391d23e3ff27a27156f55badbef3d1a4118b
There are three cases for a key, represented by the three ctors:
1. mBaseSet is non-null, mAddition is null.
2. mBaseSet is null, mAddition is non-null.
3. Both mBaseSet and mAddition are non-null.
In the three places that use the value of mPosition, condition 3
holds, so the key must have been constructed using the third ctor. For
this ctor, mPosition is equal to mBaseSet->GetInterfaceCount(), so I
substitute the value and eliminate the field.
This makes a check in NewInstanceMutate() trivially false, so I
eliminated that, too.
MozReview-Commit-ID: 1SOF6GyccU7
--HG--
extra : rebase_source : e215da19d77d6f88c216a48a07b9450c4d0e12bb
The mPosition field will be eliminated in a later patch.
MozReview-Commit-ID: EyVYZGgUWrH
--HG--
extra : rebase_source : 229ec989485bdd3ef86670a7c7db4149281a8d79
This explicitly represents the three types of keys that are used:
1. A key for an existing set.
2. A key for a new set with one interface added.
3. A key for an existing set with one new interface added at a
specified position.
MozReview-Commit-ID: Ctw41EymHbd
--HG--
extra : rebase_source : d7ce7d608a3d09df752313116c99bc2079d15a13
XPCNativeSet::GetNewOrUsed() and ::NewInstanceMutate() essentially
take XPCNativeSetKeys as arguments, but pass them around
unboxed. Passing around the keys explicitly will allow later changes
to enforce stronger invariants on keys.
MozReview-Commit-ID: CyQU3bUGinq
--HG--
extra : rebase_source : 5cdc651c1e4b9566dccd61b66de5e2bb8d6f33f5
The patch is generated from following command:
rgrep -l unused.h|xargs sed -i -e s,mozilla/unused.h,mozilla/Unused.h,
MozReview-Commit-ID: AtLcWApZfES
--HG--
rename : mfbt/unused.h => mfbt/Unused.h
XPCNativeInterface::Mark(), Unmark() and IsMarked() don't do anything
any more, so anything that calls them can be deleted. This removes the
only use of XPCCallContext::CanGetInterface(), so delete that, too.
MozReview-Commit-ID: 4w3aPTVXNDI
There are four classes that call Root() on XPCNativeInterface, and
thus keep interfaces alive. Each of these gets converted to use a
RefPtr:
1. XPCCallContext. This could be on some kind of hot path, but the
FindMemberCall involves various string operations and hashtable
lookups, so adding a single AddRef shouldn't matter. One weirdness
here is that the context only roots the interface when |mState >=
HAVE_NAME|. With a RefPtr<>, this requires nulling out
mInterface. Fortunately, in most cases where it moves from rooting to
non-rooting, it already does this. The one case it does not is in
SystemIsBeingShutDown(), so my patch adds that.
2. XPCNativeSet. This holds an array of interfaces in a weird
placement new array at the end of the object. I wasn't sure how a
non-POD class would interact with the way the array is handled with
casting, so I manually AddRef and Release things put into or removed
from the array.
3. AutoMarkingNativeInterfacePtr simply becomes RefPtr<>. This is the
bulk of the patch, in terms of number of lines changed.
4. Similarly, the one AutoMarkingNativeInterfacePtrArrayPtr becomes
nsTArray<RefPtr<>>. This is the last use of the auto marking array
class, so I deleted it.
Here are some other notes on what the patch does:
- XPCNativeInterfaces are created with placement new. This requires a
special version of refcounting that calls DestroyInstance, defined in
the previous patch. The GetNewOrUsed methods used to explicitly call
DestroyInstance(), but with refcounting this is no longer needed.
- The Mark() etc. methods are gutted so they don't do anything and
mMarked is removed because it is no longer used. The methods will be
cleaned up in later patches in this bug.
- Interfaces are removed from mIID2NativeInterfaceMap in the dtor
instead of during sweeping, requiring an extra hash table lookup.
- All of the methods that can create a new interface (NewInstance,
GetISupports, GetNewOrUsed) now return an already_AddRefed<>, which
gives some static checking that we don't accidentally fail to hold
onto a newly created interface.
MozReview-Commit-ID: CrlH1ENAzvr
Like part 2, this patch is to work around a false GC hazard in
~XPCNativeInterface in part 4. This hazard is around the return value
of WrapperFactory::PrepareForWrapping(), because ~XPCCallContext might
call ~XPCNativeInterface. The fix is to pass the return value into a
mutable handle. Unfortunately, this function is used in the JSAPI, so
JS minor engine changes are also needed.
MozReview-Commit-ID: GwFxmrXFXmb
This is to work around false GC hazards in ~XPCNativeInterface in part
4 of this bug. Putting RefPtr<XPCNativeInterface> on the stack means
that ~XPCNativeInterface can get called in various places, and the GC
hazard analysis does not understand the virtual methods involved, so
it assumes they might possibly GC.
This fixes one hazard by taking a jsid handle argument. The callers
already pass in handles, so no other changes are needed.
MozReview-Commit-ID: LpNpTlujpkm
XPCNativeInterface::Mark(), Unmark() and IsMarked() don't do anything
any more, so anything that calls them can be deleted. This removes the
only use of XPCCallContext::CanGetInterface(), so delete that, too.
MozReview-Commit-ID: 4w3aPTVXNDI
There are four classes that call Root() on XPCNativeInterface, and
thus keep interfaces alive. Each of these gets converted to use a
RefPtr:
1. XPCCallContext. This could be on some kind of hot path, but the
FindMemberCall involves various string operations and hashtable
lookups, so adding a single AddRef shouldn't matter. One weirdness
here is that the context only roots the interface when |mState >=
HAVE_NAME|. With a RefPtr<>, this requires nulling out
mInterface. Fortunately, in most cases where it moves from rooting to
non-rooting, it already does this. The one case it does not is in
SystemIsBeingShutDown(), so my patch adds that.
2. XPCNativeSet. This holds an array of interfaces in a weird
placement new array at the end of the object. I wasn't sure how a
non-POD class would interact with the way the array is handled with
casting, so I manually AddRef and Release things put into or removed
from the array.
3. AutoMarkingNativeInterfacePtr simply becomes RefPtr<>. This is the
bulk of the patch, in terms of number of lines changed.
4. Similarly, the one AutoMarkingNativeInterfacePtrArrayPtr becomes
nsTArray<RefPtr<>>. This is the last use of the auto marking array
class, so I deleted it.
Here are some other notes on what the patch does:
- XPCNativeInterfaces are created with placement new. This requires a
special version of refcounting that calls DestroyInstance, defined in
the previous patch. The GetNewOrUsed methods used to explicitly call
DestroyInstance(), but with refcounting this is no longer needed.
- The Mark() etc. methods are gutted so they don't do anything and
mMarked is removed because it is no longer used. The methods will be
cleaned up in later patches in this bug.
- Interfaces are removed from mIID2NativeInterfaceMap in the dtor
instead of during sweeping, requiring an extra hash table lookup.
- All of the methods that can create a new interface (NewInstance,
GetISupports, GetNewOrUsed) now return an already_AddRefed<>, which
gives some static checking that we don't accidentally fail to hold
onto a newly created interface.
MozReview-Commit-ID: CrlH1ENAzvr
This adds a heap allocation in
XPCNativeScriptableSharedMap::GetNewOrUsed() on the fast
path. Hopefully that code is not hot enough for it to matter. I could
work around this if needed, but it would be ugly.
mNativeScriptableSharedMap has a weak pointer to
XPCNativeScriptableShared. I moved this removal from
XPCJSRuntime::FinalizeCallback() to the dtor.
There are two types of scriptable: one is a dummy one created in
GetNewOrUsed() to do a lookup, and the other is a fully filled out
one. The dummy one is not added to the hashtable, and may have had its
name stolen if we created a new scriptable. The latter makes it so
that you crash if you try to look it up in the hashtable anyways, so
this patch only looks up fully filled out scriptables.
This patch also removes MOZ_COUNT_CTOR/DTOR because they are not
needed for refcounted classes.
Stop destroying mScriptableInfo in XPCWrappedNative's
SystemIsBeingShutDown(), because that will end up destroying
XPCNativeScriptableShared, while their js::ClassOps are still in use
by JS objects. This matches the existing behavior, which does not
sweep these ScriptableShared during shutdown. (This came up in opt
xpcshell tests.)
MozReview-Commit-ID: GeG0pAYqXpR
This patch makes most Run() declarations in subclasses of nsIRunnable have the
same form: |NS_IMETHOD Run() override|.
As a result of these changes, I had to add |override| to a couple of other
functions to satisfy clang's -Winconsistent-missing-override warning.
--HG--
extra : rebase_source : 815d0018b0b13329bb5698c410f500dddcc3ee12
I could clean up ArrayAutoMarkingPtr more, but it is going to be
removed entirely in bug 1288870.
MozReview-Commit-ID: Jyjc2ZfvF3i
--HG--
extra : rebase_source : d7954ab821722b26fe5fc4f5ddc319dd824c6879
This file is included in caps/, but it only uses generic JS things,
aside from a macro.
AccessCheck.cpp was bootlegging xpcprivate.h.
MozReview-Commit-ID: C6fGOFxsTvg
--HG--
extra : rebase_source : bd5e7bf9010acf83ccab8ce6cce77a557ad76196
The basic idea is that we assume the invariant that the "obj" argument is never
gray if "existing" is null (and add asserts to that effect). Starting from that
assumption, terrence and I audited all the return paths to ensure that gray
objects are never returned. We found a few places, generally after crossing
compartments with UncheckedUnwrap, where we could have gray things and inserted
corresponding calls to ExposeObjectToActiveJS.
If "existing" is passed in, all bets are off: both "obj" and "existing" can be
gray and can get returned from here. But the only caller that passes "existing"
doesn't allow the return value to escape, so it's actually safe to do this.
