Skip to content

Commit 50c9436

Browse files
committed
Bug 1869053 - [5/5] templatize and further simplify AgileReference r=handyman,win-reviewers
Lift `aIid` to compile-time, as a template parameter `InterfaceT`. This simplifies the common case for using `Resolve()`, where the desired and supplied interfaces are the same. (For the as-yet-unattested case where they're not, retain the old functionality by means of a small family of `ResolveAs()` functions.) Additionally, to eliminate a swath of custom logic and magic-number choices surrounding `mHResult`, eliminate `mHResult` itself as well. Instead, since its value was derived from the creation of the underlying `IAgileReference`, any callers that might care can acquire it as an additional return value from a named-constructor function. These collectively trim `AgileReference`'s footprint down to a single `RefPtr`, with all its special member functions having only default implementations. Differential Revision: https://phabricator.services.mozilla.com/D196513
1 parent d1a2476 commit 50c9436

File tree

4 files changed

+118
-138
lines changed

4 files changed

+118
-138
lines changed

ipc/mscom/AgileReference.cpp

+12-68
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,7 @@
77
#include "mozilla/mscom/AgileReference.h"
88

99
#include <utility>
10-
1110
#include "mozilla/Assertions.h"
12-
#include "mozilla/DebugOnly.h"
13-
#include "mozilla/DynamicallyLinkedFunctionPtr.h"
1411
#include "mozilla/mscom/Utils.h"
1512

1613
#if defined(__MINGW32__)
@@ -40,81 +37,28 @@ static const mozilla::StaticDynamicallyLinkedFunctionPtr<
4037

4138
#endif // defined(__MINGW32__)
4239

43-
namespace mozilla::mscom {
44-
45-
AgileReference::AgileReference() : mIid(), mHResult(E_NOINTERFACE) {}
46-
47-
AgileReference::AgileReference(REFIID aIid, IUnknown* aObject)
48-
: mIid(aIid), mHResult(E_UNEXPECTED) {
49-
AssignInternal(aObject);
50-
}
51-
52-
AgileReference::AgileReference(AgileReference&& aOther) noexcept
53-
: mIid(aOther.mIid),
54-
mAgileRef(std::move(aOther.mAgileRef)),
55-
mHResult(aOther.mHResult) {
56-
aOther.mHResult = CO_E_RELEASED;
57-
}
58-
59-
void AgileReference::Assign(REFIID aIid, IUnknown* aObject) {
60-
Clear();
61-
mIid = aIid;
62-
AssignInternal(aObject);
63-
}
64-
65-
void AgileReference::AssignInternal(IUnknown* aObject) {
66-
// We expect mIid to already be set
67-
DebugOnly<IID> zeroIid = {};
68-
MOZ_ASSERT(mIid != zeroIid);
40+
namespace mozilla::mscom::detail {
6941

42+
HRESULT AgileReference_CreateImpl(RefPtr<IAgileReference>& aRefPtr, REFIID riid,
43+
IUnknown* aObject) {
7044
MOZ_ASSERT(aObject);
71-
72-
mHResult = RoGetAgileReference(AGILEREFERENCE_DEFAULT, mIid, aObject,
73-
getter_AddRefs(mAgileRef));
74-
}
75-
76-
AgileReference::~AgileReference() { Clear(); }
77-
78-
void AgileReference::Clear() {
79-
mIid = {};
80-
mAgileRef = nullptr;
81-
mHResult = E_NOINTERFACE;
82-
}
83-
84-
AgileReference& AgileReference::operator=(const AgileReference& aOther) {
85-
Clear();
86-
mIid = aOther.mIid;
87-
mAgileRef = aOther.mAgileRef;
88-
mHResult = aOther.mHResult;
89-
return *this;
90-
}
91-
92-
AgileReference& AgileReference::operator=(AgileReference&& aOther) noexcept {
93-
Clear();
94-
mIid = aOther.mIid;
95-
mAgileRef = std::move(aOther.mAgileRef);
96-
mHResult = aOther.mHResult;
97-
aOther.mHResult = CO_E_RELEASED;
98-
return *this;
45+
MOZ_ASSERT(IsCOMInitializedOnCurrentThread());
46+
return ::RoGetAgileReference(AGILEREFERENCE_DEFAULT, riid, aObject,
47+
getter_AddRefs(aRefPtr));
9948
}
10049

101-
HRESULT
102-
AgileReference::ResolveRaw(REFIID aIid, void** aOutInterface) const {
50+
HRESULT AgileReference_ResolveImpl(RefPtr<IAgileReference> const& aRefPtr,
51+
REFIID riid, void** aOutInterface) {
52+
MOZ_ASSERT(aRefPtr);
10353
MOZ_ASSERT(aOutInterface);
104-
MOZ_ASSERT(mAgileRef);
10554
MOZ_ASSERT(IsCOMInitializedOnCurrentThread());
10655

107-
if (!aOutInterface) {
56+
if (!aRefPtr || !aOutInterface) {
10857
return E_INVALIDARG;
10958
}
11059

11160
*aOutInterface = nullptr;
112-
113-
if (mAgileRef) {
114-
return mAgileRef->Resolve(aIid, aOutInterface);
115-
}
116-
117-
return E_NOINTERFACE;
61+
return aRefPtr->Resolve(riid, aOutInterface);
11862
}
11963

120-
} // namespace mozilla::mscom
64+
} // namespace mozilla::mscom::detail

ipc/mscom/AgileReference.h

+97-57
Original file line numberDiff line numberDiff line change
@@ -9,95 +9,135 @@
99

1010
#include "mozilla/Attributes.h"
1111
#include "mozilla/RefPtr.h"
12+
#include "mozilla/Result.h"
13+
#include "mozilla/Unused.h"
14+
#include "nsDebug.h"
1215
#include "nsISupportsImpl.h"
1316

1417
#include <objidl.h>
1518

1619
namespace mozilla::mscom {
1720

21+
namespace detail {
22+
// Detemplatized implementation details of `AgileReference`.
23+
HRESULT AgileReference_CreateImpl(RefPtr<IAgileReference>&, REFIID, IUnknown*);
24+
HRESULT AgileReference_ResolveImpl(RefPtr<IAgileReference> const&, REFIID,
25+
void**);
26+
} // namespace detail
27+
1828
/**
19-
* This class encapsulates an "agile reference." These are references that
20-
* allow you to pass COM interfaces between apartments. When you have an
21-
* interface that you would like to pass between apartments, you wrap that
22-
* interface in an AgileReference and pass the agile reference instead. Then
23-
* you unwrap the interface by calling AgileReference::Resolve.
29+
* This class encapsulates an "agile reference". These are references that allow
30+
* you to pass COM interfaces between apartments. When you have an interface
31+
* that you would like to pass between apartments, you wrap that interface in an
32+
* AgileReference and pass that instead. Then you can "unwrap" the interface by
33+
* calling Resolve(), which will return a proxy object implementing the same
34+
* interface.
2435
*
2536
* Sample usage:
2637
*
27-
* // In the multithreaded apartment, foo is an IFoo*
28-
* auto myAgileRef = AgileReference(IID_IFoo, foo);
29-
*
30-
* // myAgileRef is passed to our main thread, which runs in a single-threaded
31-
* // apartment:
32-
*
33-
* RefPtr<IFoo> foo;
34-
* HRESULT hr = myAgileRef.Resolve(IID_IFoo, getter_AddRefs(foo));
35-
* // Now foo may be called from the main thread
38+
* ```
39+
* // From a non-main thread, where `foo` is an `IFoo*` or `RefPtr<IFoo>`:
40+
* auto myAgileRef = AgileReference(foo);
41+
* NS_DispatchToMainThread([mar = std::move(myAgileRef)] {
42+
* RefPtr<IFoo> foo = mar.Resolve();
43+
* // Now methods may be invoked on `foo`
44+
* });
45+
* ```
3646
*/
47+
template <typename InterfaceT>
3748
class AgileReference final {
38-
public:
39-
AgileReference();
40-
41-
template <typename InterfaceT>
42-
explicit AgileReference(RefPtr<InterfaceT>& aObject)
43-
: AgileReference(__uuidof(InterfaceT), aObject) {}
49+
static_assert(
50+
std::is_base_of_v<IUnknown, InterfaceT>,
51+
"template parameter of AgileReference must be a COM interface type");
4452

45-
AgileReference(REFIID aIid, IUnknown* aObject);
53+
public:
54+
AgileReference() = default;
55+
~AgileReference() = default;
4656

4757
AgileReference(const AgileReference& aOther) = default;
48-
AgileReference(AgileReference&& aOther) noexcept;
49-
50-
~AgileReference();
58+
AgileReference(AgileReference&& aOther) noexcept = default;
5159

52-
explicit operator bool() const { return !!mAgileRef; }
60+
AgileReference& operator=(const AgileReference& aOther) = default;
61+
AgileReference& operator=(AgileReference&& aOther) noexcept = default;
5362

54-
HRESULT GetHResult() const { return mHResult; }
63+
AgileReference& operator=(std::nullptr_t) {
64+
mAgileRef = nullptr;
65+
return *this;
66+
}
5567

56-
template <typename T>
57-
void Assign(const RefPtr<T>& aOther) {
58-
Assign(__uuidof(T), aOther);
68+
// Create a new AgileReference from an existing COM object.
69+
//
70+
// These constructors do not provide the HRESULT on failure. If that's
71+
// desired, use `AgileReference::Create()`, below.
72+
explicit AgileReference(InterfaceT* aObject) {
73+
HRESULT const hr = detail::AgileReference_CreateImpl(
74+
mAgileRef, __uuidof(InterfaceT), aObject);
75+
Unused << NS_WARN_IF(FAILED(hr));
76+
}
77+
explicit AgileReference(RefPtr<InterfaceT> const& aObject)
78+
: AgileReference(aObject.get()) {}
79+
80+
// Create a new AgileReference from an existing COM object, or alternatively,
81+
// return the HRESULT explaining why one couldn't be created.
82+
//
83+
// A convenience wrapper `MakeAgileReference()` which infers `InterfaceT` from
84+
// the RefPtr's concrete type is provided below.
85+
static Result<AgileReference<InterfaceT>, HRESULT> Create(
86+
RefPtr<InterfaceT> const& aObject) {
87+
AgileReference ret;
88+
HRESULT const hr = detail::AgileReference_CreateImpl(
89+
ret.mAgileRef, __uuidof(InterfaceT), aObject.get());
90+
if (FAILED(hr)) {
91+
return Err(hr);
92+
}
93+
return ret;
5994
}
6095

61-
// Raw version, and implementation, of Resolve(). Can be used directly if
62-
// necessary, but in general, prefer one of the templated versions below
63-
// (depending on whether or not you need the HRESULT).
64-
HRESULT ResolveRaw(REFIID aIid, void** aOutInterface) const;
96+
explicit operator bool() const { return !!mAgileRef; }
6597

66-
template <typename Interface>
67-
HRESULT Resolve(RefPtr<Interface>& aOutInterface) const {
68-
return this->ResolveRaw(__uuidof(Interface), getter_AddRefs(aOutInterface));
98+
// Common case: resolve directly to the originally-specified interface-type.
99+
RefPtr<InterfaceT> Resolve() const {
100+
auto res = ResolveAs<InterfaceT>();
101+
if (res.isErr()) return nullptr;
102+
return res.unwrap();
69103
}
70104

71-
template <typename T>
72-
RefPtr<T> Resolve() {
73-
RefPtr<T> p;
74-
Resolve<T>(p);
105+
// Uncommon cases: resolve directly to a different interface type, and/or
106+
// provide IAgileReference::Resolve()'s HRESULT.
107+
//
108+
// When used in other COM apartments, `IAgileInterface::Resolve()` returns a
109+
// proxy object which (at time of writing) is not documented to provide any
110+
// interface other than the one for which it was instantiated. (Calling
111+
// `QueryInterface` _might_ work, but isn't explicitly guaranteed.)
112+
//
113+
template <typename OtherInterface = InterfaceT>
114+
Result<RefPtr<OtherInterface>, HRESULT> ResolveAs() const {
115+
RefPtr<OtherInterface> p;
116+
auto const hr = ResolveRaw(__uuidof(OtherInterface), getter_AddRefs(p));
117+
if (FAILED(hr)) {
118+
return Err(hr);
119+
}
75120
return p;
76121
}
77122

78-
AgileReference& operator=(const AgileReference& aOther);
79-
AgileReference& operator=(AgileReference&& aOther) noexcept;
80-
81-
AgileReference& operator=(decltype(nullptr)) {
82-
Clear();
83-
return *this;
123+
// Raw version of Resolve/ResolveAs. Rarely, if ever, preferable to the
124+
// statically-typed versions.
125+
HRESULT ResolveRaw(REFIID aIid, void** aOutInterface) const {
126+
return detail::AgileReference_ResolveImpl(mAgileRef, aIid, aOutInterface);
84127
}
85128

86-
void Clear();
87-
88-
private:
89-
void Assign(REFIID aIid, IUnknown* aObject);
90-
void AssignInternal(IUnknown* aObject);
91-
92129
private:
93-
// The interface ID with which this reference was constructed.
94-
IID mIid;
95130
RefPtr<IAgileReference> mAgileRef;
96-
// The result associated with this reference's construction. May be modified
97-
// when mAgileRef changes, but is explicitly not touched by `Resolve`.
98-
HRESULT mHResult;
99131
};
100132

133+
// Attempt to create an AgileReference from a refcounted interface pointer,
134+
// providing the HRESULT as a secondary return-value.
135+
template <typename InterfaceT>
136+
inline Result<AgileReference<InterfaceT>, HRESULT> MakeAgileReference(
137+
RefPtr<InterfaceT> const& aObj) {
138+
return AgileReference<InterfaceT>::Create(aObj);
139+
}
140+
101141
} // namespace mozilla::mscom
102142

103143
#endif // mozilla_mscom_AgileReference_h

widget/windows/LegacyJumpListBuilder.cpp

+7-12
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,6 @@ LegacyJumpListBuilder::LegacyJumpListBuilder()
130130
observerService->AddObserver(this, TOPIC_PROFILE_BEFORE_CHANGE, false);
131131
observerService->AddObserver(this, TOPIC_CLEAR_PRIVATE_DATA, false);
132132
}
133-
134-
RefPtr jumpListMgr = mJumpListMgr.Resolve<ICustomDestinationList>();
135-
if (!jumpListMgr) {
136-
return;
137-
}
138133
}
139134

140135
LegacyJumpListBuilder::~LegacyJumpListBuilder() {
@@ -146,7 +141,7 @@ NS_IMETHODIMP LegacyJumpListBuilder::SetAppUserModelID(
146141
ReentrantMonitorAutoEnter lock(mMonitor);
147142
if (!mJumpListMgr) return NS_ERROR_NOT_AVAILABLE;
148143

149-
RefPtr jumpListMgr = mJumpListMgr.Resolve<ICustomDestinationList>();
144+
RefPtr<ICustomDestinationList> jumpListMgr = mJumpListMgr.Resolve();
150145
if (!jumpListMgr) {
151146
return NS_ERROR_NOT_AVAILABLE;
152147
}
@@ -187,7 +182,7 @@ NS_IMETHODIMP LegacyJumpListBuilder::GetMaxListItems(int16_t* aMaxItems) {
187182
return NS_OK;
188183
}
189184

190-
RefPtr jumpListMgr = mJumpListMgr.Resolve<ICustomDestinationList>();
185+
RefPtr<ICustomDestinationList> jumpListMgr = mJumpListMgr.Resolve();
191186
if (!jumpListMgr) {
192187
return NS_ERROR_UNEXPECTED;
193188
}
@@ -260,7 +255,7 @@ void LegacyJumpListBuilder::DoInitListBuild(RefPtr<Promise>&& aPromise) {
260255
}));
261256
});
262257

263-
RefPtr jumpListMgr = mJumpListMgr.Resolve<ICustomDestinationList>();
258+
RefPtr<ICustomDestinationList> jumpListMgr = mJumpListMgr.Resolve();
264259
if (!jumpListMgr) {
265260
return;
266261
}
@@ -335,7 +330,7 @@ NS_IMETHODIMP LegacyJumpListBuilder::AddListToBuild(int16_t aCatType,
335330
ReentrantMonitorAutoEnter lock(mMonitor);
336331
if (!mJumpListMgr) return NS_ERROR_NOT_AVAILABLE;
337332

338-
RefPtr jumpListMgr = mJumpListMgr.Resolve<ICustomDestinationList>();
333+
RefPtr<ICustomDestinationList> jumpListMgr = mJumpListMgr.Resolve();
339334
if (!jumpListMgr) {
340335
return NS_ERROR_UNEXPECTED;
341336
}
@@ -459,7 +454,7 @@ NS_IMETHODIMP LegacyJumpListBuilder::AbortListBuild() {
459454
ReentrantMonitorAutoEnter lock(mMonitor);
460455
if (!mJumpListMgr) return NS_ERROR_NOT_AVAILABLE;
461456

462-
RefPtr jumpListMgr = mJumpListMgr.Resolve<ICustomDestinationList>();
457+
RefPtr<ICustomDestinationList> jumpListMgr = mJumpListMgr.Resolve();
463458
if (!jumpListMgr) {
464459
return NS_ERROR_UNEXPECTED;
465460
}
@@ -508,7 +503,7 @@ void LegacyJumpListBuilder::DoCommitListBuild(
508503
Unused << NS_DispatchToMainThread(aCallback);
509504
});
510505

511-
RefPtr jumpListMgr = mJumpListMgr.Resolve<ICustomDestinationList>();
506+
RefPtr<ICustomDestinationList> jumpListMgr = mJumpListMgr.Resolve();
512507
if (!jumpListMgr) {
513508
return;
514509
}
@@ -531,7 +526,7 @@ NS_IMETHODIMP LegacyJumpListBuilder::DeleteActiveList(bool* _retval) {
531526
AbortListBuild();
532527
}
533528

534-
RefPtr jumpListMgr = mJumpListMgr.Resolve<ICustomDestinationList>();
529+
RefPtr<ICustomDestinationList> jumpListMgr = mJumpListMgr.Resolve();
535530
if (!jumpListMgr) {
536531
return NS_ERROR_UNEXPECTED;
537532
}

widget/windows/LegacyJumpListBuilder.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ class LegacyJumpListBuilder : public nsILegacyJumpListBuilder,
4646
static Atomic<bool> sBuildingList;
4747

4848
private:
49-
mscom::AgileReference mJumpListMgr MOZ_GUARDED_BY(mMonitor);
49+
mscom::AgileReference<ICustomDestinationList> mJumpListMgr
50+
MOZ_GUARDED_BY(mMonitor);
5051
uint32_t mMaxItems MOZ_GUARDED_BY(mMonitor);
5152
bool mHasCommit;
5253
RefPtr<LazyIdleThread> mIOThread;

0 commit comments

Comments
 (0)