- Revision
- 142883
- Author
- ma...@webkit.org
- Date
- 2013-02-14 09:15:52 -0800 (Thu, 14 Feb 2013)
Log Message
[GTK] Missing call to g_object_ref while retrieving accessible table cells
https://bugs.webkit.org/show_bug.cgi?id=106903
Reviewed by Martin Robinson.
Source/WebCore:
Add missing extra ref to implementation of atk_table_ref_at().
Test: accessibility/table-cell-for-column-and-row-crash.html
* accessibility/atk/WebKitAccessibleInterfaceTable.cpp:
(webkitAccessibleTableRefAt): This method transfers full ownership
over the returned AtkObject, so an extra reference is needed here.
Tools:
Both DRT and WKTR need to call g_object_unref() now that an extra
reference is added in the implementation of atk_table_ref_at().
* DumpRenderTree/atk/AccessibilityUIElementGtk.cpp:
(AccessibilityUIElement::cellForColumnAndRow): Call g_object_unref
before returning the new instance of AccessibilityUIElement.
* WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:
(WTR::AccessibilityUIElement::cellForColumnAndRow): Ditto.
LayoutTests:
Added new test. It should work fine at least in Mac and GTK ports,
but will need specific results for chromium and windows.
* accessibility/table-cell-for-column-and-row-crash.html: Added.
* accessibility/table-cell-for-column-and-row-crash-expected.txt: Added.
* platform/chromium/TestExpectations: Skipped test.
* platform/win/TestExpectations: Ditto.
* platform/wincairo/TestExpectations: Ditto.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (142882 => 142883)
--- trunk/LayoutTests/ChangeLog 2013-02-14 17:11:13 UTC (rev 142882)
+++ trunk/LayoutTests/ChangeLog 2013-02-14 17:15:52 UTC (rev 142883)
@@ -1,3 +1,19 @@
+2013-02-14 Mario Sanchez Prada <mario.pr...@samsung.com>
+
+ [GTK] Missing call to g_object_ref while retrieving accessible table cells
+ https://bugs.webkit.org/show_bug.cgi?id=106903
+
+ Reviewed by Martin Robinson.
+
+ Added new test. It should work fine at least in Mac and GTK ports,
+ but will need specific results for chromium and windows.
+
+ * accessibility/table-cell-for-column-and-row-crash.html: Added.
+ * accessibility/table-cell-for-column-and-row-crash-expected.txt: Added.
+ * platform/chromium/TestExpectations: Skipped test.
+ * platform/win/TestExpectations: Ditto.
+ * platform/wincairo/TestExpectations: Ditto.
+
2013-02-14 Ádám Kallai <ka...@inf.u-szeged.hu>
[Qt] Reviewing TestExpectations. Added platform specific expected files and unskip them.
Added: trunk/LayoutTests/accessibility/table-cell-for-column-and-row-crash-expected.txt (0 => 142883)
--- trunk/LayoutTests/accessibility/table-cell-for-column-and-row-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/table-cell-for-column-and-row-crash-expected.txt 2013-02-14 17:15:52 UTC (rev 142883)
@@ -0,0 +1,22 @@
+foo
+bar
+This tests that retrieving a cell for a table multiple times doesn't crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS axTable.role is 'AXRole: AXTable'
+PASS axCell.role is 'AXRole: AXCell'
+PASS axCell.role is 'AXRole: AXCell'
+PASS axCell.role is 'AXRole: AXCell'
+PASS axCell.role is 'AXRole: AXCell'
+PASS axCell.role is 'AXRole: AXCell'
+PASS axCell.role is 'AXRole: AXCell'
+PASS axCell.role is 'AXRole: AXCell'
+PASS axCell.role is 'AXRole: AXCell'
+PASS axCell.role is 'AXRole: AXCell'
+PASS axCell.role is 'AXRole: AXCell'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/accessibility/table-cell-for-column-and-row-crash.html (0 => 142883)
--- trunk/LayoutTests/accessibility/table-cell-for-column-and-row-crash.html (rev 0)
+++ trunk/LayoutTests/accessibility/table-cell-for-column-and-row-crash.html 2013-02-14 17:15:52 UTC (rev 142883)
@@ -0,0 +1,45 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+
+<body id="body">
+<table id="testTable" summary="A summary to make sure this is always exposed as an AXTable">
+ <tr><td>foo</td></tr>
+ <tr><td>bar</td></tr>
+</table>
+
+<p id="description"></p>
+<div id="console"></div>
+<script>
+description("This tests that retrieving a cell for a table multiple times doesn't crash.");
+
+if (window.testRunner) {
+ testRunner.dumpAsText();
+
+ if (window.accessibilityController) {
+ document.getElementById("body").focus();
+ var axBody = accessibilityController.focusedElement;
+
+ var axTable = axBody.childAtIndex(0);
+ shouldBe("axTable.role", "'AXRole: AXTable'");
+
+ // Trying to reference the same cell for the table
+ // multiple times shouldn't result in a crash.
+ for (var i = 0; i < 10; i++) {
+ var axCell = axTable.cellForColumnAndRow(0, 0);
+ shouldBe("axCell.role", "'AXRole: AXCell'");
+ axCell = null;
+
+ // We need to force a call to the Garbage Collector here so we are
+ // sure that axCell will get actually destroyed after using it.
+ gc();
+ }
+ }
+}
+</script>
+
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/platform/chromium/TestExpectations (142882 => 142883)
--- trunk/LayoutTests/platform/chromium/TestExpectations 2013-02-14 17:11:13 UTC (rev 142882)
+++ trunk/LayoutTests/platform/chromium/TestExpectations 2013-02-14 17:15:52 UTC (rev 142883)
@@ -1414,6 +1414,7 @@
crbug.com/10322 accessibility/plugin.html [ Skip ]
crbug.com/10322 accessibility/radio-button-group-members.html [ Skip ]
crbug.com/10322 accessibility/table-attributes.html [ Skip ]
+crbug.com/10322 accessibility/table-cell-for-column-and-row-crash.html [ Skip ]
crbug.com/10322 accessibility/table-cell-spans.html [ Skip ]
crbug.com/10322 accessibility/table-cells.html [ Skip ]
crbug.com/10322 accessibility/table-one-cell.html [ Skip ]
Modified: trunk/LayoutTests/platform/win/TestExpectations (142882 => 142883)
--- trunk/LayoutTests/platform/win/TestExpectations 2013-02-14 17:11:13 UTC (rev 142882)
+++ trunk/LayoutTests/platform/win/TestExpectations 2013-02-14 17:15:52 UTC (rev 142883)
@@ -650,6 +650,7 @@
accessibility/spinbutton-value.html
accessibility/svg-remote-element.html
accessibility/table-attributes.html
+accessibility/table-cell-for-column-and-row-crash.html
accessibility/table-cell-spans.html
accessibility/table-cells.html
accessibility/table-detection.html
Modified: trunk/LayoutTests/platform/wincairo/TestExpectations (142882 => 142883)
--- trunk/LayoutTests/platform/wincairo/TestExpectations 2013-02-14 17:11:13 UTC (rev 142882)
+++ trunk/LayoutTests/platform/wincairo/TestExpectations 2013-02-14 17:15:52 UTC (rev 142883)
@@ -1168,6 +1168,7 @@
accessibility/svg-remote-element.html
accessibility/table-attributes.html
accessibility/table-cell-spans.html
+accessibility/table-cell-for-column-and-row-crash.html
accessibility/table-cells.html
accessibility/table-detection.html
accessibility/table-modification-crash.html
Modified: trunk/Source/WebCore/ChangeLog (142882 => 142883)
--- trunk/Source/WebCore/ChangeLog 2013-02-14 17:11:13 UTC (rev 142882)
+++ trunk/Source/WebCore/ChangeLog 2013-02-14 17:15:52 UTC (rev 142883)
@@ -1,3 +1,18 @@
+2013-02-14 Mario Sanchez Prada <mario.pr...@samsung.com>
+
+ [GTK] Missing call to g_object_ref while retrieving accessible table cells
+ https://bugs.webkit.org/show_bug.cgi?id=106903
+
+ Reviewed by Martin Robinson.
+
+ Add missing extra ref to implementation of atk_table_ref_at().
+
+ Test: accessibility/table-cell-for-column-and-row-crash.html
+
+ * accessibility/atk/WebKitAccessibleInterfaceTable.cpp:
+ (webkitAccessibleTableRefAt): This method transfers full ownership
+ over the returned AtkObject, so an extra reference is needed here.
+
2013-02-14 Mike Fenton <mifen...@rim.com>
[BlackBerry] Update keyboard event details to match platform details.
Modified: trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceTable.cpp (142882 => 142883)
--- trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceTable.cpp 2013-02-14 17:11:13 UTC (rev 142882)
+++ trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceTable.cpp 2013-02-14 17:15:52 UTC (rev 142883)
@@ -94,7 +94,14 @@
AccessibilityTableCell* axCell = cell(table, row, column);
if (!axCell)
return 0;
- return axCell->wrapper();
+
+ AtkObject* cell = axCell->wrapper();
+ if (!cell)
+ return 0;
+
+ // This method transfers full ownership over the returned
+ // AtkObject, so an extra reference is needed here.
+ return ATK_OBJECT(g_object_ref(cell));
}
static gint webkitAccessibleTableGetIndexAt(AtkTable* table, gint row, gint column)
Modified: trunk/Tools/ChangeLog (142882 => 142883)
--- trunk/Tools/ChangeLog 2013-02-14 17:11:13 UTC (rev 142882)
+++ trunk/Tools/ChangeLog 2013-02-14 17:15:52 UTC (rev 142883)
@@ -1,3 +1,19 @@
+2013-02-14 Mario Sanchez Prada <mario.pr...@samsung.com>
+
+ [GTK] Missing call to g_object_ref while retrieving accessible table cells
+ https://bugs.webkit.org/show_bug.cgi?id=106903
+
+ Reviewed by Martin Robinson.
+
+ Both DRT and WKTR need to call g_object_unref() now that an extra
+ reference is added in the implementation of atk_table_ref_at().
+
+ * DumpRenderTree/atk/AccessibilityUIElementGtk.cpp:
+ (AccessibilityUIElement::cellForColumnAndRow): Call g_object_unref
+ before returning the new instance of AccessibilityUIElement.
+ * WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:
+ (WTR::AccessibilityUIElement::cellForColumnAndRow): Ditto.
+
2013-02-14 Sheriff Bot <webkit.review....@gmail.com>
Unreviewed, rolling out r142841.
Modified: trunk/Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp (142882 => 142883)
--- trunk/Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp 2013-02-14 17:11:13 UTC (rev 142882)
+++ trunk/Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp 2013-02-14 17:15:52 UTC (rev 142883)
@@ -774,8 +774,10 @@
ASSERT(ATK_IS_TABLE(m_element));
- AtkObject* foundCell = atk_table_ref_at(ATK_TABLE(m_element), row, column);
- return foundCell ? AccessibilityUIElement(foundCell) : 0;
+ // Adopt the AtkObject representing the cell because
+ // at_table_ref_at() transfers full ownership.
+ GRefPtr<AtkObject> foundCell = adoptGRef(atk_table_ref_at(ATK_TABLE(m_element), row, column));
+ return foundCell ? AccessibilityUIElement(foundCell.get()) : 0;
}
JSStringRef AccessibilityUIElement::selectedTextRange()
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp (142882 => 142883)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp 2013-02-14 17:11:13 UTC (rev 142882)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp 2013-02-14 17:15:52 UTC (rev 142883)
@@ -871,8 +871,10 @@
if (!m_element || !ATK_IS_TABLE(m_element))
return 0;
- AtkObject* foundCell = atk_table_ref_at(ATK_TABLE(m_element), row, col);
- return foundCell ? AccessibilityUIElement::create(foundCell) : 0;
+ // Adopt the AtkObject representing the cell because
+ // at_table_ref_at() transfers full ownership.
+ GRefPtr<AtkObject> foundCell = adoptGRef(atk_table_ref_at(ATK_TABLE(m_element), row, col));
+ return foundCell ? AccessibilityUIElement::create(foundCell.get()) : 0;
}
PassRefPtr<AccessibilityUIElement> AccessibilityUIElement::horizontalScrollbar() const
