ChromeVox: fixes reading of pop up buttons when expanded

The attached bug resulted from a regression in m98 via this change:
https://chromium-review.googlesource.com/c/chromium/src/+/3263318

which actually reveals an existing issue with
RenderAccessibilityImpl::MarkWebAXObjectDirty. It never passes the
correct ax::mojom::EventFrom field from the AXEvent in a higher call in
the callstack.

Furthermore, in ChromeVox js, we had a codepath in
RangeAutomationHandler which never checked the types of actions to allow
unconditionally.

This series of conditions caused the regression in m98.

R=akihiroota@chromium.org

Fixed: b/213036939
Test: cq
Change-Id: I443dcd251b3ca9d99b5d561eca7b2a60ba8593a2
AX-Relnotes: n/a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3413435
Reviewed-by: Akihiro Ota <akihiroota@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: danakj chromium <danakj@chromium.org>
Commit-Queue: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#965735}

chrome/browser/resources/chromeos/accessibility/chromevox/background/background_test.js

@@ -743,6 +743,9 @@ TEST_F(
     });
 
 TEST_F('ChromeVoxBackgroundTest', 'SelectOptionSelected', function() {
+  // Undoes the ChromeVoxNextE2E call setting this to true. The doDefault action
+  // should always be read.
+  DesktopAutomationHandler.announceActions = false;
   const mockFeedback = this.createMockFeedback();
   const site = `
     <p>start</p>

chrome/browser/resources/chromeos/accessibility/chromevox/background/base_automation_handler.js

@@ -101,10 +101,8 @@ BaseAutomationHandler = class {
 
     // Decide whether to announce and sync this event.
     const prevRange = ChromeVoxState.instance.getCurrentRangeWithoutRecovery();
-    if (!DesktopAutomationHandler.announceActions &&
-        (prevRange && !prevRange.requiresRecovery()) &&
-        evt.eventFrom === 'action' &&
-        !BaseAutomationHandler.allowEventFromAction_(evt.eventFromAction)) {
+    if ((prevRange && !prevRange.requiresRecovery()) &&
+        BaseAutomationHandler.disallowEventFromAction(evt)) {
       return;
     }
 
@@ -125,13 +123,15 @@ BaseAutomationHandler = class {
   }
 
   /**
-   * @param {ActionType} eventFromAction
+   * Returns true if the event contains an action that should not be processed.
+   * @param {!ChromeVoxEvent} evt
    * @return {boolean}
-   * @private
    */
-  static allowEventFromAction_(eventFromAction) {
-    return eventFromAction === ActionType.DO_DEFAULT ||
-        eventFromAction === ActionType.SHOW_CONTEXT_MENU;
+  static disallowEventFromAction(evt) {
+    return !DesktopAutomationHandler.announceActions &&
+        evt.eventFrom === 'action' &&
+        evt.eventFromAction !== ActionType.DO_DEFAULT &&
+        evt.eventFromAction !== ActionType.SHOW_CONTEXT_MENU;
   }
 };
 });  // goog.scope

chrome/browser/resources/chromeos/accessibility/chromevox/background/range_automation_handler.js

@@ -98,8 +98,7 @@ export class RangeAutomationHandler extends BaseAutomationHandler {
    * @param {!ChromeVoxEvent} evt
    */
   onEventIfInRange(evt) {
-    if (!DesktopAutomationHandler.announceActions &&
-        evt.eventFrom === 'action') {
+    if (BaseAutomationHandler.disallowEventFromAction(evt)) {
       return;
     }
 

content/renderer/accessibility/render_accessibility_impl.cc

@@ -447,11 +447,12 @@ void RenderAccessibilityImpl::Reset(int32_t reset_token) {
 void RenderAccessibilityImpl::MarkWebAXObjectDirty(
     const WebAXObject& obj,
     bool subtree,
+    ax::mojom::EventFrom event_from,
     ax::mojom::Action event_from_action,
     std::vector<ui::AXEventIntent> event_intents,
     ax::mojom::Event event_type) {
-  EnqueueDirtyObject(obj, ax::mojom::EventFrom::kAction, event_from_action,
-                     event_intents, dirty_objects_.end());
+  EnqueueDirtyObject(obj, event_from, event_from_action, event_intents,
+                     dirty_objects_.end());
 
   if (subtree)
     serializer_->InvalidateSubtree(obj);
@@ -508,8 +509,9 @@ void RenderAccessibilityImpl::HandleAXEvent(const ui::AXEvent& event) {
     event_schedule_mode_ = EventScheduleMode::kProcessEventsImmediately;
 
   if (ShouldSerializeNodeForEvent(obj, event)) {
-    MarkWebAXObjectDirty(obj, false, event.event_from_action,
-                         event.event_intents, event.event_type);
+    MarkWebAXObjectDirty(obj, /* subtree= */ false, event.event_from,
+                         event.event_from_action, event.event_intents,
+                         event.event_type);
   }
 
   ScheduleSendPendingAccessibilityEvents();
@@ -1306,7 +1308,8 @@ void RenderAccessibilityImpl::MarkAllAXObjectsDirty(
     objs_to_explore.pop();
 
     if (obj.Role() == role)
-      MarkWebAXObjectDirty(obj, /* subtree */ false, event_from_action);
+      MarkWebAXObjectDirty(obj, /* subtree */ false,
+                           ax::mojom::EventFrom::kNone, event_from_action);
 
     std::vector<blink::WebAXObject> children;
     tree_source_->GetChildren(obj, &children);

content/renderer/accessibility/render_accessibility_impl.h

@@ -127,6 +127,7 @@ class CONTENT_EXPORT RenderAccessibilityImpl : public RenderAccessibility,
   void MarkWebAXObjectDirty(
       const blink::WebAXObject& obj,
       bool subtree,
+      ax::mojom::EventFrom event_from = ax::mojom::EventFrom::kNone,
       ax::mojom::Action event_from_action = ax::mojom::Action::kNone,
       std::vector<ui::AXEventIntent> event_intents = {},
       ax::mojom::Event event_type = ax::mojom::Event::kNone);

content/renderer/render_frame_impl.cc

@@ -4558,12 +4558,13 @@ void RenderFrameImpl::PostAccessibilityEvent(const ui::AXEvent& event) {
 void RenderFrameImpl::MarkWebAXObjectDirty(
     const blink::WebAXObject& obj,
     bool subtree,
+    ax::mojom::EventFrom event_from,
     ax::mojom::Action event_from_action) {
   if (!IsAccessibilityEnabled())
     return;
 
   render_accessibility_manager_->GetRenderAccessibilityImpl()
-      ->MarkWebAXObjectDirty(obj, subtree, event_from_action);
+      ->MarkWebAXObjectDirty(obj, subtree, event_from, event_from_action);
 }
 
 void RenderFrameImpl::AddObserver(RenderFrameObserver* observer) {

content/renderer/render_frame_impl.h

@@ -612,10 +612,10 @@ class CONTENT_EXPORT RenderFrameImpl
   bool AllowContentInitiatedDataUrlNavigations(
       const blink::WebURL& url) override;
   void PostAccessibilityEvent(const ui::AXEvent& event) override;
-  void MarkWebAXObjectDirty(
-      const blink::WebAXObject& obj,
-      bool subtree,
-      ax::mojom::Action event_from_action = ax::mojom::Action::kNone) override;
+  void MarkWebAXObjectDirty(const blink::WebAXObject& obj,
+                            bool subtree,
+                            ax::mojom::EventFrom event_from,
+                            ax::mojom::Action event_from_action) override;
   void CheckIfAudioSinkExistsAndIsAuthorized(
       const blink::WebString& sink_id,
       blink::WebSetSinkIdCompleteCallback callback) override;

content/web_test/renderer/web_frame_test_proxy.cc

@@ -640,6 +640,7 @@ void WebFrameTestProxy::PostAccessibilityEvent(const ui::AXEvent& event) {
 void WebFrameTestProxy::MarkWebAXObjectDirty(
     const blink::WebAXObject& object,
     bool subtree,
+    ax::mojom::EventFrom event_from,
     ax::mojom::Action event_from_action) {
   HandleWebAccessibilityEvent(object, "MarkDirty",
                               std::vector<ui::AXEventIntent>());
@@ -650,7 +651,8 @@ void WebFrameTestProxy::MarkWebAXObjectDirty(
   if (object.IsDetached())
     return;  // |this| is invalid.
 
-  RenderFrameImpl::MarkWebAXObjectDirty(object, subtree, event_from_action);
+  RenderFrameImpl::MarkWebAXObjectDirty(object, subtree, event_from,
+                                        event_from_action);
 }
 
 void WebFrameTestProxy::HandleWebAccessibilityEvent(

content/web_test/renderer/web_frame_test_proxy.h

@@ -79,10 +79,10 @@ class WebFrameTestProxy : public RenderFrameImpl,
                        ForRedirect for_redirect) override;
   void BeginNavigation(std::unique_ptr<blink::WebNavigationInfo> info) override;
   void PostAccessibilityEvent(const ui::AXEvent& event) override;
-  void MarkWebAXObjectDirty(
-      const blink::WebAXObject& object,
-      bool subtree,
-      ax::mojom::Action event_from_action = ax::mojom::Action::kNone) override;
+  void MarkWebAXObjectDirty(const blink::WebAXObject& object,
+                            bool subtree,
+                            ax::mojom::EventFrom event_from,
+                            ax::mojom::Action event_from_action) override;
   void CheckIfAudioSinkExistsAndIsAuthorized(
       const blink::WebString& sink_id,
       blink::WebSetSinkIdCompleteCallback completion_callback) override;

third_party/blink/public/web/web_local_frame_client.h

@@ -653,10 +653,13 @@ class BLINK_EXPORT WebLocalFrameClient {
   // Notifies the embedder that a WebAXObject is dirty and its state needs
   // to be serialized again. If |subtree| is true, the entire subtree is
   // dirty.
-  virtual void MarkWebAXObjectDirty(
-      const WebAXObject&,
-      bool subtree,
-      ax::mojom::Action event_from_action = ax::mojom::Action::kNone) {}
+  // |event_from| and |event_from_action| annotate this node change with info
+  // about the event which caused the change. For example, an event from a user
+  // or an event from a focus action.
+  virtual void MarkWebAXObjectDirty(const WebAXObject&,
+                                    bool subtree,
+                                    ax::mojom::EventFrom event_from,
+                                    ax::mojom::Action event_from_action) {}
 
   // Audio Output Devices API --------------------------------------------
 

third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc

@@ -3410,8 +3410,12 @@ void AXObjectCacheImpl::MarkAXObjectDirtyWithCleanLayoutHelper(AXObject* obj,
 
   WebLocalFrameImpl* webframe = WebLocalFrameImpl::FromFrame(
       obj->GetDocument()->AXObjectCacheOwner().GetFrame());
-  if (webframe && webframe->Client())
-    webframe->Client()->MarkWebAXObjectDirty(WebAXObject(obj), subtree);
+  if (webframe && webframe->Client()) {
+    webframe->Client()->MarkWebAXObjectDirty(WebAXObject(obj), subtree,
+                                             active_event_from_,
+                                             active_event_from_action_);
+  }
+
   obj->UpdateCachedAttributeValuesIfNeeded(true);
   for (auto agent : agents_)
     agent->AXObjectModified(obj, subtree);