Correctly update the bounds of objects in the accessibility tree.

content/renderer/accessibility/renderer_accessibility_complete.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/accessibility/renderer_accessibility_complete.h"
 
 #include <queue>
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 153 matching lines @@
   // Make a copy of the events, because it's possible that
   // actions inside this loop will cause more events to be
   // queued up.
   std::vector<AccessibilityHostMsg_EventParams> src_events =
       pending_events_;
   pending_events_.clear();
 
   // Generate an event message from each Blink event.
   std::vector<AccessibilityHostMsg_EventParams> event_msgs;
 
+  // If there's a layout complete message, we need to send location changes.
+  bool had_layout_complete_messages = false;

[aboxhall, 2014/07/19 15:08:05]

Why is this plural?

[dmazzoni, 2014/07/20 04:24:17]

There could be more than one "layout complete" in |src_events|.

[aboxhall, 2014/07/21 01:33:47]

Obviously not a deal-breaker, but to me had_layout_complete_message meaning "at
least one" makes more sense than "had_layout_complete_messages" meaning "one or
more". Totally up to you though!

+
   // Loop over each event and generate an updated event message.
   for (size_t i = 0; i < src_events.size(); ++i) {
-    AccessibilityHostMsg_EventParams& event =
-        src_events[i];
+    AccessibilityHostMsg_EventParams& event = src_events[i];
+    if (event.event_type == ui::AX_EVENT_LAYOUT_COMPLETE)
+      had_layout_complete_messages = true;
 
-    WebAXObject obj = document.accessibilityObjectFromID(
-        event.id);
+    WebAXObject obj = document.accessibilityObjectFromID(event.id);
+
     // Make sure the object still exists.
     if (!obj.updateBackingStoreAndCheckValidity())
       continue;
+
     // Make sure it's a descendant of our root node - exceptions include the
     // scroll area that's the parent of the main document (we ignore it), and
     // possibly nodes attached to a different document.
     if (!tree_source_.IsInTree(obj))
       continue;
 
     // When we get a "selected children changed" event, Blink
     // doesn't also send us events for each child that changed
     // selection state, so make sure we re-send that whole subtree.
     if (event.event_type ==
         ui::AX_EVENT_SELECTED_CHILDREN_CHANGED) {
       serializer_.DeleteClientSubtree(obj);
     }
 
     AccessibilityHostMsg_EventParams event_msg;
     event_msg.event_type = event.event_type;
     event_msg.id = event.id;
     serializer_.SerializeChanges(obj, &event_msg.update);
     event_msgs.push_back(event_msg);
 
+    // For each node in the update, set the location in our map from
+    // ids to locations.
+    for (size_t i = 0; i < event_msg.update.nodes.size(); ++i) {
+      locations_[event_msg.update.nodes[i].id] =
+          event_msg.update.nodes[i].location;
+    }
+
     VLOG(0) << "Accessibility event: " << ui::ToString(event.event_type)
             << " on node id " << event_msg.id
             << "\n" << event_msg.update.ToString();
   }
 
   Send(new AccessibilityHostMsg_Events(routing_id(), event_msgs));
 
-  SendLocationChanges();
+  if (had_layout_complete_messages)
+    SendLocationChanges();
 }
 
 void RendererAccessibilityComplete::SendLocationChanges() {
   std::vector<AccessibilityHostMsg_LocationChangeParams> messages;
 
   // Do a breadth-first explore of the whole blink AX tree.
   base::hash_map<int, gfx::Rect> new_locations;
   std::queue<WebAXObject> objs_to_explore;
   objs_to_explore.push(tree_source_.GetRoot());
   while (objs_to_explore.size()) {
@@ skipping 11 matching lines @@
     gfx::Rect new_location = obj.boundingBoxRect();
     if (iter != locations_.end() && iter->second != new_location) {
       AccessibilityHostMsg_LocationChangeParams message;
       message.id = id;
       message.new_location = new_location;
       messages.push_back(message);
     }
 
     // Save the new location.
     new_locations[id] = new_location;
+
+    // Explore children of this object.

[aboxhall, 2014/07/19 15:08:05]

Oops! :)

[dmazzoni, 2014/07/20 04:24:17]

Yup! Accidentally lost when refactoring, easy fix, the hard part was coming up
with a good test so that this wouldn't regress in the future.

+    std::vector<blink::WebAXObject> children;
+    tree_source_.GetChildren(obj, &children);
+    for (size_t i = 0; i < children.size(); ++i)
+      objs_to_explore.push(children[i]);
   }
   locations_.swap(new_locations);
 
   Send(new AccessibilityHostMsg_LocationChanges(routing_id(), messages));
 }
 
 void RendererAccessibilityComplete::OnDoDefaultAction(int acc_obj_id) {
   const WebDocument& document = GetMainDocument();
   if (document.isNull())
     return;
@@ skipping 131 matching lines @@
     render_frame_->GetRenderView()->GetWebView()->clearFocusedElement();
   else
     obj.setFocused(true);
 }
 
 void RendererAccessibilityComplete::OnFatalError() {
   CHECK(false) << "Invalid accessibility tree.";
 }
 
 }  // namespace content