[GTK] Missing call to g_object_ref while retrieving accessible table cells
authormario@webkit.org <mario@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Feb 2013 17:15:52 +0000 (17:15 +0000)
committermario@webkit.org <mario@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Feb 2013 17:15:52 +0000 (17:15 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@142883 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/accessibility/table-cell-for-column-and-row-crash-expected.txt [new file with mode: 0644]
LayoutTests/accessibility/table-cell-for-column-and-row-crash.html [new file with mode: 0644]
LayoutTests/platform/chromium/TestExpectations
LayoutTests/platform/win/TestExpectations
LayoutTests/platform/wincairo/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceTable.cpp
Tools/ChangeLog
Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp
Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp

index 390316a7f71e289857a53e9052343d7a6548bbcf..4b066c6c3f80f01bdeb08b5032a097ff815a769f 100644 (file)
@@ -1,3 +1,19 @@
+2013-02-14  Mario Sanchez Prada  <mario.prada@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  <kadam@inf.u-szeged.hu>
 
         [Qt] Reviewing TestExpectations. Added platform specific expected files and unskip them.
diff --git a/LayoutTests/accessibility/table-cell-for-column-and-row-crash-expected.txt b/LayoutTests/accessibility/table-cell-for-column-and-row-crash-expected.txt
new file mode 100644 (file)
index 0000000..b5c4259
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/accessibility/table-cell-for-column-and-row-crash.html b/LayoutTests/accessibility/table-cell-for-column-and-row-crash.html
new file mode 100644 (file)
index 0000000..4ac930d
--- /dev/null
@@ -0,0 +1,45 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../fast/js/resources/js-test-pre.js"></script>
+</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="../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index 2f7f672d17dcfe7111c4432e9c90327aa90b3263..cda40570b250aec2f6717a4203ba1f56ffb563ca 100644 (file)
@@ -1414,6 +1414,7 @@ crbug.com/10322 accessibility/placeholder.html [ Skip ]
 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 ]
index a98eb622a7ecbb0880055d0ce1a96b4b3eff14c4..3b1fea6a6ac53a23c5755e01eca7911e5fbec67c 100644 (file)
@@ -650,6 +650,7 @@ accessibility/secure-textfield-title-ui.html
 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
index bc3b0303a9bf0e9942c1f327d4e432c79d953cc6..7313af24e6532622fa83a1b9e874b56e6d5d7bcf 100644 (file)
@@ -1168,6 +1168,7 @@ accessibility/spinbutton-value.html
 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
index 842ef23b18fc4858ee43caeb8a1761c0e34ab27c..2bbaf170b851cbe6b92ed7b5ad8e689b635d7eb6 100644 (file)
@@ -1,3 +1,18 @@
+2013-02-14  Mario Sanchez Prada  <mario.prada@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  <mifenton@rim.com>
 
         [BlackBerry] Update keyboard event details to match platform details.
index b58746052af0a0179f675eed5db884b316c4b7d4..aee7b7b6972b30fcfe3ec79e34c5df67959b5f80 100644 (file)
@@ -94,7 +94,14 @@ static AtkObject* webkitAccessibleTableRefAt(AtkTable* table, gint row, gint col
     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)
index 0734d4d40a75eb1c6873776a917fd1d6245698a3..785956db89733c08216efc2c815d6925ee185235 100644 (file)
@@ -1,3 +1,19 @@
+2013-02-14  Mario Sanchez Prada  <mario.prada@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.bot@gmail.com>
 
         Unreviewed, rolling out r142841.
index 0cf206631e6a16e1607a466cf6204f6959a85697..2047b3e3f6a363b047231ae3aad7d2ffb5468a3a 100644 (file)
@@ -774,8 +774,10 @@ AccessibilityUIElement AccessibilityUIElement::cellForColumnAndRow(unsigned colu
 
     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()
index 5062a8f8f2a914d5a29be480133fe8a58c2bb1e7..babeb70fa0b7e9ec79db34da5a062f2384c302a8 100644 (file)
@@ -871,8 +871,10 @@ PassRefPtr<AccessibilityUIElement> AccessibilityUIElement::cellForColumnAndRow(u
     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