Bug 1418448 part 1: Accessible HandlerProvider: Clean up DynamicIA2Data correctly. r?aklotz draft
authorJames Teh <jteh@mozilla.com>
Fri, 24 Nov 2017 15:12:37 +1000
changeset 702984 f53857ce9acae929939979deb8f2a503a6f25afd
parent 702247 960f50c2e0a991ab2ab313132e69fb2c96cb7866
child 702985 a0147b807e921064cd78a32ff868c90098e55fcd
push id90653
push userbmo:jteh@mozilla.com
push dateFri, 24 Nov 2017 05:33:22 +0000
reviewersaklotz
bugs1418448
milestone59.0a1
Bug 1418448 part 1: Accessible HandlerProvider: Clean up DynamicIA2Data correctly. r?aklotz DynamicIA2Data contains several BSTRs that need to be freed with SysFreeString. Previously, we just did a ZeroMemory without actually freeing them. This cleanup code is placed in a header file so it can be used by AccessibleHandler as well. MozReview-Commit-ID: 4SWuK9oMRYZ
accessible/ipc/win/HandlerProvider.cpp
accessible/ipc/win/HandlerProvider.h
accessible/ipc/win/handler/HandlerDataCleanup.h
accessible/ipc/win/handler/moz.build
--- a/accessible/ipc/win/HandlerProvider.cpp
+++ b/accessible/ipc/win/HandlerProvider.cpp
@@ -12,16 +12,17 @@
 #include "AccessibleDocument.h"
 #include "AccessibleTable.h"
 #include "AccessibleTable2.h"
 #include "AccessibleTableCell.h"
 #include "HandlerData.h"
 #include "HandlerData_i.c"
 #include "mozilla/Assertions.h"
 #include "mozilla/a11y/AccessibleWrap.h"
+#include "mozilla/a11y/HandlerDataCleanup.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"
@@ -125,17 +126,18 @@ HandlerProvider::GetAndSerializePayload(
   // 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 clean up any
   // BSTRs, interfaces, etc. fetched in BuildInitialIA2Data.
   CleanupStaticIA2Data(payload.mStaticData);
-  CleanupDynamicIA2Data(payload.mDynamicData);
+  // No need to zero memory, since payload is going out of scope.
+  CleanupDynamicIA2Data(payload.mDynamicData, false);
 }
 
 HRESULT
 HandlerProvider::GetHandlerPayloadSize(NotNull<mscom::IInterceptor*> aInterceptor,
                                        NotNull<DWORD*> aOutPayloadSize)
 {
   MOZ_ASSERT(mscom::IsCurrentThreadMTA());
 
@@ -415,23 +417,16 @@ HandlerProvider::CleanupStaticIA2Data(St
   }
   if (aData.mIATableCell) {
     aData.mIATableCell->Release();
   }
   ZeroMemory(&aData, sizeof(StaticIA2Data));
 }
 
 void
-HandlerProvider::CleanupDynamicIA2Data(DynamicIA2Data& aData)
-{
-  ::VariantClear(&aData.mRole);
-  ZeroMemory(&aData, sizeof(DynamicIA2Data));
-}
-
-void
 HandlerProvider::BuildInitialIA2Data(
   NotNull<mscom::IInterceptor*> aInterceptor,
   StaticIA2Data* aOutStaticData,
   DynamicIA2Data* aOutDynamicData)
 {
   BuildStaticIA2Data(aInterceptor, aOutStaticData);
   if (!aOutStaticData->mIA2) {
     return;
--- a/accessible/ipc/win/HandlerProvider.h
+++ b/accessible/ipc/win/HandlerProvider.h
@@ -65,17 +65,16 @@ private:
                               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;
new file mode 100644
--- /dev/null
+++ b/accessible/ipc/win/handler/HandlerDataCleanup.h
@@ -0,0 +1,55 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* 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/. */
+
+#ifndef mozilla_a11y_HandlerDataCleanup_h
+#define mozilla_a11y_HandlerDataCleanup_h
+
+#include <OleAuto.h>
+#include "HandlerData.h"
+
+namespace mozilla {
+namespace a11y {
+
+inline void
+CleanupDynamicIA2Data(DynamicIA2Data& aData, bool aZero=true)
+{
+  ::VariantClear(&aData.mRole);
+  if (aData.mKeyboardShortcut) {
+    ::SysFreeString(aData.mKeyboardShortcut);
+  }
+  if (aData.mName) {
+    ::SysFreeString(aData.mName);
+  }
+  if (aData.mDescription) {
+    ::SysFreeString(aData.mDescription);
+  }
+  if (aData.mDefaultAction) {
+    ::SysFreeString(aData.mDefaultAction);
+  }
+  if (aData.mValue) {
+    ::SysFreeString(aData.mValue);
+  }
+  if (aData.mAttributes) {
+    ::SysFreeString(aData.mAttributes);
+  }
+  if (aData.mIA2Locale.language)  {
+    ::SysFreeString(aData.mIA2Locale.language);
+  }
+  if (aData.mIA2Locale.country)  {
+    ::SysFreeString(aData.mIA2Locale.country);
+  }
+  if (aData.mIA2Locale.variant)  {
+    ::SysFreeString(aData.mIA2Locale.variant);
+  }
+  if (aZero) {
+    ZeroMemory(&aData, sizeof(DynamicIA2Data));
+  }
+}
+
+} // namespace a11y
+} // namespace mozilla
+
+#endif // mozilla_a11y_HandlerDataCleanup_h
--- a/accessible/ipc/win/handler/moz.build
+++ b/accessible/ipc/win/handler/moz.build
@@ -1,17 +1,20 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # 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/.
 
 SharedLibrary('AccessibleHandler')
 
-EXPORTS.mozilla.a11y += ['AccessibleHandler.h']
+EXPORTS.mozilla.a11y += [
+    'AccessibleHandler.h',
+    'HandlerDataCleanup.h',
+]
 
 LOCAL_INCLUDES += [
     '/accessible/interfaces/ia2',
     '/ipc/mscom/oop',
 ]
 
 SOURCES += [
     '!dlldata.c',