Bug 1416986 part 2: Include interfaces the client is likely to request in the accessible handler payload. r?aklotz
Now that virtual buffers have to render across processes, we want to eliminate as many cross-process calls as possible.
This includes QueryInterface calls, since buffers query for several interfaces on every node they visit.
To avoid these cross-process QI calls, we include interfaces clients are likely to request in the handler payload.
This way, they get marshaled in the single call used to retrieve the object.
This patch does the following:
1. Passes the interceptor when building the payload.
We need this so we can get interceptors for other interfaces.
2. Splits the payload into two main parts: a static part and a dynamic part.
The (new) static part contains the interface pointers. The dynamic part contains the rest.
This is necessary because the refresh call cannot pass the interceptor, but the interceptor is needed to build the static part.
Also, re-building the static part is pointless when refreshing.
3. Includes the interface pointers in the payload (BuildStaticIA2Data).
The pointers also have to be cleaned up after marshaling.
4. Releases the interface pointers in the handler after the payload is received.
We do this because they're aggregated by the proxy manager as they're unmarshaled.
MozReview-Commit-ID: 6VRLMNScgwW
--- a/accessible/ipc/win/HandlerProvider.cpp
+++ b/accessible/ipc/win/HandlerProvider.cpp
@@ -16,16 +16,17 @@
#include "HandlerData.h"
#include "HandlerData_i.c"
#include "mozilla/Assertions.h"
#include "mozilla/a11y/AccessibleWrap.h"
#include "mozilla/dom/ContentChild.h"
#include "mozilla/Move.h"
#include "mozilla/mscom/AgileReference.h"
#include "mozilla/mscom/FastMarshaler.h"
+#include "mozilla/mscom/Interceptor.h"
#include "mozilla/mscom/MainThreadInvoker.h"
#include "mozilla/mscom/Ptr.h"
#include "mozilla/mscom/StructStream.h"
#include "mozilla/mscom/Utils.h"
#include "nsThreadUtils.h"
#include <memory.h>
@@ -92,62 +93,65 @@ HandlerProvider::GetHandler(NotNull<CLSI
return E_NOINTERFACE;
}
*aHandlerClsid = CLSID_AccessibleHandler;
return S_OK;
}
void
-HandlerProvider::GetAndSerializePayload(const MutexAutoLock&)
+HandlerProvider::GetAndSerializePayload(const MutexAutoLock&,
+ NotNull<mscom::IInterceptor*> aInterceptor)
{
MOZ_ASSERT(mscom::IsCurrentThreadMTA());
if (mSerializer) {
return;
}
IA2Payload payload{};
- if (!mscom::InvokeOnMainThread("HandlerProvider::BuildIA2Data",
- this, &HandlerProvider::BuildIA2Data,
- &payload.mData) ||
- !payload.mData.mUniqueId) {
+ if (!mscom::InvokeOnMainThread("HandlerProvider::BuildInitialIA2Data",
+ this, &HandlerProvider::BuildInitialIA2Data,
+ aInterceptor,
+ &payload.mStaticData, &payload.mDynamicData) ||
+ !payload.mDynamicData.mUniqueId) {
return;
}
// But we set mGeckoBackChannel on the current thread which resides in the
// MTA. This is important to ensure that COM always invokes
// IGeckoBackChannel methods in an MTA background thread.
RefPtr<IGeckoBackChannel> payloadRef(this);
// AddRef/Release pair for this reference is handled by payloadRef
payload.mGeckoBackChannel = this;
mSerializer = MakeUnique<mscom::StructToStream>(payload, &IA2Payload_Encode);
- // Now that we have serialized payload, we should free any BSTRs that were
- // allocated in BuildIA2Data.
- ClearIA2Data(payload.mData);
+ // Now that we have serialized payload, we should clean up any
+ // BSTRs, interfaces, etc. fetched in BuildInitialIA2Data.
+ CleanupStaticIA2Data(payload.mStaticData);
+ CleanupDynamicIA2Data(payload.mDynamicData);
}
HRESULT
HandlerProvider::GetHandlerPayloadSize(NotNull<mscom::IInterceptor*> aInterceptor,
NotNull<DWORD*> aOutPayloadSize)
{
MOZ_ASSERT(mscom::IsCurrentThreadMTA());
if (!IsTargetInterfaceCacheable()) {
*aOutPayloadSize = mscom::StructToStream::GetEmptySize();
return S_OK;
}
MutexAutoLock lock(mMutex);
- GetAndSerializePayload(lock);
+ GetAndSerializePayload(lock, aInterceptor);
if (!mSerializer || !(*mSerializer)) {
// Failed payload serialization is non-fatal
*aOutPayloadSize = mscom::StructToStream::GetEmptySize();
return S_OK;
}
*aOutPayloadSize = mSerializer->GetSize();
@@ -177,17 +181,82 @@ public:
ExecuteWhen& operator=(ExecuteWhen&&) = delete;
private:
CondFnT& mCondFn;
ExeFnT& mExeFn;
};
void
-HandlerProvider::BuildIA2Data(IA2Data* aOutIA2Data)
+HandlerProvider::BuildStaticIA2Data(
+ NotNull<mscom::IInterceptor*> aInterceptor,
+ StaticIA2Data* aOutData)
+{
+ MOZ_ASSERT(aOutData);
+ MOZ_ASSERT(NS_IsMainThread());
+ MOZ_ASSERT(mTargetUnk);
+ MOZ_ASSERT(IsTargetInterfaceCacheable());
+
+ // Include interfaces the client is likely to request.
+ // This is cheap here and saves multiple cross-process calls later.
+ // These interfaces must be released in CleanupStaticIA2Data!
+
+ // If the target is already an IAccessible2, this pointer is redundant.
+ // However, the target might be an IAccessibleHyperlink, etc., in which
+ // case the client will almost certainly QI for IAccessible2.
+ HRESULT hr = aInterceptor->GetInterceptorForIID(NEWEST_IA2_IID,
+ (void**)&aOutData->mIA2);
+ if (FAILED(hr)) {
+ // IA2 should always be present, so something has
+ // gone very wrong if this fails.
+ aOutData->mIA2 = nullptr;
+ return;
+ }
+
+ // Some of these interfaces aren't present on all accessibles,
+ // so it's not a failure if these interfaces can't be fetched.
+ hr = aInterceptor->GetInterceptorForIID(IID_IEnumVARIANT,
+ (void**)&aOutData->mIEnumVARIANT);
+ if (FAILED(hr)) {
+ aOutData->mIEnumVARIANT = nullptr;
+ }
+
+ hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleHypertext2,
+ (void**)&aOutData->mIAHypertext);
+ if (FAILED(hr)) {
+ aOutData->mIAHypertext = nullptr;
+ }
+
+ hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleHyperlink,
+ (void**)&aOutData->mIAHyperlink);
+ if (FAILED(hr)) {
+ aOutData->mIAHyperlink = nullptr;
+ }
+
+ hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleTable,
+ (void**)&aOutData->mIATable);
+ if (FAILED(hr)) {
+ aOutData->mIATable = nullptr;
+ }
+
+ hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleTable2,
+ (void**)&aOutData->mIATable2);
+ if (FAILED(hr)) {
+ aOutData->mIATable2 = nullptr;
+ }
+
+ hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleTableCell,
+ (void**)&aOutData->mIATableCell);
+ if (FAILED(hr)) {
+ aOutData->mIATableCell = nullptr;
+ }
+}
+
+void
+HandlerProvider::BuildDynamicIA2Data(DynamicIA2Data* aOutIA2Data)
{
MOZ_ASSERT(aOutIA2Data);
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(mTargetUnk);
MOZ_ASSERT(IsTargetInterfaceCacheable());
RefPtr<NEWEST_IA2_INTERFACE> target;
HRESULT hr = mTargetUnk.get()->QueryInterface(NEWEST_IA2_IID,
@@ -198,17 +267,17 @@ HandlerProvider::BuildIA2Data(IA2Data* a
hr = E_UNEXPECTED;
auto hasFailed = [&hr]() -> bool {
return FAILED(hr);
};
auto cleanup = [this, aOutIA2Data]() -> void {
- ClearIA2Data(*aOutIA2Data);
+ CleanupDynamicIA2Data(*aOutIA2Data);
};
ExecuteWhen<decltype(hasFailed), decltype(cleanup)> onFail(hasFailed, cleanup);
const VARIANT kChildIdSelf = {VT_I4};
VARIANT varVal;
hr = target->accLocation(&aOutIA2Data->mLeft, &aOutIA2Data->mTop,
@@ -286,20 +355,67 @@ HandlerProvider::BuildIA2Data(IA2Data* a
// NB: get_uniqueID should be the final property retrieved in this method,
// as its presence is used to determine whether the rest of this data
// retrieval was successful.
hr = target->get_uniqueID(&aOutIA2Data->mUniqueId);
}
void
-HandlerProvider::ClearIA2Data(IA2Data& aData)
+HandlerProvider::CleanupStaticIA2Data(StaticIA2Data& aData)
+{
+ // When CoMarshalInterface writes interfaces out to a stream, it AddRefs.
+ // Therefore, we must release our references after this.
+ if (aData.mIA2) {
+ aData.mIA2->Release();
+ }
+ if (aData.mIEnumVARIANT) {
+ aData.mIEnumVARIANT->Release();
+ }
+ if (aData.mIAHypertext) {
+ aData.mIAHypertext->Release();
+ }
+ if (aData.mIAHyperlink) {
+ aData.mIAHyperlink->Release();
+ }
+ if (aData.mIATable) {
+ aData.mIATable->Release();
+ }
+ if (aData.mIATable2) {
+ aData.mIATable2->Release();
+ }
+ if (aData.mIATableCell) {
+ aData.mIATableCell->Release();
+ }
+ ZeroMemory(&aData, sizeof(StaticIA2Data));
+}
+
+void
+HandlerProvider::CleanupDynamicIA2Data(DynamicIA2Data& aData)
{
::VariantClear(&aData.mRole);
- ZeroMemory(&aData, sizeof(IA2Data));
+ ZeroMemory(&aData, sizeof(DynamicIA2Data));
+}
+
+void
+HandlerProvider::BuildInitialIA2Data(
+ NotNull<mscom::IInterceptor*> aInterceptor,
+ StaticIA2Data* aOutStaticData,
+ DynamicIA2Data* aOutDynamicData)
+{
+ BuildStaticIA2Data(aInterceptor, aOutStaticData);
+ if (!aOutStaticData->mIA2) {
+ return;
+ }
+ BuildDynamicIA2Data(aOutDynamicData);
+ if (!aOutDynamicData->mUniqueId) {
+ // Building dynamic data failed, which means building the payload failed.
+ // However, we've already built static data, so we must clean this up.
+ CleanupStaticIA2Data(*aOutStaticData);
+ }
}
bool
HandlerProvider::IsTargetInterfaceCacheable()
{
return MarshalAs(mTargetUnkIid) == NEWEST_IA2_IID ||
mTargetUnkIid == IID_IAccessibleHyperlink;
}
@@ -402,22 +518,22 @@ HandlerProvider::put_HandlerControl(long
static_cast<DWORD>(aPid), Move(ptrProxy))) {
return E_FAIL;
}
return S_OK;
}
HRESULT
-HandlerProvider::Refresh(IA2Data* aOutData)
+HandlerProvider::Refresh(DynamicIA2Data* aOutData)
{
MOZ_ASSERT(mscom::IsCurrentThreadMTA());
- if (!mscom::InvokeOnMainThread("HandlerProvider::BuildIA2Data",
- this, &HandlerProvider::BuildIA2Data,
+ if (!mscom::InvokeOnMainThread("HandlerProvider::BuildDynamicIA2Data",
+ this, &HandlerProvider::BuildDynamicIA2Data,
aOutData)) {
return E_FAIL;
}
return S_OK;
}
} // namespace a11y
--- a/accessible/ipc/win/HandlerProvider.h
+++ b/accessible/ipc/win/HandlerProvider.h
@@ -49,26 +49,33 @@ public:
STDMETHODIMP_(REFIID) GetEffectiveOutParamIid(REFIID aCallIid,
ULONG aCallMethod) override;
STDMETHODIMP NewInstance(REFIID aIid,
mscom::InterceptorTargetPtr<IUnknown> aTarget,
NotNull<mscom::IHandlerProvider**> aOutNewPayload) override;
// IGeckoBackChannel
STDMETHODIMP put_HandlerControl(long aPid, IHandlerControl* aCtrl) override;
- STDMETHODIMP Refresh(IA2Data* aOutData) override;
+ STDMETHODIMP Refresh(DynamicIA2Data* aOutData) override;
private:
~HandlerProvider() = default;
void SetHandlerControlOnMainThread(DWORD aPid,
mscom::ProxyUniquePtr<IHandlerControl> aCtrl);
- void GetAndSerializePayload(const MutexAutoLock&);
- void BuildIA2Data(IA2Data* aOutIA2Data);
- static void ClearIA2Data(IA2Data& aData);
+ void GetAndSerializePayload(const MutexAutoLock&,
+ NotNull<mscom::IInterceptor*> aInterceptor);
+ void BuildStaticIA2Data(NotNull<mscom::IInterceptor*> aInterceptor,
+ StaticIA2Data* aOutData);
+ void BuildDynamicIA2Data(DynamicIA2Data* aOutIA2Data);
+ void BuildInitialIA2Data(NotNull<mscom::IInterceptor*> aInterceptor,
+ StaticIA2Data* aOutStaticData,
+ DynamicIA2Data* aOutDynamicData);
+ static void CleanupStaticIA2Data(StaticIA2Data& aData);
+ static void CleanupDynamicIA2Data(DynamicIA2Data& aData);
bool IsTargetInterfaceCacheable();
Atomic<uint32_t> mRefCnt;
Mutex mMutex; // Protects mSerializer
const IID mTargetUnkIid;
mscom::InterceptorTargetPtr<IUnknown> mTargetUnk; // Constant, main thread only
UniquePtr<mscom::StructToStream> mSerializer;
RefPtr<IUnknown> mFastMarshalUnk;
--- a/accessible/ipc/win/handler/AccessibleHandler.cpp
+++ b/accessible/ipc/win/handler/AccessibleHandler.cpp
@@ -142,17 +142,17 @@ AccessibleHandler::MaybeUpdateCachedData
if (gen == mCacheGen) {
return S_OK;
}
if (!mCachedData.mGeckoBackChannel) {
return E_POINTER;
}
- return mCachedData.mGeckoBackChannel->Refresh(&mCachedData.mData);
+ return mCachedData.mGeckoBackChannel->Refresh(&mCachedData.mDynamicData);
}
HRESULT
AccessibleHandler::ResolveIDispatch()
{
if (mDispatch) {
return S_OK;
}
@@ -240,16 +240,44 @@ AccessibleHandler::ReadHandlerPayload(IS
if (deserializer.IsEmpty()) {
return S_FALSE;
}
if (!deserializer.Read(&mCachedData, &IA2Payload_Decode)) {
return E_FAIL;
}
+ // These interfaces have been aggregated into the proxy manager.
+ // The proxy manager will resolve these interfaces now on QI,
+ // so we can release these pointers.
+ // Note that if pointers to other objects (in contrast to
+ // interfaces of *this* object) are added in future, we should not release
+ // those pointers.
+ if (mCachedData.mStaticData.mIA2) {
+ mCachedData.mStaticData.mIA2->Release();
+ }
+ if (mCachedData.mStaticData.mIEnumVARIANT) {
+ mCachedData.mStaticData.mIEnumVARIANT->Release();
+ }
+ if (mCachedData.mStaticData.mIAHypertext) {
+ mCachedData.mStaticData.mIAHypertext->Release();
+ }
+ if (mCachedData.mStaticData.mIAHyperlink) {
+ mCachedData.mStaticData.mIAHyperlink->Release();
+ }
+ if (mCachedData.mStaticData.mIATable) {
+ mCachedData.mStaticData.mIATable->Release();
+ }
+ if (mCachedData.mStaticData.mIATable2) {
+ mCachedData.mStaticData.mIATable2->Release();
+ }
+ if (mCachedData.mStaticData.mIATableCell) {
+ mCachedData.mStaticData.mIATableCell->Release();
+ }
+
if (!mCachedData.mGeckoBackChannel) {
return S_OK;
}
RefPtr<AccessibleHandlerControl> ctl(gControlFactory.GetOrCreateSingleton());
if (!ctl) {
return E_OUTOFMEMORY;
}
@@ -406,22 +434,22 @@ CopyBSTR(BSTR aSrc)
HRESULT hr; \
if (FAILED(hr = MaybeUpdateCachedData())) { \
return hr; \
} \
}
#define GET_FIELD(member, assignTo) \
{ \
- assignTo = mCachedData.mData.member; \
+ assignTo = mCachedData.mDynamicData.member; \
}
#define GET_BSTR(member, assignTo) \
{ \
- assignTo = CopyBSTR(mCachedData.mData.member); \
+ assignTo = CopyBSTR(mCachedData.mDynamicData.member); \
}
/*** IAccessible ***/
HRESULT
AccessibleHandler::get_accParent(IDispatch **ppdispParent)
{
HRESULT hr = ResolveIA2();
@@ -542,17 +570,17 @@ AccessibleHandler::get_accRole(VARIANT v
HRESULT hr = ResolveIA2();
if (FAILED(hr)) {
return hr;
}
return mIA2PassThru->get_accRole(varChild, pvarRole);
}
BEGIN_CACHE_ACCESS;
- return ::VariantCopy(pvarRole, &mCachedData.mData.mRole);
+ return ::VariantCopy(pvarRole, &mCachedData.mDynamicData.mRole);
}
HRESULT
AccessibleHandler::get_accState(VARIANT varChild, VARIANT *pvarState)
{
if (!pvarState) {
return E_INVALIDARG;
@@ -914,17 +942,17 @@ AccessibleHandler::get_uniqueID(long* un
}
if (!HasPayload()) {
HRESULT hr = ResolveIA2();
if (FAILED(hr)) {
return hr;
}
return mIA2PassThru->get_uniqueID(uniqueID);
}
- *uniqueID = mCachedData.mData.mUniqueId;
+ *uniqueID = mCachedData.mDynamicData.mUniqueId;
return S_OK;
}
HRESULT
AccessibleHandler::get_windowHandle(HWND* windowHandle)
{
if (!windowHandle) {
return E_INVALIDARG;
--- a/accessible/ipc/win/handler/HandlerData.idl
+++ b/accessible/ipc/win/handler/HandlerData.idl
@@ -2,22 +2,39 @@
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "mozilla-config.h"
#include "AccessibleHandler.h"
+import "oaidl.idl";
import "ocidl.idl";
import "ServProv.idl";
-import "AccessibleText.idl";
+import "Accessible2_3.idl";
+import "AccessibleHypertext2.idl";
+import "AccessibleHyperlink.idl";
+import "AccessibleTable.idl";
+import "AccessibleTable2.idl";
+import "AccessibleTableCell.idl";
-typedef struct _IA2Data
+typedef struct _StaticIA2Data
+{
+ NEWEST_IA2_INTERFACE* mIA2;
+ IEnumVARIANT* mIEnumVARIANT;
+ IAccessibleHypertext2* mIAHypertext;
+ IAccessibleHyperlink* mIAHyperlink;
+ IAccessibleTable* mIATable;
+ IAccessibleTable2* mIATable2;
+ IAccessibleTableCell* mIATableCell;
+} StaticIA2Data;
+
+typedef struct _DynamicIA2Data
{
VARIANT mRole;
long mState;
long mChildCount;
long mIA2Role;
AccessibleStates mIA2States;
long mLeft;
long mTop;
@@ -27,17 +44,17 @@ typedef struct _IA2Data
BSTR mKeyboardShortcut;
BSTR mName;
BSTR mDescription;
BSTR mDefaultAction;
BSTR mValue;
BSTR mAttributes;
IA2Locale mIA2Locale;
long mUniqueId;
-} IA2Data;
+} DynamicIA2Data;
interface IGeckoBackChannel;
// We define different CLSIDs and IIDs depending on channel and officiality.
// This prevents handlers from installing overtop one another when multiple
// channels are present. Note that we do not do this for all UUIDs in this IDL,
// just the ones that are written to the registry (coclass and interfaces that
// have the [object] annotation)
@@ -95,17 +112,18 @@ interface IGeckoBackChannel;
#endif
[uuid(2b0e83b3-fd1a-443f-9ed6-c00d39055b58)]
interface HandlerData
{
typedef struct _IA2Payload
{
- IA2Data mData;
+ StaticIA2Data mStaticData;
+ DynamicIA2Data mDynamicData;
IGeckoBackChannel* mGeckoBackChannel;
} IA2Payload;
}
[object,
uuid(IHANDLERCONTROL_IID),
async_uuid(ASYNCIHANDLERCONTROL_IID),
pointer_default(unique)]
@@ -118,17 +136,17 @@ interface IHandlerControl : IUnknown
}
[object,
uuid(IGECKOBACKCHANNEL_IID),
pointer_default(unique)]
interface IGeckoBackChannel : IUnknown
{
[propput] HRESULT HandlerControl([in] long aPid, [in] IHandlerControl* aCtrl);
- HRESULT Refresh([out] IA2Data* aOutData);
+ HRESULT Refresh([out] DynamicIA2Data* aOutData);
}
[uuid(1e545f07-f108-4912-9471-546827a80983)]
library AccessibleHandlerTypeLib
{
/**
* This definition is required in order for the handler implementation to
* support IDispatch (aka Automation). This is used by interpreted language