Bug 1419362 part 4: AccessibleHandler: When a caller asks for all text, cache hyperlinks and text attributes in the same call. r?MarcoZ draft
authorJames Teh <jteh@mozilla.com>
Wed, 22 Nov 2017 21:32:28 +1000
changeset 703077 c55ed6628bcd7283ad296d95c9a829ec6909b7a7
parent 703076 6ca36ba4e39e48f8a2cc48f7c02cc48f161c5096
child 703078 bc7ada8d7e9bf57f86918dda812ac2b19fbcb99d
push id90698
push userbmo:jteh@mozilla.com
push dateFri, 24 Nov 2017 09:13:42 +0000
reviewersMarcoZ
bugs1419362
milestone59.0a1
Bug 1419362 part 4: AccessibleHandler: When a caller asks for all text, cache hyperlinks and text attributes in the same call. r?MarcoZ If a client requests all text (via IAccessibleText::text with IA2_TEXT_OFFSET_LENGTH), it's quite likely they will want all other information about the text as well; i.e. embedded objects and attributes. Therefore, fetch all of this using a single cross-process call. The text is immediately returned to the client. The hyperlinks and attributes are cached for later return to the client when they call the appropriate methods. They are only cached for one call; i.e. after the client retrieves them, the cache is dropped. This makes memory management simpler and lowers the risk of cache invalidation problems. MozReview-Commit-ID: FgFkX8J7wg1
accessible/ipc/win/handler/AccessibleHandler.cpp
accessible/ipc/win/handler/AccessibleHandler.h
--- a/accessible/ipc/win/handler/AccessibleHandler.cpp
+++ b/accessible/ipc/win/handler/AccessibleHandler.cpp
@@ -68,16 +68,20 @@ AccessibleHandler::AccessibleHandler(IUn
   , mDispatch(nullptr)
   , mIA2PassThru(nullptr)
   , mServProvPassThru(nullptr)
   , mIAHyperlinkPassThru(nullptr)
   , mIATableCellPassThru(nullptr)
   , mIAHypertextPassThru(nullptr)
   , mCachedData()
   , mCacheGen(0)
+  , mCachedHyperlinks(nullptr)
+  , mCachedNHyperlinks(-1)
+  , mCachedTextAttribRuns(nullptr)
+  , mCachedNTextAttribRuns(-1)
 {
   RefPtr<AccessibleHandlerControl> ctl(gControlFactory.GetOrCreateSingleton());
   MOZ_ASSERT(ctl);
   if (!ctl) {
     if (aResult) {
       *aResult = E_UNEXPECTED;
     }
     return;
@@ -86,16 +90,17 @@ AccessibleHandler::AccessibleHandler(IUn
   mCacheGen = ctl->GetCacheGen();
 }
 
 AccessibleHandler::~AccessibleHandler()
 {
   if (mCachedData.mGeckoBackChannel) {
     mCachedData.mGeckoBackChannel->Release();
   }
+  ClearTextCache();
 }
 
 HRESULT
 AccessibleHandler::ResolveIA2()
 {
   if (mIA2PassThru) {
     return S_OK;
   }
@@ -200,16 +205,58 @@ AccessibleHandler::MaybeUpdateCachedData
   if (!mCachedData.mGeckoBackChannel) {
     return E_POINTER;
   }
 
   return mCachedData.mGeckoBackChannel->Refresh(&mCachedData.mDynamicData);
 }
 
 HRESULT
+AccessibleHandler::GetAllTextInfo(BSTR* aText)
+{
+  MOZ_ASSERT(mCachedData.mGeckoBackChannel);
+
+  ClearTextCache();
+
+  return mCachedData.mGeckoBackChannel->get_AllTextInfo(aText,
+    &mCachedHyperlinks, &mCachedNHyperlinks,
+    &mCachedTextAttribRuns, &mCachedNTextAttribRuns);
+}
+
+void
+AccessibleHandler::ClearTextCache()
+{
+  if (mCachedNHyperlinks >= 0) {
+    // We cached hyperlinks, but the caller never retrieved them.
+    for (long index = 0; index < mCachedNHyperlinks; ++index) {
+      mCachedHyperlinks[index]->Release();
+    }
+    // mCachedHyperlinks might already be null if there are no hyperlinks.
+    if (mCachedHyperlinks) {
+      ::CoTaskMemFree(mCachedHyperlinks);
+      mCachedHyperlinks = nullptr;
+    }
+    mCachedNHyperlinks = -1;
+  }
+
+  if (mCachedTextAttribRuns) {
+    for (long index = 0; index < mCachedNTextAttribRuns; ++index) {
+      if (mCachedTextAttribRuns[index].text) {
+        // The caller never requested this attribute run.
+        ::SysFreeString(mCachedTextAttribRuns[index].text);
+      }
+    }
+    // This array is internal to us, so we must always free it.
+    ::CoTaskMemFree(mCachedTextAttribRuns);
+    mCachedTextAttribRuns = nullptr;
+    mCachedNTextAttribRuns = -1;
+  }
+}
+
+HRESULT
 AccessibleHandler::ResolveIDispatch()
 {
   if (mDispatch) {
     return S_OK;
   }
 
   HRESULT hr;
 
@@ -1572,16 +1619,38 @@ AccessibleHandler::addSelection(long sta
 
   return mIAHypertextPassThru->addSelection(startOffset, endOffset);
 }
 
 HRESULT
 AccessibleHandler::get_attributes(long offset, long *startOffset,
                                   long *endOffset, BSTR *textAttributes)
 {
+  if (!startOffset || !endOffset || !textAttributes) {
+    return E_INVALIDARG;
+  }
+
+  if (mCachedNTextAttribRuns >= 0) {
+    // We have cached attributes.
+    for (long index = 0; index < mCachedNTextAttribRuns; ++index) {
+      auto& attribRun = mCachedTextAttribRuns[index];
+      if (attribRun.start <= offset && offset < attribRun.end) {
+        *startOffset = attribRun.start;
+        *endOffset = attribRun.end;
+        *textAttributes = attribRun.text;
+        // The caller will clean this up.
+        // (We only keep each cached attribute run for one call.)
+        attribRun.text = nullptr;
+        // The cache for this run is now invalid, so don't visit it again.
+        attribRun.end = 0;
+        return S_OK;
+      }
+    }
+  }
+
   HRESULT hr = ResolveIAHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
   return mIAHypertextPassThru->get_attributes(offset, startOffset, endOffset,
                                               textAttributes);
 }
@@ -1647,17 +1716,33 @@ AccessibleHandler::get_selection(long se
 
   return mIAHypertextPassThru->get_selection(selectionIndex, startOffset,
                                              endOffset);
 }
 
 HRESULT
 AccessibleHandler::get_text(long startOffset, long endOffset, BSTR *text)
 {
-  HRESULT hr = ResolveIAHypertext();
+  if (!text) {
+    return E_INVALIDARG;
+  }
+
+  HRESULT hr;
+  if (mCachedData.mGeckoBackChannel &&
+      startOffset == 0 && endOffset == IA2_TEXT_OFFSET_LENGTH) {
+    // If the caller is retrieving all text, they will probably want all
+    // hyperlinks and attributes as well.
+    hr = GetAllTextInfo(text);
+    if (SUCCEEDED(hr)) {
+      return hr;
+    }
+    // We fall back to a normal call if this fails.
+  }
+
+  hr = ResolveIAHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
   return mIAHypertextPassThru->get_text(startOffset, endOffset, text);
 }
 
 HRESULT
@@ -1861,16 +1946,30 @@ AccessibleHandler::get_hyperlinkIndex(lo
 }
 
 /*** IAccessibleHypertext2 ***/
 
 HRESULT
 AccessibleHandler::get_hyperlinks(IAccessibleHyperlink*** hyperlinks,
                                   long* nHyperlinks)
 {
+  if (!hyperlinks || !nHyperlinks) {
+    return E_INVALIDARG;
+  }
+
+  if (mCachedNHyperlinks >= 0) {
+    // We have cached hyperlinks.
+    *hyperlinks = mCachedHyperlinks;
+    *nHyperlinks = mCachedNHyperlinks;
+    // The client will clean these up. (We only keep the cache for one call.)
+    mCachedHyperlinks = nullptr;
+    mCachedNHyperlinks = -1;
+    return mCachedNHyperlinks == 0 ? S_FALSE : S_OK;
+  }
+
   HRESULT hr = ResolveIAHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
   return mIAHypertextPassThru->get_hyperlinks(hyperlinks, nHyperlinks);
 }
 
--- a/accessible/ipc/win/handler/AccessibleHandler.h
+++ b/accessible/ipc/win/handler/AccessibleHandler.h
@@ -247,16 +247,18 @@ private:
   virtual ~AccessibleHandler();
 
   HRESULT ResolveIA2();
   HRESULT ResolveIDispatch();
   HRESULT ResolveIAHyperlink();
   HRESULT ResolveIAHypertext();
   HRESULT ResolveIATableCell();
   HRESULT MaybeUpdateCachedData();
+  HRESULT GetAllTextInfo(BSTR* aText);
+  void ClearTextCache();
 
   RefPtr<IUnknown>                  mDispatchUnk;
   /**
    * Handlers aggregate their proxies. This means that their proxies delegate
    * their IUnknown implementation to us.
    *
    * mDispatchUnk and the result of Handler::GetProxy() are both strong
    * references to the aggregated objects. OTOH, any interfaces that are QI'd
@@ -277,16 +279,20 @@ private:
   NEWEST_IA2_INTERFACE*             mIA2PassThru;      // weak
   IServiceProvider*                 mServProvPassThru; // weak
   IAccessibleHyperlink*             mIAHyperlinkPassThru; // weak
   IAccessibleTableCell*             mIATableCellPassThru; // weak
   IAccessibleHypertext2*             mIAHypertextPassThru; // weak
   IA2Payload                        mCachedData;
   UniquePtr<mscom::StructToStream>  mSerializer;
   uint32_t                          mCacheGen;
+  IAccessibleHyperlink**            mCachedHyperlinks;
+  long                              mCachedNHyperlinks;
+  IA2TextSegment*                   mCachedTextAttribRuns;
+  long                              mCachedNTextAttribRuns;
 };
 
 } // namespace a11y
 } // namespace mozilla
 
 #endif // !defined(MOZILLA_INTERNAL_API)
 
 #endif // defined(__midl)