[GTK] WebCore::returnString is unsafe and should be removed!
https://bugs.webkit.org/show_bug.cgi?id=110423
Reviewed by Martin Robinson.
Remove returnString() and replace it in callers with a new
function that will cache and return the values of string
properties for ATK interfaces in the private section of the
wrapper AtkObject WebKitAccessible.
* accessibility/atk/WebKitAccessibleUtil.cpp: Remove returnString().
* accessibility/atk/WebKitAccessibleUtil.h: Ditto.
* accessibility/atk/WebKitAccessibleWrapperAtk.cpp:
(_WebKitAccessiblePrivate): New private structure to store cached
values of string property for the different ATK interfaces.
(cacheAndReturnAtkProperty): New helper function to make sure the
returned const gchar* value is previously cached in the private
section of the wrapper AtkObject.
(webkitAccessibleGetName): Use the new helper function.
(webkitAccessibleGetDescription): Ditto.
(webkitAccessibleInit): Initialize pointer to private structure.
(webkitAccessibleFinalize): Remove unneeded code.
(webkitAccessibleClassInit): Add private struct to class type/
* accessibility/atk/WebKitAccessibleWrapperAtk.h:
(_WebKitAccessible): New member pointing to the private structure,
made the new helper function cacheAndReturnAtkProperty() available
to external callers (implementation files for ATK interfaces).
(AtkCachedProperty): New enum to allow reusing code when calling
cacheAndReturnAtkProperty() to cache and return different properties.
* accessibility/atk/WebKitAccessibleInterfaceAction.cpp:
(webkitAccessibleActionGetKeybinding): Replace calls to returnString()
with calls to the new helper function cacheAndReturnAtkProperty().
(webkitAccessibleActionGetName): Ditto.
* accessibility/atk/WebKitAccessibleInterfaceDocument.cpp:
(documentAttributeValue): Ditto.
(webkitAccessibleDocumentGetLocale): Ditto.
* accessibility/atk/WebKitAccessibleInterfaceImage.cpp:
(webkitAccessibleImageGetImageDescription): Ditto.
Remove returnString() from WebKitAccessibleHyperlink (which is not
an AtkObject, but a GObject) as well, replacing it in callers with
simple code that will cache and return the required values in the
private section of these kind of objects.
* accessibility/atk/WebKitAccessibleHyperlink.cpp:
(_WebKitAccessibleHyperlinkPrivate): Added two new fields to cache
string values for the key binding and name properties from the
AtkAction interface, which is implemented by AtkHyperlink.
(webkitAccessibleHyperlinkActionGetKeybinding): Cache the string
value for the key binding before returning a const gchar* pointer.
(webkitAccessibleHyperlinkActionGetName): Ditto.
(webkitAccessibleHyperlinkGetURI): Do not cache the URI here, as
this function returns a gchar* that will be owned by the caller.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@144636 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp b/Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp
index 92e1f67..dd4702a 100644
--- a/Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp
+++ b/Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp
@@ -2,6 +2,7 @@
* Copyright (C) 2008 Nuanti Ltd.
* Copyright (C) 2009 Jan Alonzo
* Copyright (C) 2009, 2010, 2011, 2012 Igalia S.L.
+ * Copyright (C) 2013 Samsung Electronics
*
* Portions from Mozilla a11y, copyright as follows:
*
@@ -66,6 +67,27 @@
using namespace WebCore;
+struct _WebKitAccessiblePrivate {
+ // Cached data for AtkObject.
+ CString accessibleName;
+ CString accessibleDescription;
+
+ // Cached data for AtkAction.
+ CString actionName;
+ CString actionKeyBinding;
+
+ // Cached data for AtkDocument.
+ CString documentLocale;
+ CString documentType;
+ CString documentEncoding;
+ CString documentURI;
+
+ // Cached data for AtkImage.
+ CString imageDescription;
+};
+
+#define WEBKIT_ACCESSIBLE_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE((obj), WEBKIT_TYPE_ACCESSIBLE, WebKitAccessiblePrivate))
+
static AccessibilityObject* fallbackObject()
{
// FIXME: An AXObjectCache with a Document is meaningless.
@@ -100,7 +122,7 @@
{
AccessibilityObject* coreObject = core(object);
if (!coreObject->isAccessibilityRenderObject())
- return returnString(coreObject->stringValue());
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleName, coreObject->stringValue());
if (coreObject->isFieldset()) {
AccessibilityObject* label = coreObject->titleUIElement();
@@ -122,7 +144,7 @@
// Try text under the node.
String textUnder = coreObject->textUnderElement();
if (textUnder.length())
- return returnString(textUnder);
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleName, textUnder);
}
if (coreObject->isImage() || coreObject->isInputImage()) {
@@ -131,7 +153,7 @@
// Get the attribute rather than altText String so as not to fall back on title.
String alt = toHTMLElement(node)->getAttribute(HTMLNames::altAttr);
if (!alt.isEmpty())
- return returnString(alt);
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleName, alt);
}
}
@@ -139,16 +161,16 @@
if (coreObject->isWebArea()) {
Document* document = coreObject->document();
if (document)
- return returnString(document->title());
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleName, document->title());
}
// Nothing worked so far, try with the AccessibilityObject's
// title() before going ahead with stringValue().
String axTitle = accessibilityTitle(coreObject);
if (!axTitle.isEmpty())
- return returnString(axTitle);
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleName, axTitle);
- return returnString(coreObject->stringValue());
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleName, coreObject->stringValue());
}
static const gchar* webkitAccessibleGetDescription(AtkObject* object)
@@ -158,22 +180,22 @@
if (coreObject->isAccessibilityRenderObject())
node = coreObject->node();
if (!node || !node->isHTMLElement() || coreObject->ariaRoleAttribute() != UnknownRole)
- return returnString(accessibilityDescription(coreObject));
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleDescription, accessibilityDescription(coreObject));
// atk_table_get_summary returns an AtkObject. We have no summary object, so expose summary here.
if (coreObject->roleValue() == TableRole) {
String summary = static_cast<HTMLTableElement*>(node)->summary();
if (!summary.isEmpty())
- return returnString(summary);
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleDescription, summary);
}
// The title attribute should be reliably available as the object's descripton.
// We do not want to fall back on other attributes in its absence. See bug 25524.
String title = toHTMLElement(node)->title();
if (!title.isEmpty())
- return returnString(title);
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleDescription, title);
- return returnString(accessibilityDescription(coreObject));
+ return cacheAndReturnAtkProperty(object, AtkCachedAccessibleDescription, accessibilityDescription(coreObject));
}
static void setAtkRelationSetFromCoreObject(AccessibilityObject* coreObject, AtkRelationSet* relationSet)
@@ -763,14 +785,13 @@
if (ATK_OBJECT_CLASS(webkitAccessibleParentClass)->initialize)
ATK_OBJECT_CLASS(webkitAccessibleParentClass)->initialize(object, data);
- WEBKIT_ACCESSIBLE(object)->m_object = reinterpret_cast<AccessibilityObject*>(data);
+ WebKitAccessible* accessible = WEBKIT_ACCESSIBLE(object);
+ accessible->m_object = reinterpret_cast<AccessibilityObject*>(data);
+ accessible->priv = WEBKIT_ACCESSIBLE_GET_PRIVATE(accessible);
}
static void webkitAccessibleFinalize(GObject* object)
{
- // This is a good time to clear the return buffer.
- returnString(String());
-
G_OBJECT_CLASS(webkitAccessibleParentClass)->finalize(object);
}
@@ -793,6 +814,8 @@
klass->get_index_in_parent = webkitAccessibleGetIndexInParent;
klass->get_attributes = webkitAccessibleGetAttributes;
klass->ref_relation_set = webkitAccessibleRefRelationSet;
+
+ g_type_class_add_private(klass, sizeof(WebKitAccessiblePrivate));
}
GType
@@ -1121,4 +1144,58 @@
return firstUnignoredParent;
}
+const char* cacheAndReturnAtkProperty(AtkObject* object, AtkCachedProperty property, String value)
+{
+ WebKitAccessiblePrivate* priv = WEBKIT_ACCESSIBLE(object)->priv;
+ CString* propertyPtr = 0;
+
+ switch (property) {
+ case AtkCachedAccessibleName:
+ propertyPtr = &priv->accessibleName;
+ break;
+
+ case AtkCachedAccessibleDescription:
+ propertyPtr = &priv->accessibleDescription;
+ break;
+
+ case AtkCachedActionName:
+ propertyPtr = &priv->actionName;
+ break;
+
+ case AtkCachedActionKeyBinding:
+ propertyPtr = &priv->actionKeyBinding;
+ break;
+
+ case AtkCachedDocumentLocale:
+ propertyPtr = &priv->documentLocale;
+ break;
+
+ case AtkCachedDocumentType:
+ propertyPtr = &priv->documentType;
+ break;
+
+ case AtkCachedDocumentEncoding:
+ propertyPtr = &priv->documentEncoding;
+ break;
+
+ case AtkCachedDocumentURI:
+ propertyPtr = &priv->documentURI;
+ break;
+
+ case AtkCachedImageDescription:
+ propertyPtr = &priv->imageDescription;
+ break;
+
+ default:
+ ASSERT_NOT_REACHED();
+ }
+
+ // Don't invalidate old memory if not stricly needed, since other
+ // callers might be still holding on to it.
+ if (*propertyPtr != value.utf8())
+ *propertyPtr = value.utf8();
+
+ return (*propertyPtr).data();
+}
+
#endif // HAVE(ACCESSIBILITY)
