From b494f3eab9def84fc9cd5a44989c8872bfe4ad8b Mon Sep 17 00:00:00 2001 From: Eitan Isaacson Date: Mon, 31 Jan 2022 23:10:52 +0000 Subject: [PATCH] Bug 1743967 - Support async DoAction. r=Jamie Now that we cache ActionCount, we can check for the absence of actions and return false, or send an async message and return true. Differential Revision: https://phabricator.services.mozilla.com/D135909 --- accessible/atk/nsMaiInterfaceAction.cpp | 9 ++++----- accessible/basetypes/Accessible.h | 5 +++++ accessible/generic/LocalAccessible.h | 5 +---- accessible/html/HTMLElementAccessibles.cpp | 8 +++++--- accessible/ipc/DocAccessibleChildBase.cpp | 9 +++++++++ accessible/ipc/DocAccessibleChildBase.h | 3 +++ accessible/ipc/RemoteAccessibleBase.cpp | 10 ++++++++++ accessible/ipc/RemoteAccessibleBase.h | 2 ++ accessible/ipc/RemoteAccessibleShared.h | 1 - accessible/ipc/other/PDocAccessible.ipdl | 1 + accessible/ipc/other/RemoteAccessible.cpp | 6 +++++- accessible/ipc/other/RemoteAccessible.h | 2 ++ accessible/ipc/win/PDocAccessible.ipdl | 2 ++ .../tests/browser/e10s/browser_caching_actions.js | 15 +++++++++++++++ accessible/windows/ia2/ia2AccessibleAction.cpp | 2 +- accessible/windows/msaa/MsaaAccessible.cpp | 5 +---- accessible/xpcom/xpcAccessible.cpp | 11 +++++------ 17 files changed, 71 insertions(+), 25 deletions(-) diff --git a/accessible/atk/nsMaiInterfaceAction.cpp b/accessible/atk/nsMaiInterfaceAction.cpp index 08436b718fa6..2feec1479097 100644 --- a/accessible/atk/nsMaiInterfaceAction.cpp +++ b/accessible/atk/nsMaiInterfaceAction.cpp @@ -18,13 +18,12 @@ using namespace mozilla::a11y; extern "C" { static gboolean doActionCB(AtkAction* aAction, gint aActionIndex) { - AccessibleWrap* accWrap = GetAccessibleWrap(ATK_OBJECT(aAction)); - if (accWrap) { - return accWrap->DoAction(aActionIndex); + AtkObject* atkObject = ATK_OBJECT(aAction); + if (Accessible* acc = GetInternalObj(atkObject)) { + return acc->DoAction(aActionIndex); } - RemoteAccessible* proxy = GetProxy(ATK_OBJECT(aAction)); - return proxy && proxy->DoAction(aActionIndex); + return false; } static gint getActionCountCB(AtkAction* aAction) { diff --git a/accessible/basetypes/Accessible.h b/accessible/basetypes/Accessible.h index 9d2e20bf4052..a537373bee2d 100644 --- a/accessible/basetypes/Accessible.h +++ b/accessible/basetypes/Accessible.h @@ -223,6 +223,11 @@ class Accessible { TranslateString(name, aDescription); } + /** + * Invoke the accessible action. + */ + virtual bool DoAction(uint8_t aIndex) const = 0; + // Type "is" methods bool IsDoc() const { return HasGenericType(eDocument); } diff --git a/accessible/generic/LocalAccessible.h b/accessible/generic/LocalAccessible.h index e401e6c7fea3..ae363e787136 100644 --- a/accessible/generic/LocalAccessible.h +++ b/accessible/generic/LocalAccessible.h @@ -508,10 +508,7 @@ class LocalAccessible : public nsISupports, public Accessible { virtual void ActionNameAt(uint8_t aIndex, nsAString& aName) override; - /** - * Invoke the accessible action. - */ - virtual bool DoAction(uint8_t aIndex) const; + virtual bool DoAction(uint8_t aIndex) const override; /** * Return access key, such as Alt+D. diff --git a/accessible/html/HTMLElementAccessibles.cpp b/accessible/html/HTMLElementAccessibles.cpp index 5b073544e652..3e58eb957f5e 100644 --- a/accessible/html/HTMLElementAccessibles.cpp +++ b/accessible/html/HTMLElementAccessibles.cpp @@ -81,10 +81,12 @@ void HTMLLabelAccessible::ActionNameAt(uint8_t aIndex, nsAString& aName) { } bool HTMLLabelAccessible::DoAction(uint8_t aIndex) const { - if (aIndex != 0) return false; + if (aIndex == 0 && ActionCount()) { + DoCommand(); + return true; + } - DoCommand(); - return true; + return false; } //////////////////////////////////////////////////////////////////////////////// diff --git a/accessible/ipc/DocAccessibleChildBase.cpp b/accessible/ipc/DocAccessibleChildBase.cpp index 003ed4f82782..b23c29798f2c 100644 --- a/accessible/ipc/DocAccessibleChildBase.cpp +++ b/accessible/ipc/DocAccessibleChildBase.cpp @@ -170,6 +170,15 @@ mozilla::ipc::IPCResult DocAccessibleChildBase::RecvVerifyCache( return IPC_OK(); } +mozilla::ipc::IPCResult DocAccessibleChildBase::RecvDoActionAsync( + const uint64_t& aID, const uint8_t& aIndex) { + if (LocalAccessible* acc = IdToAccessible(aID)) { + Unused << acc->DoAction(aIndex); + } + + return IPC_OK(); +} + LocalAccessible* DocAccessibleChildBase::IdToAccessible( const uint64_t& aID) const { if (!aID) return mDoc; diff --git a/accessible/ipc/DocAccessibleChildBase.h b/accessible/ipc/DocAccessibleChildBase.h index dcff2314bb0a..3a3890fa9591 100644 --- a/accessible/ipc/DocAccessibleChildBase.h +++ b/accessible/ipc/DocAccessibleChildBase.h @@ -63,6 +63,9 @@ class DocAccessibleChildBase : public PDocAccessibleChild { const uint64_t& aID, const uint64_t& aCacheDomain, AccAttributes* aFields) override; + virtual mozilla::ipc::IPCResult RecvDoActionAsync( + const uint64_t& aID, const uint8_t& aIndex) override; + protected: static void FlattenTree(LocalAccessible* aRoot, nsTArray& aTree); diff --git a/accessible/ipc/RemoteAccessibleBase.cpp b/accessible/ipc/RemoteAccessibleBase.cpp index 155d6bec80dd..968214715cf8 100644 --- a/accessible/ipc/RemoteAccessibleBase.cpp +++ b/accessible/ipc/RemoteAccessibleBase.cpp @@ -566,6 +566,16 @@ void RemoteAccessibleBase::ActionNameAt(uint8_t aIndex, VERIFY_CACHE(CacheDomain::Actions); } +template +bool RemoteAccessibleBase::DoAction(uint8_t aIndex) const { + if (ActionCount() < aIndex + 1) { + return false; + } + + Unused << mDoc->SendDoActionAsync(mID, aIndex); + return true; +} + template void RemoteAccessibleBase::ARIAGroupPosition( int32_t* aLevel, int32_t* aSetSize, int32_t* aPosInSet) const { diff --git a/accessible/ipc/RemoteAccessibleBase.h b/accessible/ipc/RemoteAccessibleBase.h index b2a75d8b603e..325577dd0c4c 100644 --- a/accessible/ipc/RemoteAccessibleBase.h +++ b/accessible/ipc/RemoteAccessibleBase.h @@ -188,6 +188,8 @@ class RemoteAccessibleBase : public Accessible, public HyperTextAccessibleBase { virtual void ActionNameAt(uint8_t aIndex, nsAString& aName) override; + virtual bool DoAction(uint8_t aIndex) const override; + // Methods that interact with content. virtual void TakeFocus() const override; diff --git a/accessible/ipc/RemoteAccessibleShared.h b/accessible/ipc/RemoteAccessibleShared.h index 3ffad33f0219..9f556764b647 100644 --- a/accessible/ipc/RemoteAccessibleShared.h +++ b/accessible/ipc/RemoteAccessibleShared.h @@ -216,7 +216,6 @@ bool UnselectAll(); void TakeSelection(); void SetSelected(bool aSelect); -bool DoAction(uint8_t aIndex); KeyBinding AccessKey(); KeyBinding KeyboardShortcut(); void AtkKeyBinding(nsString& aBinding); diff --git a/accessible/ipc/other/PDocAccessible.ipdl b/accessible/ipc/other/PDocAccessible.ipdl index 98788f472ee8..4b3d4e91bd61 100644 --- a/accessible/ipc/other/PDocAccessible.ipdl +++ b/accessible/ipc/other/PDocAccessible.ipdl @@ -310,6 +310,7 @@ child: async SetSelected(uint64_t aID, bool aSelected); [Nested=inside_sync] sync DoAction(uint64_t aID, uint8_t aIndex) returns(bool aSuccess); + async DoActionAsync(uint64_t aID, uint8_t aIndex); [Nested=inside_sync] sync ActionCount(uint64_t aID) returns(uint8_t aCount); [Nested=inside_sync] sync ActionNameAt(uint64_t aID, uint8_t aIndex) returns(nsString aName); [Nested=inside_sync] sync AccessKey(uint64_t aID) returns(uint32_t aKey, uint32_t aModifierMask); diff --git a/accessible/ipc/other/RemoteAccessible.cpp b/accessible/ipc/other/RemoteAccessible.cpp index a585baca452f..58c3bf308801 100644 --- a/accessible/ipc/other/RemoteAccessible.cpp +++ b/accessible/ipc/other/RemoteAccessible.cpp @@ -743,7 +743,11 @@ void RemoteAccessible::SetSelected(bool aSelect) { Unused << mDoc->SendSetSelected(mID, aSelect); } -bool RemoteAccessible::DoAction(uint8_t aIndex) { +bool RemoteAccessible::DoAction(uint8_t aIndex) const { + if (StaticPrefs::accessibility_cache_enabled_AtStartup()) { + return RemoteAccessibleBase::DoAction(aIndex); + } + bool success = false; Unused << mDoc->SendDoAction(mID, aIndex, &success); return success; diff --git a/accessible/ipc/other/RemoteAccessible.h b/accessible/ipc/other/RemoteAccessible.h index c4ad52a6326f..af8b955f6d5e 100644 --- a/accessible/ipc/other/RemoteAccessible.h +++ b/accessible/ipc/other/RemoteAccessible.h @@ -45,6 +45,8 @@ class RemoteAccessible : public RemoteAccessibleBase { virtual int32_t LinkIndexAtOffset(uint32_t aOffset) override; + virtual bool DoAction(uint8_t aIndex) const override; + virtual uint8_t ActionCount() const override; virtual void ActionNameAt(uint8_t aIndex, nsAString& aName) override; diff --git a/accessible/ipc/win/PDocAccessible.ipdl b/accessible/ipc/win/PDocAccessible.ipdl index a957e28685e7..c805561eade8 100644 --- a/accessible/ipc/win/PDocAccessible.ipdl +++ b/accessible/ipc/win/PDocAccessible.ipdl @@ -125,6 +125,8 @@ child: */ async VerifyCache(uint64_t aID, uint64_t aCacheDomain, AccAttributes aFields); + async DoActionAsync(uint64_t aID, uint8_t aIndex); + async __delete__(); }; diff --git a/accessible/tests/browser/e10s/browser_caching_actions.js b/accessible/tests/browser/e10s/browser_caching_actions.js index 869550e278f5..d0154d27a54b 100644 --- a/accessible/tests/browser/e10s/browser_caching_actions.js +++ b/accessible/tests/browser/e10s/browser_caching_actions.js @@ -123,6 +123,13 @@ addAccessibleTask( let acc = findAccessibleChildByID(docAcc, "li_clickable1"); await untilCacheIs(() => acc.actionCount, 0, "li has no actions"); + let thrown = false; + try { + acc.doAction(0); + } catch (e) { + thrown = true; + } + ok(thrown, "doAction should throw exception"); // Remove 'for' from label await invokeContentTask(browser, [], () => { @@ -130,6 +137,14 @@ addAccessibleTask( }); acc = findAccessibleChildByID(docAcc, "label1"); await untilCacheIs(() => acc.actionCount, 0, "label has no actions"); + thrown = false; + try { + acc.doAction(0); + ok(false, "doAction should throw exception"); + } catch (e) { + thrown = true; + } + ok(thrown, "doAction should throw exception"); // Add 'longdesc' to image await invokeContentTask(browser, [], () => { diff --git a/accessible/windows/ia2/ia2AccessibleAction.cpp b/accessible/windows/ia2/ia2AccessibleAction.cpp index f0e1cbdc85d8..5fe77f95ed42 100644 --- a/accessible/windows/ia2/ia2AccessibleAction.cpp +++ b/accessible/windows/ia2/ia2AccessibleAction.cpp @@ -56,7 +56,7 @@ ia2AccessibleAction::nActions(long* aActionCount) { STDMETHODIMP ia2AccessibleAction::doAction(long aActionIndex) { - AccessibleWrap* acc = LocalAcc(); + Accessible* acc = Acc(); if (!acc) return CO_E_OBJNOTCONNECTED; uint8_t index = static_cast(aActionIndex); diff --git a/accessible/windows/msaa/MsaaAccessible.cpp b/accessible/windows/msaa/MsaaAccessible.cpp index 1c28779bf89f..78e59ce43206 100644 --- a/accessible/windows/msaa/MsaaAccessible.cpp +++ b/accessible/windows/msaa/MsaaAccessible.cpp @@ -1681,11 +1681,8 @@ MsaaAccessible::accDoDefaultAction( if (accessible) { return accessible->accDoDefaultAction(kVarChildIdSelf); } - if (mAcc->IsRemote()) { - return E_NOTIMPL; // XXX Not supported for RemoteAccessible yet. - } - return LocalAcc()->DoAction(0) ? S_OK : E_INVALIDARG; + return mAcc->DoAction(0) ? S_OK : E_INVALIDARG; } STDMETHODIMP diff --git a/accessible/xpcom/xpcAccessible.cpp b/accessible/xpcom/xpcAccessible.cpp index 0d5845962e06..b4b377172cb6 100644 --- a/accessible/xpcom/xpcAccessible.cpp +++ b/accessible/xpcom/xpcAccessible.cpp @@ -732,15 +732,14 @@ NS_IMETHODIMP xpcAccessible::DoAction(uint8_t aIndex) { if (!IntlGeneric()) return NS_ERROR_FAILURE; - if (RemoteAccessible* proxy = IntlGeneric()->AsRemote()) { #if defined(XP_WIN) + if (IntlGeneric()->IsRemote() && + !StaticPrefs::accessibility_cache_enabled_AtStartup()) { return NS_ERROR_NOT_IMPLEMENTED; -#else - return proxy->DoAction(aIndex) ? NS_OK : NS_ERROR_INVALID_ARG; -#endif - } else { - return Intl()->DoAction(aIndex) ? NS_OK : NS_ERROR_INVALID_ARG; } +#endif + + return IntlGeneric()->DoAction(aIndex) ? NS_OK : NS_ERROR_INVALID_ARG; } NS_IMETHODIMP