Skip to content

Commit bd0f2fc

Browse files
committed
Bug 1393592 - manually de-virtualize nsIWeakReference::QueryReferent; r=ehsan
The virtual call to this method shows up in profiles, and therefore would be nice to avoid if possible. We can do that by making nsIWeakReference hold the weak reference itself and therefore implementing a non-virtual QueryReferent method.
1 parent 0b3b189 commit bd0f2fc

File tree

4 files changed

+72
-44
lines changed

4 files changed

+72
-44
lines changed

dom/base/FragmentOrElement.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -693,18 +693,19 @@ NS_IMPL_ISUPPORTS(nsNodeWeakReference,
693693

694694
nsNodeWeakReference::~nsNodeWeakReference()
695695
{
696-
if (mNode) {
697-
NS_ASSERTION(mNode->Slots()->mWeakReference == this,
696+
nsINode* node = static_cast<nsINode*>(mObject);
697+
698+
if (node) {
699+
NS_ASSERTION(node->Slots()->mWeakReference == this,
698700
"Weak reference has wrong value");
699-
mNode->Slots()->mWeakReference = nullptr;
701+
node->Slots()->mWeakReference = nullptr;
700702
}
701703
}
702704

703705
NS_IMETHODIMP
704-
nsNodeWeakReference::QueryReferent(const nsIID& aIID, void** aInstancePtr)
706+
nsNodeWeakReference::QueryReferentFromScript(const nsIID& aIID, void** aInstancePtr)
705707
{
706-
return mNode ? mNode->QueryInterface(aIID, aInstancePtr) :
707-
NS_ERROR_NULL_POINTER;
708+
return QueryReferent(aIID, aInstancePtr);
708709
}
709710

710711
size_t

dom/base/FragmentOrElement.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class nsNodeWeakReference final : public nsIWeakReference
5353
{
5454
public:
5555
explicit nsNodeWeakReference(nsINode* aNode)
56-
: mNode(aNode)
56+
: nsIWeakReference(aNode)
5757
{
5858
}
5959

@@ -63,17 +63,14 @@ class nsNodeWeakReference final : public nsIWeakReference
6363
// nsIWeakReference
6464
NS_DECL_NSIWEAKREFERENCE
6565
virtual size_t SizeOfOnlyThis(mozilla::MallocSizeOf aMallocSizeOf) const override;
66-
virtual bool IsAlive() const override { return mNode != nullptr; }
6766

6867
void NoticeNodeDestruction()
6968
{
70-
mNode = nullptr;
69+
mObject = nullptr;
7170
}
7271

7372
private:
7473
~nsNodeWeakReference();
75-
76-
nsINode* MOZ_NON_OWNING_REF mNode;
7774
};
7875

7976
/**

xpcom/base/nsIWeakReference.idl

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,28 @@
77
#include "nsISupports.idl"
88

99
%{C++
10+
#include "mozilla/Attributes.h"
1011
#include "mozilla/MemoryReporting.h"
12+
13+
// For MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED.
14+
#include "nsDebug.h"
15+
16+
#ifdef MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED
17+
18+
#define MOZ_WEAKREF_DECL_OWNINGTHREAD nsAutoOwningThread _mWeakRefOwningThread;
19+
#define MOZ_WEAKREF_ASSERT_OWNINGTHREAD \
20+
_mWeakRefOwningThread.AssertOwnership("nsWeakReference not thread-safe")
21+
#define MOZ_WEAKREF_ASSERT_OWNINGTHREAD_DELEGATED(that) \
22+
(that)->_mWeakRefOwningThread.AssertOwnership("nsWeakReference not thread-safe")
23+
24+
#else
25+
26+
#define MOZ_WEAKREF_DECL_OWNINGTHREAD
27+
#define MOZ_WEAKREF_ASSERT_OWNINGTHREAD do { } while (false)
28+
#define MOZ_WEAKREF_ASSERT_OWNINGTHREAD_DELEGATED(that) do { } while (false)
29+
30+
#endif
31+
1132
%}
1233

1334
/**
@@ -33,15 +54,36 @@ interface nsIWeakReference : nsISupports
3354
* like (a proxied) |QueryInterface|. Don't hold on to the produced interface permanently;
3455
* that would defeat the purpose of using a non-owning |nsIWeakReference| in the first place.
3556
*/
57+
[binaryname(QueryReferentFromScript)]
3658
void QueryReferent( in nsIIDRef uuid, [iid_is(uuid), retval] out nsQIResult result );
3759

3860
%{C++
39-
virtual size_t SizeOfOnlyThis(mozilla::MallocSizeOf aMallocSizeOf) const = 0;
61+
virtual size_t SizeOfOnlyThis(mozilla::MallocSizeOf aMallocSizeOf) const = 0;
4062

41-
/**
42-
* Returns true if the referring object is alive. Otherwise, false.
43-
*/
44-
virtual bool IsAlive() const = 0;
63+
/**
64+
* Returns true if the referring object is alive. Otherwise, false.
65+
*/
66+
bool IsAlive() const
67+
{
68+
return !!mObject;
69+
}
70+
71+
nsresult QueryReferent(const nsIID& aIID, void** aInstancePtr);
72+
73+
protected:
74+
friend class nsSupportsWeakReference;
75+
76+
nsIWeakReference(nsISupports* aObject)
77+
: mObject(aObject)
78+
{
79+
}
80+
81+
nsIWeakReference() = delete;
82+
83+
MOZ_WEAKREF_DECL_OWNINGTHREAD
84+
85+
// The object we're holding a weak reference to.
86+
nsISupports* MOZ_NON_OWNING_REF mObject;
4587
%}
4688
};
4789

xpcom/base/nsWeakReference.cpp

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,6 @@
1212
#include "nsCOMPtr.h"
1313
#include "nsDebug.h"
1414

15-
#ifdef MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED
16-
17-
#define MOZ_WEAKREF_DECL_OWNINGTHREAD nsAutoOwningThread _mWeakRefOwningThread;
18-
#define MOZ_WEAKREF_ASSERT_OWNINGTHREAD \
19-
_mWeakRefOwningThread.AssertOwnership("nsWeakReference not thread-safe")
20-
#define MOZ_WEAKREF_ASSERT_OWNINGTHREAD_DELEGATED(that) \
21-
(that)->_mWeakRefOwningThread.AssertOwnership("nsWeakReference not thread-safe")
22-
23-
#else
24-
25-
#define MOZ_WEAKREF_DECL_OWNINGTHREAD
26-
#define MOZ_WEAKREF_ASSERT_OWNINGTHREAD do { } while (false)
27-
#define MOZ_WEAKREF_ASSERT_OWNINGTHREAD_DELEGATED(that) do { } while (false)
28-
29-
#endif
30-
3115
class nsWeakReference final : public nsIWeakReference
3216
{
3317
public:
@@ -37,15 +21,12 @@ class nsWeakReference final : public nsIWeakReference
3721
// nsIWeakReference...
3822
NS_DECL_NSIWEAKREFERENCE
3923
size_t SizeOfOnlyThis(mozilla::MallocSizeOf aMallocSizeOf) const override;
40-
bool IsAlive() const override { return mReferent != nullptr; }
4124

4225
private:
43-
MOZ_WEAKREF_DECL_OWNINGTHREAD
44-
4526
friend class nsSupportsWeakReference;
4627

4728
explicit nsWeakReference(nsSupportsWeakReference* aReferent)
48-
: mReferent(aReferent)
29+
: nsIWeakReference(aReferent)
4930
// ...I can only be constructed by an |nsSupportsWeakReference|
5031
{
5132
}
@@ -54,8 +35,8 @@ class nsWeakReference final : public nsIWeakReference
5435
// ...I will only be destroyed by calling |delete| myself.
5536
{
5637
MOZ_WEAKREF_ASSERT_OWNINGTHREAD;
57-
if (mReferent) {
58-
mReferent->NoticeProxyDestruction();
38+
if (mObject) {
39+
static_cast<nsSupportsWeakReference*>(mObject)->NoticeProxyDestruction();
5940
}
6041
}
6142

@@ -64,10 +45,8 @@ class nsWeakReference final : public nsIWeakReference
6445
// ...called (only) by an |nsSupportsWeakReference| from _its_ dtor.
6546
{
6647
MOZ_WEAKREF_ASSERT_OWNINGTHREAD;
67-
mReferent = nullptr;
48+
mObject = nullptr;
6849
}
69-
70-
nsSupportsWeakReference* MOZ_NON_OWNING_REF mReferent;
7150
};
7251

7352
nsresult
@@ -140,12 +119,21 @@ nsSupportsWeakReference::GetWeakReference(nsIWeakReference** aInstancePtr)
140119
NS_IMPL_ISUPPORTS(nsWeakReference, nsIWeakReference)
141120

142121
NS_IMETHODIMP
143-
nsWeakReference::QueryReferent(const nsIID& aIID, void** aInstancePtr)
122+
nsWeakReference::QueryReferentFromScript(const nsIID& aIID, void** aInstancePtr)
123+
{
124+
return QueryReferent(aIID, aInstancePtr);
125+
}
126+
127+
nsresult
128+
nsIWeakReference::QueryReferent(const nsIID& aIID, void** aInstancePtr)
144129
{
145130
MOZ_WEAKREF_ASSERT_OWNINGTHREAD;
146131

147-
return mReferent ? mReferent->QueryInterface(aIID, aInstancePtr) :
148-
NS_ERROR_NULL_POINTER;
132+
if (!mObject) {
133+
return NS_ERROR_NULL_POINTER;
134+
}
135+
136+
return mObject->QueryInterface(aIID, aInstancePtr);
149137
}
150138

151139
size_t

0 commit comments

Comments
 (0)