Bug 1416986 part 3: AccessibleHandler: Avoid cross-process QI calls for interfaces which we know don't exist. r?aklotz
The proxy manager caches interfaces marshaled in the payload and returns them on QI without a cross-process call.
However, it doesn't know about interfaces which don't exist.
We can determine this from the payload, since interfaces which don't exist will have a null pointer.
We use this information to avoid querying the proxy in this case.
MozReview-Commit-ID: FnzDetmTiPP
--- a/accessible/ipc/win/handler/AccessibleHandler.cpp
+++ b/accessible/ipc/win/handler/AccessibleHandler.cpp
@@ -24,17 +24,21 @@
#include <uiautomation.h>
#include <winreg.h>
#include "AccessibleHypertext.h"
#include "AccessibleHypertext2.h"
#include "Accessible2_i.c"
#include "Accessible2_2_i.c"
#include "Accessible2_3_i.c"
+#include "AccessibleAction_i.c"
#include "AccessibleHyperlink_i.c"
+#include "AccessibleTable_i.c"
+#include "AccessibleTable2_i.c"
+#include "AccessibleTableCell_i.c"
namespace mozilla {
namespace a11y {
static mscom::Factory<AccessibleHandler> sHandlerFactory;
HRESULT
AccessibleHandler::Create(IUnknown* aOuter, REFIID aIid, void** aOutInterface)
@@ -205,16 +209,42 @@ AccessibleHandler::QueryHandlerInterface
}
if (aIid == IID_IServiceProvider) {
RefPtr<IServiceProvider> svcProv(static_cast<IServiceProvider*>(this));
svcProv.forget(aOutInterface);
return S_OK;
}
+ if (HasPayload()) {
+ // The proxy manager caches interfaces marshaled in the payload
+ // and returns them on QI without a cross-process call.
+ // However, it doesn't know about interfaces which don't exist.
+ // We can determine this from the payload.
+ if ((aIid == IID_IEnumVARIANT &&
+ !mCachedData.mStaticData.mIEnumVARIANT) ||
+ ((aIid == IID_IAccessibleText || aIid == IID_IAccessibleHypertext ||
+ aIid == IID_IAccessibleHypertext2) &&
+ !mCachedData.mStaticData.mIAHypertext) ||
+ ((aIid == IID_IAccessibleAction || aIid == IID_IAccessibleHyperlink) &&
+ !mCachedData.mStaticData.mIAHyperlink) ||
+ (aIid == IID_IAccessibleTable &&
+ !mCachedData.mStaticData.mIATable) ||
+ (aIid == IID_IAccessibleTable2 &&
+ !mCachedData.mStaticData.mIATable2) ||
+ (aIid == IID_IAccessibleTableCell &&
+ !mCachedData.mStaticData.mIATableCell)) {
+ // We already know this interface is not available, so don't query
+ // the proxy, thus avoiding a pointless cross-process call.
+ // If we return E_NOINTERFACE here, mscom::Handler will try the COM
+ // proxy. S_FALSE signals that the proxy should not be tried.
+ return S_FALSE;
+ }
+ }
+
if (aIid == IID_IAccessibleText || aIid == IID_IAccessibleHypertext ||
aIid == IID_IAccessibleHypertext2) {
RefPtr<IAccessibleHypertext2> textTearoff(new AccessibleTextTearoff(this));
textTearoff.forget(aOutInterface);
return S_OK;
}
if (aIid == IID_IProvideClassInfo) {
@@ -236,23 +266,34 @@ AccessibleHandler::ReadHandlerPayload(IS
mscom::StructFromStream deserializer(aStream);
if (!deserializer) {
return E_FAIL;
}
if (deserializer.IsEmpty()) {
return S_FALSE;
}
- if (!deserializer.Read(&mCachedData, &IA2Payload_Decode)) {
+ // QueryHandlerInterface might get called while we deserialize the payload,
+ // but that checks the interface pointers in the payload to determine what
+ // interfaces are available. Therefore, deserialize into a temporary struct
+ // and update mCachedData only after deserialization completes.
+ // The decoding functions can misbehave if their target memory is not zeroed
+ // beforehand, so ensure we do that.
+ IA2Payload newData{};
+ if (!deserializer.Read(&newData, &IA2Payload_Decode)) {
return E_FAIL;
}
+ mCachedData = newData;
// 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.
+ // However, we don't null them out because we use their presence
+ // to determine whether the interface is available
+ // so as to avoid pointless cross-proc QI calls returning E_NOINTERFACE.
// 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();