From 1a4038f6e755d80853025d3108598cb5707799e4 Mon Sep 17 00:00:00 2001 From: Simon Giesecke Date: Mon, 1 Mar 2021 09:59:30 +0000 Subject: [PATCH] Bug 1693541 - Improve uses of nsBaseHashtable and descendants and avoid multiple subsequent lookups in accessible. r=Jamie Differential Revision: https://phabricator.services.mozilla.com/D106093 --- accessible/android/DocAccessibleWrap.cpp | 12 +++- accessible/base/DocManager.cpp | 8 +-- accessible/generic/DocAccessible.cpp | 20 +++--- accessible/windows/msaa/MsaaIdGenerator.cpp | 38 ++++++----- accessible/xpcom/xpcAccessibleDocument.cpp | 70 +++++++++------------ accessible/xul/XULTreeAccessible.cpp | 22 ++++--- 6 files changed, 84 insertions(+), 86 deletions(-) diff --git a/accessible/android/DocAccessibleWrap.cpp b/accessible/android/DocAccessibleWrap.cpp index ee93d28a1be8..7dd0ce641cd1 100644 --- a/accessible/android/DocAccessibleWrap.cpp +++ b/accessible/android/DocAccessibleWrap.cpp @@ -117,10 +117,18 @@ void DocAccessibleWrap::CacheViewportCallback(nsITimer* aTimer, for (LocalAccessible* acc = visibleAcc; acc && acc != docAcc->LocalParent(); acc = acc->LocalParent()) { - if (inViewAccs.Contains(acc->UniqueID())) { + const bool alreadyPresent = + inViewAccs.WithEntryHandle(acc->UniqueID(), [&](auto&& entry) { + if (entry) { + return true; + } + + entry.Insert(RefPtr{acc}); + return false; + }); + if (alreadyPresent) { break; } - inViewAccs.InsertOrUpdate(acc->UniqueID(), RefPtr{acc}); } } diff --git a/accessible/base/DocManager.cpp b/accessible/base/DocManager.cpp index d3606fed754d..a2d52e79da16 100644 --- a/accessible/base/DocManager.cpp +++ b/accessible/base/DocManager.cpp @@ -140,12 +140,8 @@ void DocManager::NotifyOfRemoteDocShutdown(DocAccessibleParent* aDoc) { xpcAccessibleDocument* DocManager::GetXPCDocument(DocAccessible* aDocument) { if (!aDocument) return nullptr; - xpcAccessibleDocument* xpcDoc = mXPCDocumentCache.GetWeak(aDocument); - if (!xpcDoc) { - xpcDoc = new xpcAccessibleDocument(aDocument); - mXPCDocumentCache.InsertOrUpdate(aDocument, RefPtr{xpcDoc}); - } - return xpcDoc; + return mXPCDocumentCache.LookupOrInsertWith( + aDocument, [&] { return MakeRefPtr(aDocument); }); } xpcAccessibleDocument* DocManager::GetXPCDocument(DocAccessibleParent* aDoc) { diff --git a/accessible/generic/DocAccessible.cpp b/accessible/generic/DocAccessible.cpp index 4be9a44f9399..ac2991fcba0c 100644 --- a/accessible/generic/DocAccessible.cpp +++ b/accessible/generic/DocAccessible.cpp @@ -593,18 +593,20 @@ void DocAccessible::ScrollTimerCallback(nsITimer* aTimer, void* aClosure) { void DocAccessible::HandleScroll(nsINode* aTarget) { const uint32_t kScrollEventInterval = 100; - TimeStamp now = TimeStamp::Now(); - TimeStamp lastDispatch; // If we haven't dispatched a scrolling event for a target in at least // kScrollEventInterval milliseconds, dispatch one now. - if (!mLastScrollingDispatch.Get(aTarget, &lastDispatch) || - (now - lastDispatch).ToMilliseconds() >= kScrollEventInterval) { - // We can't fire events on a document whose tree isn't constructed yet. - if (HasLoadState(eTreeConstructed)) { - DispatchScrollingEvent(aTarget, nsIAccessibleEvent::EVENT_SCROLLING); + mLastScrollingDispatch.WithEntryHandle(aTarget, [&](auto&& lastDispatch) { + const TimeStamp now = TimeStamp::Now(); + + if (!lastDispatch || + (now - lastDispatch.Data()).ToMilliseconds() >= kScrollEventInterval) { + // We can't fire events on a document whose tree isn't constructed yet. + if (HasLoadState(eTreeConstructed)) { + DispatchScrollingEvent(aTarget, nsIAccessibleEvent::EVENT_SCROLLING); + } + lastDispatch.InsertOrUpdate(now); } - mLastScrollingDispatch.InsertOrUpdate(aTarget, now); - } + }); // If timer callback is still pending, push it 100ms into the future. // When scrolling ends and we don't fire this callback anymore, the diff --git a/accessible/windows/msaa/MsaaIdGenerator.cpp b/accessible/windows/msaa/MsaaIdGenerator.cpp index 3eb66457073a..fa3c292b8447 100644 --- a/accessible/windows/msaa/MsaaIdGenerator.cpp +++ b/accessible/windows/msaa/MsaaIdGenerator.cpp @@ -174,29 +174,27 @@ uint32_t MsaaIdGenerator::GetContentProcessIDFor( ClearOnShutdown(&sContentParentIdMap); } - uint32_t value = 0; - if (sContentParentIdMap->Get(aIPCContentProcessID, &value)) { - return value; - } - - uint32_t index = 0; - for (; index < ArrayLength(sContentProcessIdBitmap); ++index) { - if (sContentProcessIdBitmap[index] == UINT64_MAX) { - continue; + return sContentParentIdMap->LookupOrInsertWith(aIPCContentProcessID, [] { + uint32_t value = 0; + uint32_t index = 0; + for (; index < ArrayLength(sContentProcessIdBitmap); ++index) { + if (sContentProcessIdBitmap[index] == UINT64_MAX) { + continue; + } + uint32_t bitIndex = + CountTrailingZeroes64(~sContentProcessIdBitmap[index]); + MOZ_ASSERT(!(sContentProcessIdBitmap[index] & (1ULL << bitIndex))); + MOZ_ASSERT(bitIndex != 0 || index != 0); + sContentProcessIdBitmap[index] |= (1ULL << bitIndex); + value = index * kBitsPerElement + bitIndex; + break; } - uint32_t bitIndex = CountTrailingZeroes64(~sContentProcessIdBitmap[index]); - MOZ_ASSERT(!(sContentProcessIdBitmap[index] & (1ULL << bitIndex))); - MOZ_ASSERT(bitIndex != 0 || index != 0); - sContentProcessIdBitmap[index] |= (1ULL << bitIndex); - value = index * kBitsPerElement + bitIndex; - break; - } - // If we run out of content process IDs, we're in trouble - MOZ_RELEASE_ASSERT(index < ArrayLength(sContentProcessIdBitmap)); + // If we run out of content process IDs, we're in trouble + MOZ_RELEASE_ASSERT(index < ArrayLength(sContentProcessIdBitmap)); - sContentParentIdMap->InsertOrUpdate(aIPCContentProcessID, value); - return value; + return value; + }); } void MsaaIdGenerator::ReleaseContentProcessIDFor( diff --git a/accessible/xpcom/xpcAccessibleDocument.cpp b/accessible/xpcom/xpcAccessibleDocument.cpp index bb7e1fff1ad3..c1c43383dc53 100644 --- a/accessible/xpcom/xpcAccessibleDocument.cpp +++ b/accessible/xpcom/xpcAccessibleDocument.cpp @@ -157,23 +157,22 @@ xpcAccessibleGeneric* xpcAccessibleDocument::GetAccessible( if (aAccessible->IsDoc()) return this; - xpcAccessibleGeneric* xpcAcc = mCache.Get(aAccessible); - if (xpcAcc) return xpcAcc; + return mCache.LookupOrInsertWith(aAccessible, [&]() -> xpcAccessibleGeneric* { + if (aAccessible->IsImage()) { + return new xpcAccessibleImage(aAccessible); + } + if (aAccessible->IsTable()) { + return new xpcAccessibleTable(aAccessible); + } + if (aAccessible->IsTableCell()) { + return new xpcAccessibleTableCell(aAccessible); + } + if (aAccessible->IsHyperText()) { + return new xpcAccessibleHyperText(aAccessible); + } - if (aAccessible->IsImage()) { - xpcAcc = new xpcAccessibleImage(aAccessible); - } else if (aAccessible->IsTable()) { - xpcAcc = new xpcAccessibleTable(aAccessible); - } else if (aAccessible->IsTableCell()) { - xpcAcc = new xpcAccessibleTableCell(aAccessible); - } else if (aAccessible->IsHyperText()) { - xpcAcc = new xpcAccessibleHyperText(aAccessible); - } else { - xpcAcc = new xpcAccessibleGeneric(aAccessible); - } - - mCache.InsertOrUpdate(aAccessible, xpcAcc); - return xpcAcc; + return new xpcAccessibleGeneric(aAccessible); + }); } xpcAccessibleGeneric* xpcAccessibleDocument::GetXPCAccessible( @@ -184,33 +183,24 @@ xpcAccessibleGeneric* xpcAccessibleDocument::GetXPCAccessible( return this; } - xpcAccessibleGeneric* acc = mCache.Get(aProxy); - if (acc) { - return acc; - } + return mCache.LookupOrInsertWith(aProxy, [&]() -> xpcAccessibleGeneric* { + // XXX support exposing optional interfaces. + uint8_t interfaces = 0; + if (aProxy->mHasValue) { + interfaces |= eValue; + } - // XXX support exposing optional interfaces. - uint8_t interfaces = 0; - if (aProxy->mHasValue) { - interfaces |= eValue; - } + if (aProxy->mIsHyperLink) { + interfaces |= eHyperLink; + } - if (aProxy->mIsHyperLink) { - interfaces |= eHyperLink; - } + if (aProxy->mIsHyperText) { + interfaces |= eText; + return new xpcAccessibleHyperText(aProxy, interfaces); + } - if (aProxy->mIsHyperText) { - interfaces |= eText; - acc = new xpcAccessibleHyperText(aProxy, interfaces); - mCache.InsertOrUpdate(aProxy, acc); - - return acc; - } - - acc = new xpcAccessibleGeneric(aProxy, interfaces); - mCache.InsertOrUpdate(aProxy, acc); - - return acc; + return new xpcAccessibleGeneric(aProxy, interfaces); + }); } void xpcAccessibleDocument::Shutdown() { diff --git a/accessible/xul/XULTreeAccessible.cpp b/accessible/xul/XULTreeAccessible.cpp index f0da334853d0..45116d2101b2 100644 --- a/accessible/xul/XULTreeAccessible.cpp +++ b/accessible/xul/XULTreeAccessible.cpp @@ -438,17 +438,21 @@ LocalAccessible* XULTreeAccessible::GetTreeItemAccessible(int32_t aRow) const { if (NS_FAILED(rv) || aRow >= rowCount) return nullptr; void* key = reinterpret_cast(intptr_t(aRow)); - LocalAccessible* cachedTreeItem = mAccessibleCache.GetWeak(key); - if (cachedTreeItem) return cachedTreeItem; + return mAccessibleCache.WithEntryHandle( + key, [&](auto&& entry) -> LocalAccessible* { + if (entry) { + return entry->get(); + } - RefPtr treeItem = CreateTreeItemAccessible(aRow); - if (treeItem) { - mAccessibleCache.InsertOrUpdate(key, RefPtr{treeItem}); - Document()->BindToDocument(treeItem, nullptr); - return treeItem; - } + RefPtr treeItem = CreateTreeItemAccessible(aRow); + if (treeItem) { + entry.Insert(RefPtr{treeItem}); + Document()->BindToDocument(treeItem, nullptr); + return treeItem.get(); + } - return nullptr; + return nullptr; + }); } void XULTreeAccessible::InvalidateCache(int32_t aRow, int32_t aCount) {