diff options
| -rw-r--r-- | chrome/browser/accessibility/browser_accessibility.cc | 1 | ||||
| -rw-r--r-- | chrome/browser/accessibility/browser_accessibility_manager.cc | 6 | ||||
| -rw-r--r-- | chrome/browser/accessibility/renderer_accessibility_browsertest.cc | 31 | ||||
| -rw-r--r-- | webkit/glue/webaccessibility.cc | 12 | ||||
| -rw-r--r-- | webkit/glue/webaccessibility.h | 9 |
5 files changed, 50 insertions, 9 deletions
diff --git a/chrome/browser/accessibility/browser_accessibility.cc b/chrome/browser/accessibility/browser_accessibility.cc index 126b7a5..03a51e8 100644 --- a/chrome/browser/accessibility/browser_accessibility.cc +++ b/chrome/browser/accessibility/browser_accessibility.cc @@ -78,6 +78,7 @@ void BrowserAccessibility::DetachTree( for (size_t i = 0; i < children_.size(); i++) children_[i]->DetachTree(nodes); children_.clear(); + parent_ = NULL; } void BrowserAccessibility::UpdateParent(BrowserAccessibility* parent, diff --git a/chrome/browser/accessibility/browser_accessibility_manager.cc b/chrome/browser/accessibility/browser_accessibility_manager.cc index e3e2545..daba26f 100644 --- a/chrome/browser/accessibility/browser_accessibility_manager.cc +++ b/chrome/browser/accessibility/browser_accessibility_manager.cc @@ -325,6 +325,12 @@ BrowserAccessibility* BrowserAccessibilityManager::CreateAccessibilityTree( instance = NULL; if (instance) { + // If we're reusing a node, it should already be detached from a parent + // and any children. If not, that means we have a serious bug somewhere, + // like the same child is reachable from two places in the same tree. + DCHECK_EQ(static_cast<BrowserAccessibility*>(NULL), instance->parent()); + DCHECK_EQ(0U, instance->child_count()); + // If we're reusing a node, update its parent and increment its // reference count. instance->UpdateParent(parent, index_in_parent); diff --git a/chrome/browser/accessibility/renderer_accessibility_browsertest.cc b/chrome/browser/accessibility/renderer_accessibility_browsertest.cc index 3547969..de91a40 100644 --- a/chrome/browser/accessibility/renderer_accessibility_browsertest.cc +++ b/chrome/browser/accessibility/renderer_accessibility_browsertest.cc @@ -44,6 +44,15 @@ class RendererAccessibilityBrowserTest : public InProcessBrowserTest { return view_host->accessibility_tree(); } + // Make sure each node in the tree has an unique id. + void RecursiveAssertUniqueIds( + const WebAccessibility& node, base::hash_set<int>* ids) { + ASSERT_TRUE(ids->find(node.id) == ids->end()); + ids->insert(node.id); + for (size_t i = 0; i < node.children.size(); i++) + RecursiveAssertUniqueIds(node.children[i], ids); + } + // InProcessBrowserTest void SetUpInProcessBrowserTestFixture(); void TearDownInProcessBrowserTestFixture(); @@ -237,4 +246,26 @@ IN_PROC_BROWSER_TEST_F(RendererAccessibilityBrowserTest, EXPECT_EQ(cell2.id, column2.indirect_child_ids[0]); } +IN_PROC_BROWSER_TEST_F(RendererAccessibilityBrowserTest, + CrossPlatformMultipleInheritanceAccessibility2) { + // Here's another html snippet where WebKit puts the same node as a child + // of two different parents. Instead of checking the exact output, just + // make sure that no id is reused in the resulting tree. + const char url_str[] = + "data:text/html," + "<!doctype html>" + "<script>\n" + " document.writeln('<q><section></section></q><q><li>');\n" + " setTimeout(function() {\n" + " document.close();\n" + " }, 1);\n" + "</script>"; + GURL url(url_str); + browser()->OpenURL(url, GURL(), CURRENT_TAB, PageTransition::TYPED); + + const WebAccessibility& tree = GetWebAccessibilityTree(); + base::hash_set<int> ids; + RecursiveAssertUniqueIds(tree, &ids); +} + } // namespace diff --git a/webkit/glue/webaccessibility.cc b/webkit/glue/webaccessibility.cc index 48cc339..09a9212 100644 --- a/webkit/glue/webaccessibility.cc +++ b/webkit/glue/webaccessibility.cc @@ -395,10 +395,10 @@ void WebAccessibility::Init(const WebKit::WebAccessibilityObject& src, // Some nodes appear in the tree in more than one place: for example, // a cell in a table appears as a child of both a row and a column. - // Only recursively add child nodes that have this node as one of its - // ancestors. For child nodes that are actually parented to something - // else, store only the ID. - if (IsAncestorOf(src, child)) { + // Only recursively add child nodes that have this node as its + // unignored parent. For child nodes that are actually parented to + // somethinng else, store only the ID. + if (IsParentUnignoredOf(src, child)) { children.push_back(WebAccessibility(child, cache, include_children)); } else { indirect_child_ids.push_back(cache->addOrGetId(child)); @@ -407,11 +407,11 @@ void WebAccessibility::Init(const WebKit::WebAccessibilityObject& src, } } -bool WebAccessibility::IsAncestorOf( +bool WebAccessibility::IsParentUnignoredOf( const WebKit::WebAccessibilityObject& ancestor, const WebKit::WebAccessibilityObject& child) { WebKit::WebAccessibilityObject parent = child.parentObject(); - while (!parent.isNull() && !parent.equals(ancestor)) + while (!parent.isNull() && parent.accessibilityIsIgnored()) parent = parent.parentObject(); return parent.equals(ancestor); } diff --git a/webkit/glue/webaccessibility.h b/webkit/glue/webaccessibility.h index b67ce83..85fe6d2 100644 --- a/webkit/glue/webaccessibility.h +++ b/webkit/glue/webaccessibility.h @@ -198,9 +198,12 @@ struct WebAccessibility { WebKit::WebAccessibilityCache* cache, bool include_children); - // Returns true if |ancestor| is an ancestor of |child|. - bool IsAncestorOf(const WebKit::WebAccessibilityObject& ancestor, - const WebKit::WebAccessibilityObject& child); + // Returns true if |ancestor| is the first unignored parent of |child|, + // which means that when walking up the parent chain from |child|, + // |ancestor| is the *first* ancestor that isn't marked as + // accessibilityIsIgnored(). + bool IsParentUnignoredOf(const WebKit::WebAccessibilityObject& ancestor, + const WebKit::WebAccessibilityObject& child); public: // This is a simple serializable struct. All member variables should be |