Bug 1418448 part 3: Accessible Handler/Provider: Unify release of interfaces in StaticIA2Data. r?aklotz draft
authorJames Teh <jteh@mozilla.com>
Fri, 24 Nov 2017 15:10:41 +1000
changeset 704794 8ab8603daa264735093616c93ba861840dee32c5
parent 702985 a0147b807e921064cd78a32ff868c90098e55fcd
child 704813 11c5f2f6a1411a6a193168bb5cbb5f319fa27496
push id91246
push userbmo:jteh@mozilla.com
push dateTue, 28 Nov 2017 23:57:37 +0000
reviewersaklotz
bugs1418448
milestone59.0a1
Bug 1418448 part 3: Accessible Handler/Provider: Unify release of interfaces in StaticIA2Data. r?aklotz Both Provider and Handler need to release the interfaces in StaticIA2Data. Therefore, move this into a common function to avoid duplication pain in future. MozReview-Commit-ID: 7J4iuvDa8m2
accessible/ipc/win/HandlerProvider.cpp
accessible/ipc/win/handler/AccessibleHandler.cpp
accessible/ipc/win/handler/HandlerDataCleanup.h
--- a/accessible/ipc/win/HandlerProvider.cpp
+++ b/accessible/ipc/win/HandlerProvider.cpp
@@ -392,37 +392,17 @@ HandlerProvider::BuildDynamicIA2Data(Dyn
   hr = target->get_uniqueID(&aOutIA2Data->mUniqueId);
 }
 
 void
 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();
-  }
+  ReleaseStaticIA2DataInterfaces(aData);
   ZeroMemory(&aData, sizeof(StaticIA2Data));
 }
 
 void
 HandlerProvider::BuildInitialIA2Data(
   NotNull<mscom::IInterceptor*> aInterceptor,
   StaticIA2Data* aOutStaticData,
   DynamicIA2Data* aOutDynamicData)
--- a/accessible/ipc/win/handler/AccessibleHandler.cpp
+++ b/accessible/ipc/win/handler/AccessibleHandler.cpp
@@ -331,37 +331,17 @@ AccessibleHandler::ReadHandlerPayload(IS
   // 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();
-  }
-  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();
-  }
+  ReleaseStaticIA2DataInterfaces(mCachedData.mStaticData);
 
   if (!mCachedData.mGeckoBackChannel) {
     return S_OK;
   }
 
   RefPtr<AccessibleHandlerControl> ctl(gControlFactory.GetOrCreateSingleton());
   if (!ctl) {
     return E_OUTOFMEMORY;
--- a/accessible/ipc/win/handler/HandlerDataCleanup.h
+++ b/accessible/ipc/win/handler/HandlerDataCleanup.h
@@ -9,16 +9,44 @@
 
 #include <OleAuto.h>
 #include "HandlerData.h"
 
 namespace mozilla {
 namespace a11y {
 
 inline void
+ReleaseStaticIA2DataInterfaces(StaticIA2Data& aData)
+{
+  // Only interfaces of the IA2 object should be released here, never other
+  // objects!
+  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();
+  }
+}
+
+inline void
 CleanupDynamicIA2Data(DynamicIA2Data& aData, bool aZero=true)
 {
   ::VariantClear(&aData.mRole);
   if (aData.mKeyboardShortcut) {
     ::SysFreeString(aData.mKeyboardShortcut);
   }
   if (aData.mName) {
     ::SysFreeString(aData.mName);