diff options
| author | dmazzoni@chromium.org <dmazzoni@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-13 02:43:16 +0000 |
|---|---|---|
| committer | dmazzoni@chromium.org <dmazzoni@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-13 02:43:16 +0000 |
| commit | 072dbfba5989b516352a7a5e96e5064c00f535dd (patch) | |
| tree | c8424cee0f5fc296eb945e1e77a82fab4306af2c /content | |
| parent | a9d9418aeb531f5a900b4db14696d7620987453a (diff) | |
| download | chromium_src-072dbfba5989b516352a7a5e96e5064c00f535dd.zip chromium_src-072dbfba5989b516352a7a5e96e5064c00f535dd.tar.gz chromium_src-072dbfba5989b516352a7a5e96e5064c00f535dd.tar.bz2 | |
Keep track of accessibility ids serialized to avoid duplicates/loops.
This fixes a top renderer crash, see bug for details.
BUG=290233
Review URL: https://chromiumcodereview.appspot.com/23637013
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222969 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
3 files changed, 85 insertions, 5 deletions
diff --git a/content/renderer/accessibility/renderer_accessibility_browsertest.cc b/content/renderer/accessibility/renderer_accessibility_browsertest.cc index cf2297b..7f91243 100644 --- a/content/renderer/accessibility/renderer_accessibility_browsertest.cc +++ b/content/renderer/accessibility/renderer_accessibility_browsertest.cc @@ -465,4 +465,73 @@ TEST_F(RendererAccessibilityTest, ShowAccessibilityObject) { EXPECT_EQ(3, CountAccessibilityNodesSentToBrowser()); } +TEST_F(RendererAccessibilityTest, DetachAccessibilityObject) { + // Test RendererAccessibilityComplete and make sure it sends the + // proper event to the browser when an object in the tree + // is detached, but its children are not. This can happen when + // a layout occurs and an anonymous render block is no longer needed. + std::string html = + "<body aria-label='Body'>" + "<span>1</span><span style='display:block'>2</span>" + "</body>"; + LoadHTML(html.c_str()); + + scoped_ptr<TestRendererAccessibilityComplete> accessibility( + new TestRendererAccessibilityComplete(view())); + accessibility->SendPendingAccessibilityEvents(); + EXPECT_EQ(5, accessibility->browser_tree_node_count()); + EXPECT_EQ(5, CountAccessibilityNodesSentToBrowser()); + + // Initially, the accessibility tree looks like this: + // + // Document + // +--Body + // +--Anonymous Block + // +--Static Text "1" + // +--Static Text "2" + WebDocument document = view()->GetWebView()->mainFrame()->document(); + WebAXObject root_obj = document.accessibilityObject(); + WebAXObject body = root_obj.childAt(0); + WebAXObject anonymous_block = body.childAt(0); + WebAXObject text_1 = anonymous_block.childAt(0); + WebAXObject text_2 = body.childAt(1); + + // Change the display of the second 'span' back to inline, which causes the + // anonymous block to be destroyed. + ExecuteJavaScript( + "document.querySelectorAll('span')[1].style.display = 'inline';"); + // Force layout now. + ExecuteJavaScript("document.body.offsetLeft;"); + + // Send a childrenChanged on the body. + sink_->ClearMessages(); + accessibility->HandleWebAccessibilityEvent( + body, + WebKit::WebAXEventChildrenChanged); + + accessibility->SendPendingAccessibilityEvents(); + + // Afterwards, the accessibility tree looks like this: + // + // Document + // +--Body + // +--Static Text "1" + // +--Static Text "2" + // + // We just assert that there are now four nodes in the + // accessibility tree and that only three nodes needed + // to be updated (the body, the static text 1, and + // the static text 2). + EXPECT_EQ(4, accessibility->browser_tree_node_count()); + + AccessibilityHostMsg_EventParams event; + GetLastAccEvent(&event); + ASSERT_EQ(3U, event.nodes.size()); + + EXPECT_EQ(body.axID(), event.nodes[0].id); + EXPECT_EQ(text_1.axID(), event.nodes[1].id); + // The third event is to update text_2, but its id changes + // so we don't have a test expectation for it. +} + } // namespace content diff --git a/content/renderer/accessibility/renderer_accessibility_complete.cc b/content/renderer/accessibility/renderer_accessibility_complete.cc index 10ca2fc..d70a00a 100644 --- a/content/renderer/accessibility/renderer_accessibility_complete.cc +++ b/content/renderer/accessibility/renderer_accessibility_complete.cc @@ -258,7 +258,8 @@ void RendererAccessibilityComplete::SendPendingAccessibilityEvents() { AccessibilityHostMsg_EventParams event_msg; event_msg.event_type = event.event_type; event_msg.id = event.id; - SerializeChangedNodes(obj, &event_msg.nodes); + std::set<int> ids_serialized; + SerializeChangedNodes(obj, &event_msg.nodes, &ids_serialized); event_msgs.push_back(event_msg); #ifndef NDEBUG @@ -328,7 +329,12 @@ RendererAccessibilityComplete::CreateBrowserTreeNode() { void RendererAccessibilityComplete::SerializeChangedNodes( const WebKit::WebAXObject& obj, - std::vector<AccessibilityNodeData>* dst) { + std::vector<AccessibilityNodeData>* dst, + std::set<int>* ids_serialized) { + if (ids_serialized->find(obj.axID()) != ids_serialized->end()) + return; + ids_serialized->insert(obj.axID()); + // This method has three responsibilities: // 1. Serialize |obj| into an AccessibilityNodeData, and append it to // the end of the |dst| vector to be send to the browser process. @@ -383,6 +389,7 @@ void RendererAccessibilityComplete::SerializeChangedNodes( WebAXObject parent_obj; while (parent) { parent_obj = document.accessibilityObjectFromID(parent->id); + if (!parent_obj.isDetached()) break; parent = parent->parent; @@ -392,7 +399,7 @@ void RendererAccessibilityComplete::SerializeChangedNodes( // so that the update that clears |child| from its old parent // occurs stricly before the update that adds |child| to its // new parent. - SerializeChangedNodes(parent_obj, dst); + SerializeChangedNodes(parent_obj, dst, ids_serialized); } } } @@ -465,7 +472,7 @@ void RendererAccessibilityComplete::SerializeChangedNodes( // Serialize all of the new children, recursively. for (size_t i = 0; i < children_to_serialize.size(); ++i) - SerializeChangedNodes(children_to_serialize[i], dst); + SerializeChangedNodes(children_to_serialize[i], dst, ids_serialized); } void RendererAccessibilityComplete::ClearBrowserTreeNode( diff --git a/content/renderer/accessibility/renderer_accessibility_complete.h b/content/renderer/accessibility/renderer_accessibility_complete.h index dd3111ff..174062a 100644 --- a/content/renderer/accessibility/renderer_accessibility_complete.h +++ b/content/renderer/accessibility/renderer_accessibility_complete.h @@ -5,6 +5,7 @@ #ifndef CONTENT_RENDERER_ACCESSIBILITY_RENDERER_ACCESSIBILITY_COMPLETE_H_ #define CONTENT_RENDERER_ACCESSIBILITY_RENDERER_ACCESSIBILITY_COMPLETE_H_ +#include <set> #include <vector> #include "base/containers/hash_tables.h" @@ -75,8 +76,11 @@ class CONTENT_EXPORT RendererAccessibilityComplete // Serialize the given accessibility object |obj| and append it to // |dst|, and then recursively also serialize any *new* children of // |obj|, based on what object ids we know the browser already has. + // The set of ids serialized is added to |ids_serialized|, and any + // ids previously in that set are not serialized again. void SerializeChangedNodes(const WebKit::WebAXObject& obj, - std::vector<AccessibilityNodeData>* dst); + std::vector<AccessibilityNodeData>* dst, + std::set<int>* ids_serialized); // Clear the given node and recursively delete all of its descendants // from the browser tree. (Does not delete |browser_node|). |