Bug 1413072: Eliminate pointless cross-process QueryInterface for IAccessibleText in AccessibleHandler. r=MarcoZ draft
authorJames Teh <jteh@mozilla.com>
Tue, 31 Oct 2017 12:11:39 +1000
changeset 691809 f508100e4f588983e155dec8dfc3907a70063c6b
parent 691808 a0334f789772302ba5cfb6fd61290408842c7432
child 738586 1845bb87212d8f75eaa6afe742ff9f81fb9d91cc
push id87315
push userbmo:jteh@mozilla.com
push dateThu, 02 Nov 2017 02:20:43 +0000
reviewersMarcoZ
bugs1413072
milestone58.0a1
Bug 1413072: Eliminate pointless cross-process QueryInterface for IAccessibleText in AccessibleHandler. r=MarcoZ AccessibleHandler's AccessibleTextTearoff currently makes separate queries for the IAccessibleText and IAccessibleHypertext interfaces. However, IAccessibleHypertext inherits from IAccessibleText, and wherever one of these interfaces are present, Gecko always implements both. Therefore, we should just query for and use IAccessibleHypertext for all of these methods, thus saving a cross-process call should a client want both interfaces. MozReview-Commit-ID: Fb5P7IGDAZZ
accessible/ipc/win/handler/AccessibleTextTearoff.cpp
accessible/ipc/win/handler/AccessibleTextTearoff.h
--- a/accessible/ipc/win/handler/AccessibleTextTearoff.cpp
+++ b/accessible/ipc/win/handler/AccessibleTextTearoff.cpp
@@ -22,32 +22,16 @@ namespace a11y {
 
 AccessibleTextTearoff::AccessibleTextTearoff(AccessibleHandler* aHandler)
   : mHandler(aHandler)
 {
   MOZ_ASSERT(aHandler);
 }
 
 HRESULT
-AccessibleTextTearoff::ResolveAccText()
-{
-  if (mAccTextProxy) {
-    return S_OK;
-  }
-
-  RefPtr<IUnknown> proxy(mHandler->GetProxy());
-  if (!proxy) {
-    return E_UNEXPECTED;
-  }
-
-  return proxy->QueryInterface(IID_IAccessibleText,
-                               getter_AddRefs(mAccTextProxy));
-}
-
-HRESULT
 AccessibleTextTearoff::ResolveAccHypertext()
 {
   if (mAccHypertextProxy) {
     return S_OK;
   }
 
   RefPtr<IUnknown> proxy(mHandler->GetProxy());
   if (!proxy) {
@@ -61,223 +45,223 @@ AccessibleTextTearoff::ResolveAccHyperte
 IMPL_IUNKNOWN_QUERY_HEAD(AccessibleTextTearoff)
 IMPL_IUNKNOWN_QUERY_IFACE(IAccessibleText)
 IMPL_IUNKNOWN_QUERY_IFACE(IAccessibleHypertext)
 IMPL_IUNKNOWN_QUERY_TAIL_AGGREGATED(mHandler)
 
 HRESULT
 AccessibleTextTearoff::addSelection(long startOffset, long endOffset)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->addSelection(startOffset, endOffset);
+  return mAccHypertextProxy->addSelection(startOffset, endOffset);
 }
 
 HRESULT
 AccessibleTextTearoff::get_attributes(long offset, long *startOffset,
                                       long *endOffset, BSTR *textAttributes)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_attributes(offset, startOffset, endOffset,
+  return mAccHypertextProxy->get_attributes(offset, startOffset, endOffset,
                                        textAttributes);
 }
 
 HRESULT
 AccessibleTextTearoff::get_caretOffset(long *offset)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_caretOffset(offset);
+  return mAccHypertextProxy->get_caretOffset(offset);
 }
 
 HRESULT
 AccessibleTextTearoff::get_characterExtents(long offset,
                                             enum IA2CoordinateType coordType,
                                             long *x, long *y, long *width,
                                             long *height)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_characterExtents(offset, coordType, x, y, width,
+  return mAccHypertextProxy->get_characterExtents(offset, coordType, x, y, width,
                                              height);
 }
 
 HRESULT
 AccessibleTextTearoff::get_nSelections(long *nSelections)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_nSelections(nSelections);
+  return mAccHypertextProxy->get_nSelections(nSelections);
 }
 
 HRESULT
 AccessibleTextTearoff::get_offsetAtPoint(long x, long y,
                                          enum IA2CoordinateType coordType,
                                          long *offset)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_offsetAtPoint(x, y, coordType, offset);
+  return mAccHypertextProxy->get_offsetAtPoint(x, y, coordType, offset);
 }
 
 HRESULT
 AccessibleTextTearoff::get_selection(long selectionIndex, long *startOffset,
                                      long *endOffset)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_selection(selectionIndex, startOffset, endOffset);
+  return mAccHypertextProxy->get_selection(selectionIndex, startOffset, endOffset);
 }
 
 HRESULT
 AccessibleTextTearoff::get_text(long startOffset, long endOffset, BSTR *text)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_text(startOffset, endOffset, text);
+  return mAccHypertextProxy->get_text(startOffset, endOffset, text);
 }
 
 HRESULT
 AccessibleTextTearoff::get_textBeforeOffset(long offset,
                                             enum IA2TextBoundaryType boundaryType,
                                             long *startOffset, long *endOffset,
                                             BSTR *text)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_textBeforeOffset(offset, boundaryType, startOffset,
+  return mAccHypertextProxy->get_textBeforeOffset(offset, boundaryType, startOffset,
                                              endOffset, text);
 }
 
 HRESULT
 AccessibleTextTearoff::get_textAfterOffset(long offset,
                                            enum IA2TextBoundaryType boundaryType,
                                            long *startOffset, long *endOffset,
                                            BSTR *text)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_textAfterOffset(offset, boundaryType,
+  return mAccHypertextProxy->get_textAfterOffset(offset, boundaryType,
                                             startOffset, endOffset, text);
 }
 
 HRESULT
 AccessibleTextTearoff::get_textAtOffset(long offset,
                                         enum IA2TextBoundaryType boundaryType,
                                         long *startOffset, long *endOffset,
                                         BSTR *text)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_textAtOffset(offset, boundaryType, startOffset,
+  return mAccHypertextProxy->get_textAtOffset(offset, boundaryType, startOffset,
                                          endOffset, text);
 }
 
 HRESULT
 AccessibleTextTearoff::removeSelection(long selectionIndex)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->removeSelection(selectionIndex);
+  return mAccHypertextProxy->removeSelection(selectionIndex);
 }
 
 HRESULT
 AccessibleTextTearoff::setCaretOffset(long offset)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->setCaretOffset(offset);
+  return mAccHypertextProxy->setCaretOffset(offset);
 }
 
 HRESULT
 AccessibleTextTearoff::setSelection(long selectionIndex, long startOffset,
                                     long endOffset)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->setSelection(selectionIndex, startOffset, endOffset);
+  return mAccHypertextProxy->setSelection(selectionIndex, startOffset, endOffset);
 }
 
 HRESULT
 AccessibleTextTearoff::get_nCharacters(long *nCharacters)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->get_nCharacters(nCharacters);
+  return mAccHypertextProxy->get_nCharacters(nCharacters);
 }
 
 HRESULT
 AccessibleTextTearoff::scrollSubstringTo(long startIndex, long endIndex,
                                          enum IA2ScrollType scrollType)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->scrollSubstringTo(startIndex, endIndex, scrollType);
+  return mAccHypertextProxy->scrollSubstringTo(startIndex, endIndex, scrollType);
 }
 
 HRESULT
 AccessibleTextTearoff::scrollSubstringToPoint(long startIndex, long endIndex,
                                               enum IA2CoordinateType coordinateType,
                                               long x, long y)
 {
-  HRESULT hr = ResolveAccText();
+  HRESULT hr = ResolveAccHypertext();
   if (FAILED(hr)) {
     return hr;
   }
 
-  return mAccTextProxy->scrollSubstringToPoint(startIndex, endIndex,
+  return mAccHypertextProxy->scrollSubstringToPoint(startIndex, endIndex,
                                                coordinateType, x, y);
 }
 
 HRESULT
 AccessibleTextTearoff::get_newText(IA2TextSegment *newText)
 {
   if (!newText) {
     return E_INVALIDARG;
--- a/accessible/ipc/win/handler/AccessibleTextTearoff.h
+++ b/accessible/ipc/win/handler/AccessibleTextTearoff.h
@@ -69,20 +69,18 @@ public:
   // IAccessibleHypertext
   STDMETHODIMP get_nHyperlinks(long *hyperlinkCount) override;
   STDMETHODIMP get_hyperlink(long index,
                              IAccessibleHyperlink **hyperlink) override;
   STDMETHODIMP get_hyperlinkIndex(long charIndex, long *hyperlinkIndex) override;
 
 private:
   ~AccessibleTextTearoff() = default;
-  HRESULT ResolveAccText();
   HRESULT ResolveAccHypertext();
 
   RefPtr<AccessibleHandler>     mHandler;
-  RefPtr<IAccessibleText>       mAccTextProxy;
   RefPtr<IAccessibleHypertext>  mAccHypertextProxy;
 };
 
 } // namespace a11y
 } // namespace mozilla
 
 #endif // mozilla_a11y_AccessibleTextTearoff_h