Backed out 3 changesets (bug 1640553) for causing Windows build bustages CLOSED TREE
authorNoemi Erli <nerli@mozilla.com>
Tue, 26 May 2020 03:50:11 +0300
changeset 532106 f4014f06729515f3f30125d994d1dafb740fda6c
parent 532105 d6b6d63e601dd2b8d5f6967eb1fc5dc4508f1c5b
child 532107 c7488bc9dace31c66d321021ccba91133b964a22
push id37450
push usercbrindusan@mozilla.com
push dateTue, 26 May 2020 15:44:42 +0000
treeherdermozilla-central@4da52a3b8dfd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1640553
milestone78.0a1
backs out25dd5eeb676f29f53df9418804f6d6b686d61028
4e0cd8a3d83c96210a40788511d79f40f6e87258
6658e0ae7e356f7b17896e1ee0316416fec2f2e0
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out 3 changesets (bug 1640553) for causing Windows build bustages CLOSED TREE Backed out changeset 25dd5eeb676f (bug 1640553) Backed out changeset 4e0cd8a3d83c (bug 1640553) Backed out changeset 6658e0ae7e35 (bug 1640553)
accessible/ipc/win/HandlerProvider.cpp
accessible/ipc/win/handler/AccessibleHandler.cpp
accessible/ipc/win/handler/AccessibleHandlerControl.cpp
accessible/ipc/win/handler/AccessibleHandlerControl.h
accessible/ipc/win/handler/HandlerData.idl
accessible/ipc/win/handler/HandlerDataCleanup.h
--- a/accessible/ipc/win/HandlerProvider.cpp
+++ b/accessible/ipc/win/HandlerProvider.cpp
@@ -348,64 +348,16 @@ void HandlerProvider::BuildDynamicIA2Dat
   if (SUCCEEDED(hr)) {
     hr = cell->get_rowColumnExtents(
         &aOutIA2Data->mRowIndex, &aOutIA2Data->mColumnIndex,
         &aOutIA2Data->mRowExtent, &aOutIA2Data->mColumnExtent,
         &aOutIA2Data->mCellIsSelected);
     if (FAILED(hr)) {
       return;
     }
-
-    // Because the same headers can apply to many cells, include the ids of
-    // header cells, rather than the actual objects. Otherwise, we might
-    // end up marshaling the same objects (and their payloads) many times.
-    IUnknown** headers = nullptr;
-    hr = cell->get_rowHeaderCells(&headers, &aOutIA2Data->mNRowHeaderCells);
-    if (FAILED(hr)) {
-      return;
-    }
-    if (aOutIA2Data->mNRowHeaderCells > 0) {
-      // We use midl_user_allocate rather than CoTaskMemAlloc because this
-      // struct is being marshaled by RPC, not COM.
-      aOutIA2Data->mRowHeaderCellIds = static_cast<long*>(
-          ::midl_user_allocate(sizeof(long) * aOutIA2Data->mNRowHeaderCells));
-      for (long i = 0; i < aOutIA2Data->mNRowHeaderCells; ++i) {
-        RefPtr<IAccessible2> headerAcc;
-        hr = headers[i]->QueryInterface(IID_IAccessible2,
-                                        getter_AddRefs(headerAcc));
-        MOZ_ASSERT(succeeded(hr));
-        headers[i]->Release();
-        hr = headerAcc->get_uniqueID(&aOutIA2Data->mRowHeaderCellIds[i]);
-        MOZ_ASSERT(SUCCEEDED(hr));
-      }
-    }
-    ::CoTaskMemFree(headers);
-
-    hr = cell->get_columnHeaderCells(&headers,
-                                     &aOutIA2Data->mNColumnHeaderCells);
-    if (FAILED(hr)) {
-      return;
-    }
-    if (aOutIA2Data->mNColumnHeaderCells > 0) {
-      // We use midl_user_allocate rather than CoTaskMemAlloc because this
-      // struct is being marshaled by RPC, not COM.
-      aOutIA2Data->mColumnHeaderCellIds =
-          static_cast<long*>(::midl_user_allocate(
-              sizeof(long) * aOutIA2Data->mNColumnHeaderCells));
-      for (long i = 0; i < aOutIA2Data->mNColumnHeaderCells; ++i) {
-        RefPtr<IAccessible2> headerAcc;
-        hr = headers[i]->QueryInterface(IID_IAccessible2,
-                                        getter_AddRefs(headerAcc));
-        MOZ_ASSERT(succeeded(hr));
-        headers[i]->Release();
-        hr = headerAcc->get_uniqueID(&aOutIA2Data->mColumnHeaderCellIds[i]);
-        MOZ_ASSERT(SUCCEEDED(hr));
-      }
-    }
-    ::CoTaskMemFree(headers);
   }
 
   // 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);
 }
 
--- a/accessible/ipc/win/handler/AccessibleHandler.cpp
+++ b/accessible/ipc/win/handler/AccessibleHandler.cpp
@@ -493,25 +493,16 @@ AccessibleHandler::ReadHandlerPayload(IS
     return S_OK;
   }
 
   RefPtr<AccessibleHandlerControl> ctl(gControlFactory.GetOrCreateSingleton());
   if (!ctl) {
     return E_OUTOFMEMORY;
   }
 
-  if (mCachedData.mDynamicData.mIA2Role == ROLE_SYSTEM_COLUMNHEADER ||
-      mCachedData.mDynamicData.mIA2Role == ROLE_SYSTEM_ROWHEADER) {
-    // Because the same headers can apply to many cells, handler payloads
-    // include the ids of header cells, rather than potentially marshaling the
-    // same objects many times. We need to cache header cells here so we can
-    // get them by id later.
-    ctl->CacheAccessible(mCachedData.mDynamicData.mUniqueId, this);
-  }
-
   return ctl->Register(WrapNotNull(mCachedData.mGeckoBackChannel));
 }
 
 REFIID
 AccessibleHandler::MarshalAs(REFIID aIid) {
   static_assert(&NEWEST_IA2_IID == &IID_IAccessible2_3,
                 "You have modified NEWEST_IA2_IID. This code needs updating.");
   if (aIid == IID_IAccessible2_3 || aIid == IID_IAccessible2_2 ||
@@ -1531,59 +1522,17 @@ AccessibleHandler::get_columnExtent(long
   BEGIN_CACHE_ACCESS;
   GET_FIELD(mColumnExtent, *nColumnsSpanned);
   return S_OK;
 }
 
 HRESULT
 AccessibleHandler::get_columnHeaderCells(IUnknown*** cellAccessibles,
                                          long* nColumnHeaderCells) {
-  if (!cellAccessibles || !nColumnHeaderCells) {
-    return E_INVALIDARG;
-  }
-
-  HRESULT hr = S_OK;
-  if (HasPayload()) {
-    RefPtr<AccessibleHandlerControl> ctl(
-        gControlFactory.GetOrCreateSingleton());
-    if (!ctl) {
-      return E_OUTOFMEMORY;
-    }
-    *nColumnHeaderCells = mCachedData.mDynamicData.mNColumnHeaderCells;
-    *cellAccessibles = static_cast<IUnknown**>(
-        ::CoTaskMemAlloc(sizeof(IUnknown*) * *nColumnHeaderCells));
-    long i;
-    for (i = 0; i < *nColumnHeaderCells; ++i) {
-      RefPtr<AccessibleHandler> headerAcc;
-      hr = ctl->GetCachedAccessible(
-          mCachedData.mDynamicData.mColumnHeaderCellIds[i],
-          getter_AddRefs(headerAcc));
-      if (FAILED(hr)) {
-        break;
-      }
-      hr = headerAcc->QueryInterface(IID_IUnknown,
-                                     (void**)&(*cellAccessibles)[i]);
-      if (FAILED(hr)) {
-        break;
-      }
-    }
-    if (SUCCEEDED(hr)) {
-      return S_OK;
-    }
-    // If we failed to get any of the headers from the cache, don't use the
-    // cache at all. We need to clean up anything we did so far.
-    long failedHeader = i;
-    for (i = 0; i < failedHeader; ++i) {
-      (*cellAccessibles)[i]->Release();
-    }
-    ::CoTaskMemFree(*cellAccessibles);
-    *cellAccessibles = nullptr;
-  }
-
-  hr = ResolveIATableCell();
+  HRESULT hr = ResolveIATableCell();
   if (FAILED(hr)) {
     return hr;
   }
 
   return mIATableCellPassThru->get_columnHeaderCells(cellAccessibles,
                                                      nColumnHeaderCells);
 }
 
@@ -1623,59 +1572,17 @@ AccessibleHandler::get_rowExtent(long* n
   BEGIN_CACHE_ACCESS;
   GET_FIELD(mRowExtent, *nRowsSpanned);
   return S_OK;
 }
 
 HRESULT
 AccessibleHandler::get_rowHeaderCells(IUnknown*** cellAccessibles,
                                       long* nRowHeaderCells) {
-  if (!cellAccessibles || !nRowHeaderCells) {
-    return E_INVALIDARG;
-  }
-
-  HRESULT hr = S_OK;
-  if (HasPayload()) {
-    RefPtr<AccessibleHandlerControl> ctl(
-        gControlFactory.GetOrCreateSingleton());
-    if (!ctl) {
-      return E_OUTOFMEMORY;
-    }
-    *nRowHeaderCells = mCachedData.mDynamicData.mNRowHeaderCells;
-    *cellAccessibles = static_cast<IUnknown**>(
-        ::CoTaskMemAlloc(sizeof(IUnknown*) * *nRowHeaderCells));
-    long i;
-    for (i = 0; i < *nRowHeaderCells; ++i) {
-      RefPtr<AccessibleHandler> headerAcc;
-      hr = ctl->GetCachedAccessible(
-          mCachedData.mDynamicData.mRowHeaderCellIds[i],
-          getter_AddRefs(headerAcc));
-      if (FAILED(hr)) {
-        break;
-      }
-      hr = headerAcc->QueryInterface(IID_IUnknown,
-                                     (void**)&(*cellAccessibles)[i]);
-      if (FAILED(hr)) {
-        break;
-      }
-    }
-    if (SUCCEEDED(hr)) {
-      return S_OK;
-    }
-    // If we failed to get any of the headers from the cache, don't use the
-    // cache at all. We need to clean up anything we did so far.
-    long failedHeader = i;
-    for (i = 0; i < failedHeader; ++i) {
-      (*cellAccessibles)[i]->Release();
-    }
-    ::CoTaskMemFree(*cellAccessibles);
-    *cellAccessibles = nullptr;
-  }
-
-  hr = ResolveIATableCell();
+  HRESULT hr = ResolveIATableCell();
   if (FAILED(hr)) {
     return hr;
   }
 
   return mIATableCellPassThru->get_rowHeaderCells(cellAccessibles,
                                                   nRowHeaderCells);
 }
 
--- a/accessible/ipc/win/handler/AccessibleHandlerControl.cpp
+++ b/accessible/ipc/win/handler/AccessibleHandlerControl.cpp
@@ -113,17 +113,16 @@ AccessibleHandlerControl::AccessibleHand
   MOZ_ASSERT(mIA2Proxy);
 }
 
 IMPL_IUNKNOWN1(AccessibleHandlerControl, IHandlerControl)
 
 HRESULT
 AccessibleHandlerControl::Invalidate() {
   ++mCacheGen;
-  mAccessibleCache.clear();
   return S_OK;
 }
 
 HRESULT
 AccessibleHandlerControl::OnTextChange(long aHwnd, long aIA2UniqueId,
                                        VARIANT_BOOL aIsInsert,
                                        IA2TextSegment* aText) {
   if (!aText) {
@@ -167,28 +166,10 @@ AccessibleHandlerControl::Register(NotNu
 
   long pid = static_cast<long>(::GetCurrentProcessId());
   HRESULT hr = aGecko->put_HandlerControl(pid, this);
   mIsRegistered = SUCCEEDED(hr);
   MOZ_ASSERT(mIsRegistered);
   return hr;
 }
 
-void AccessibleHandlerControl::CacheAccessible(long aUniqueId,
-                                               AccessibleHandler* aAccessible) {
-  MOZ_ASSERT(aUniqueId && aAccessible);
-  mAccessibleCache[aUniqueId] = aAccessible;
-}
-
-HRESULT AccessibleHandlerControl::GetCachedAccessible(
-    long aUniqueId, AccessibleHandler** aAccessible) {
-  MOZ_ASSERT(aUniqueId && aAccessible);
-  auto it = mAccessibleCache.find(aUniqueId);
-  if (it == mAccessibleCache.end()) {
-    return E_INVALIDARG;
-  }
-  RefPtr<AccessibleHandler> ref = it->second;
-  ref.forget(aAccessible);
-  return S_OK;
-}
-
 }  // namespace a11y
 }  // namespace mozilla
--- a/accessible/ipc/win/handler/AccessibleHandlerControl.h
+++ b/accessible/ipc/win/handler/AccessibleHandlerControl.h
@@ -6,17 +6,16 @@
 
 #if defined(MOZILLA_INTERNAL_API)
 #  error This code is NOT for internal Gecko use!
 #endif  // defined(MOZILLA_INTERNAL_API)
 
 #ifndef mozilla_a11y_AccessibleHandlerControl_h
 #  define mozilla_a11y_AccessibleHandlerControl_h
 
-#  include <unordered_map>
 #  include "Factory.h"
 #  include "HandlerData.h"
 #  include "IUnknownImpl.h"
 #  include "mozilla/mscom/Registration.h"
 #  include "mozilla/NotNull.h"
 
 namespace mozilla {
 namespace a11y {
@@ -44,18 +43,16 @@ class TextChange final {
 
   long mIA2UniqueId;
   bool mIsInsert;
   IA2TextSegment mText;
 };
 
 }  // namespace detail
 
-class AccessibleHandler;
-
 class AccessibleHandlerControl final : public IHandlerControl {
  public:
   static HRESULT Create(AccessibleHandlerControl** aOutObject);
 
   DECL_IUNKNOWN
 
   // IHandlerControl
   STDMETHODIMP Invalidate() override;
@@ -67,30 +64,25 @@ class AccessibleHandlerControl final : p
 
   HRESULT GetNewText(long aIA2UniqueId, NotNull<IA2TextSegment*> aOutNewText);
   HRESULT GetOldText(long aIA2UniqueId, NotNull<IA2TextSegment*> aOutOldText);
 
   HRESULT GetHandlerTypeInfo(ITypeInfo** aOutTypeInfo);
 
   HRESULT Register(NotNull<IGeckoBackChannel*> aGecko);
 
-  void CacheAccessible(long aUniqueId, AccessibleHandler* aAccessible);
-  HRESULT GetCachedAccessible(long aUniqueId, AccessibleHandler** aAccessible);
-
  private:
   AccessibleHandlerControl();
   ~AccessibleHandlerControl() = default;
 
   bool mIsRegistered;
   uint32_t mCacheGen;
   detail::TextChange mTextChange;
   UniquePtr<mscom::RegisteredProxy> mIA2Proxy;
   UniquePtr<mscom::RegisteredProxy> mHandlerProxy;
-  // We can't use Gecko APIs in this dll, hence the use of std::unordered_map.
-  std::unordered_map<long, RefPtr<AccessibleHandler>> mAccessibleCache;
 };
 
 extern mscom::SingletonFactory<AccessibleHandlerControl> gControlFactory;
 
 }  // namespace a11y
 }  // namespace mozilla
 
 #endif  // mozilla_a11y_AccessibleHandlerControl_h
--- a/accessible/ipc/win/handler/HandlerData.idl
+++ b/accessible/ipc/win/handler/HandlerData.idl
@@ -50,20 +50,16 @@ typedef struct _DynamicIA2Data
   // From IAccessibleAction
   long              mNActions;
   // From IAccessibleTableCell
   long              mRowIndex;
   long              mColumnIndex;
   long              mRowExtent;
   long              mColumnExtent;
   boolean           mCellIsSelected;
-  long mNRowHeaderCells;
-  [size_is(mNRowHeaderCells)] long* mRowHeaderCellIds;
-  long mNColumnHeaderCells;
-  [size_is(mNColumnHeaderCells)] long* mColumnHeaderCellIds;
   // From IAccessible2
   long              mUniqueId;
 } DynamicIA2Data;
 
 interface IGeckoBackChannel;
 
 // We define different CLSIDs and IIDs depending on channel and officiality.
 // This prevents handlers from installing overtop one another when multiple
--- a/accessible/ipc/win/handler/HandlerDataCleanup.h
+++ b/accessible/ipc/win/handler/HandlerDataCleanup.h
@@ -61,20 +61,14 @@ inline void CleanupDynamicIA2Data(Dynami
     ::SysFreeString(aData.mIA2Locale.language);
   }
   if (aData.mIA2Locale.country) {
     ::SysFreeString(aData.mIA2Locale.country);
   }
   if (aData.mIA2Locale.variant) {
     ::SysFreeString(aData.mIA2Locale.variant);
   }
-  if (aData.mRowHeaderCellIds) {
-    ::midl_user_free(aData.mRowHeaderCellIds);
-  }
-  if (aData.mColumnHeaderCellIds) {
-    ::midl_user_free(aData.mColumnHeaderCellIds);
-  }
 }
 
 }  // namespace a11y
 }  // namespace mozilla
 
 #endif  // mozilla_a11y_HandlerDataCleanup_h