Modified: trunk/LayoutTests/platform/gtk/TestExpectations (155191 => 155192)
--- trunk/LayoutTests/platform/gtk/TestExpectations 2013-09-06 16:13:52 UTC (rev 155191)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2013-09-06 16:32:30 UTC (rev 155192)
@@ -497,12 +497,6 @@
webkit.org/b/120200 [ Debug ] http/tests/security/XFrameOptions/x-frame-options-allowall.html [ Crash Pass ]
-webkit.org/b/120667 [ Debug ] accessibility/multiselect-list-reports-active-option.html [ Crash ]
-webkit.org/b/120667 [ Debug ] accessibility/notification-listeners.html [ Crash ]
-webkit.org/b/120667 [ Debug ] accessibility/menu-list-sends-change-notification.html [ Crash ]
-webkit.org/b/120667 [ Debug ] accessibility/aria-invalid.html [ Crash ]
-webkit.org/b/120667 [ Debug ] accessibility/aria-checkbox-sends-notification.html [ Crash ]
-
#////////////////////////////////////////////////////////////////////////////////////////
# End of Crashing tests
#////////////////////////////////////////////////////////////////////////////////////////
@@ -1459,7 +1453,7 @@
webkit.org/b/120203 http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html [ Failure ]
-webkit.org/b/120669 [ Release ] accessibility/notification-listeners.html [ Failure ]
+webkit.org/b/120669 accessibility/notification-listeners.html [ Failure ]
webkit.org/b/120670 accessibility/lists.html [ Failure ]
webkit.org/b/120670 accessibility/plugin.html [ Failure ]
Modified: trunk/Tools/ChangeLog (155191 => 155192)
--- trunk/Tools/ChangeLog 2013-09-06 16:13:52 UTC (rev 155191)
+++ trunk/Tools/ChangeLog 2013-09-06 16:32:30 UTC (rev 155192)
@@ -1,3 +1,31 @@
+2013-09-06 Denis Nomiyama <[email protected]>
+
+ [GTK] AccessibilityUIElement::addNotificationListener() crashes on debug build
+ https://bugs.webkit.org/show_bug.cgi?id=120416
+
+ Reviewed by Mario Sanchez Prada.
+
+ Fixed crashes when running debug DRT. Simplified loops at AccessibilityCallbackAtk.cpp where the HashMap
+ iterator was removed inside a loop. Fixed AccessibilityUIElement::addNotificationListener() where
+ m_notificationHandler expected RefPtr.
+
+ The global notification handler was stored in the HashMap with key 0. And this caused an assertion when
+ HashMap::add() or find() are called. To fix it, moved the global handler to a separated pointer.
+
+ * DumpRenderTree/atk/AccessibilityCallbacks.h: Removed the global notification key.
+ * DumpRenderTree/atk/AccessibilityCallbacksAtk.cpp:
+ (axObjectEventListener): Simplified the code by using HashMap::find() and a separate pointer for the
+ global notification handler.
+ (addAccessibilityNotificationHandler): Simplified the code by using HashMap::find() and a separate pointer
+ for the global notification handler.
+ (removeAccessibilityNotificationHandler): Added the removal for the global notification handler.
+ * DumpRenderTree/atk/AccessibilityNotificationHandlerAtk.h:
+ (AccessibilityNotificationHandler::create): Added static function to create
+ AccessibilityNotificationHandler.
+ * DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:
+ (AccessibilityUIElement::addNotificationListener): Assigned m_notificationHandler with
+ AccessibilityNotificationHandler::create().
+
2013-09-06 Gabor Abraham <[email protected]>
[Qt] REGRESSION(r155140) Pixel tests is still broken on Qt with QT_WEBKIT_DISABLE_UIPROCESS_DUMPPIXELS=1
Modified: trunk/Tools/DumpRenderTree/atk/AccessibilityCallbacks.h (155191 => 155192)
--- trunk/Tools/DumpRenderTree/atk/AccessibilityCallbacks.h 2013-09-06 16:13:52 UTC (rev 155191)
+++ trunk/Tools/DumpRenderTree/atk/AccessibilityCallbacks.h 2013-09-06 16:32:30 UTC (rev 155192)
@@ -32,8 +32,6 @@
#include "AccessibilityNotificationHandlerAtk.h"
#include "AccessibilityUIElement.h"
-const PlatformUIElement GlobalNotificationKey = 0;
-
void connectAccessibilityCallbacks();
void disconnectAccessibilityCallbacks();
void addAccessibilityNotificationHandler(AccessibilityNotificationHandler*);
Modified: trunk/Tools/DumpRenderTree/atk/AccessibilityCallbacksAtk.cpp (155191 => 155192)
--- trunk/Tools/DumpRenderTree/atk/AccessibilityCallbacksAtk.cpp 2013-09-06 16:13:52 UTC (rev 155191)
+++ trunk/Tools/DumpRenderTree/atk/AccessibilityCallbacksAtk.cpp 2013-09-06 16:32:30 UTC (rev 155192)
@@ -48,13 +48,16 @@
#include "WebCoreSupport/DumpRenderTreeSupportEfl.h"
#endif
+typedef HashMap<PlatformUIElement, AccessibilityNotificationHandler*> NotificationHandlersMap;
+
static guint stateChangeListenerId = 0;
static guint focusEventListenerId = 0;
static guint activeDescendantChangedListenerId = 0;
static guint childrenChangedListenerId = 0;
static guint propertyChangedListenerId = 0;
static guint visibleDataChangedListenerId = 0;
-static HashMap<PlatformUIElement, AccessibilityNotificationHandler*> notificationHandlers;
+static NotificationHandlersMap notificationHandlers;
+static AccessibilityNotificationHandler* globalNotificationHandler = 0;
extern bool loggingAccessibilityEvents;
@@ -127,23 +130,18 @@
return TRUE;
if (notificationName.length()) {
- for (HashMap<PlatformUIElement, AccessibilityNotificationHandler*>::iterator it = notificationHandlers.begin(); it != notificationHandlers.end(); ++it) {
- if (it->key == accessible || it->key == GlobalNotificationKey) {
- JSRetainPtr<JSStringRef> jsNotificationEventName(Adopt, JSStringCreateWithUTF8CString(reinterpret_cast<const char*>(notificationName.utf8().data())));
- JSValueRef notificationNameArgument = JSValueMakeString(jsContext, jsNotificationEventName.get());
- AccessibilityNotificationHandler* notificationHandler = it->value;
- if (notificationHandler->platformElement()) {
- JSValueRef argument = notificationNameArgument;
- // Listener for one element just gets one argument, the notification name.
- JSObjectCallAsFunction(jsContext, notificationHandler->notificationFunctionCallback(), 0, 1, &argument, 0);
- } else {
- // A global listener gets the element and the notification name as arguments.
- JSValueRef arguments[2];
- arguments[0] = AccessibilityUIElement::makeJSAccessibilityUIElement(jsContext, AccessibilityUIElement(accessible));
- arguments[1] = notificationNameArgument;
- JSObjectCallAsFunction(jsContext, notificationHandler->notificationFunctionCallback(), 0, 2, arguments, 0);
- }
- }
+ JSRetainPtr<JSStringRef> jsNotificationEventName(Adopt, JSStringCreateWithUTF8CString(notificationName.utf8().data()));
+ JSValueRef notificationNameArgument = JSValueMakeString(jsContext, jsNotificationEventName.get());
+ NotificationHandlersMap::iterator elementNotificationHandler = notificationHandlers.find(accessible);
+ if (elementNotificationHandler != notificationHandlers.end()) {
+ // Listener for one element just gets one argument, the notification name.
+ JSObjectCallAsFunction(jsContext, elementNotificationHandler->value->notificationFunctionCallback(), 0, 1, ¬ificationNameArgument, 0);
+ } else if (globalNotificationHandler) {
+ // A global listener gets the element and the notification name as arguments.
+ JSValueRef arguments[2];
+ arguments[0] = AccessibilityUIElement::makeJSAccessibilityUIElement(jsContext, AccessibilityUIElement(accessible));
+ arguments[1] = notificationNameArgument;
+ JSObjectCallAsFunction(jsContext, globalNotificationHandler->notificationFunctionCallback(), 0, 2, arguments, 0);
}
}
@@ -228,17 +226,17 @@
JSValueProtect(jsContext, notificationHandler->notificationFunctionCallback());
// Check if this notification handler is related to a specific element.
if (notificationHandler->platformElement()) {
- if (notificationHandlers.contains(notificationHandler->platformElement())) {
- JSValueUnprotect(jsContext, notificationHandlers.find(notificationHandler->platformElement())->value->notificationFunctionCallback());
- notificationHandlers.remove(notificationHandler->platformElement());
+ NotificationHandlersMap::iterator currentNotificationHandler = notificationHandlers.find(notificationHandler->platformElement());
+ if (currentNotificationHandler != notificationHandlers.end()) {
+ ASSERT(currentNotificationHandler->value->platformElement());
+ JSValueUnprotect(jsContext, currentNotificationHandler->value->notificationFunctionCallback());
+ notificationHandlers.remove(currentNotificationHandler->value->platformElement());
}
notificationHandlers.add(notificationHandler->platformElement(), notificationHandler);
} else {
- if (notificationHandlers.contains(GlobalNotificationKey)) {
- JSValueUnprotect(jsContext, notificationHandlers.find(GlobalNotificationKey)->value->notificationFunctionCallback());
- notificationHandlers.remove(GlobalNotificationKey);
- }
- notificationHandlers.add(GlobalNotificationKey, notificationHandler);
+ if (globalNotificationHandler)
+ JSValueUnprotect(jsContext, globalNotificationHandler->notificationFunctionCallback());
+ globalNotificationHandler = notificationHandler;
}
connectAccessibilityCallbacks();
@@ -257,10 +255,14 @@
if (!jsContext)
return;
- for (HashMap<PlatformUIElement, AccessibilityNotificationHandler*>::iterator it = notificationHandlers.begin(); it != notificationHandlers.end(); ++it) {
- if (it->value == notificationHandler) {
- JSValueUnprotect(jsContext, notificationHandler->notificationFunctionCallback());
- notificationHandlers.remove(it);
+ if (globalNotificationHandler == notificationHandler) {
+ JSValueUnprotect(jsContext, globalNotificationHandler->notificationFunctionCallback());
+ globalNotificationHandler = 0;
+ } else if (notificationHandler->platformElement()) {
+ NotificationHandlersMap::iterator removeNotificationHandler = notificationHandlers.find(notificationHandler->platformElement());
+ if (removeNotificationHandler != notificationHandlers.end()) {
+ JSValueUnprotect(jsContext, removeNotificationHandler->value->notificationFunctionCallback());
+ notificationHandlers.remove(removeNotificationHandler);
}
}
}
Modified: trunk/Tools/DumpRenderTree/atk/AccessibilityNotificationHandlerAtk.cpp (155191 => 155192)
--- trunk/Tools/DumpRenderTree/atk/AccessibilityNotificationHandlerAtk.cpp 2013-09-06 16:13:52 UTC (rev 155191)
+++ trunk/Tools/DumpRenderTree/atk/AccessibilityNotificationHandlerAtk.cpp 2013-09-06 16:32:30 UTC (rev 155192)
@@ -23,7 +23,7 @@
#include "AccessibilityCallbacks.h"
AccessibilityNotificationHandler::AccessibilityNotificationHandler(void)
- : m_platformElement(GlobalNotificationKey)
+ : m_platformElement(0)
, m_notificationFunctionCallback(0)
{
}
Modified: trunk/Tools/DumpRenderTree/atk/AccessibilityNotificationHandlerAtk.h (155191 => 155192)
--- trunk/Tools/DumpRenderTree/atk/AccessibilityNotificationHandlerAtk.h 2013-09-06 16:13:52 UTC (rev 155191)
+++ trunk/Tools/DumpRenderTree/atk/AccessibilityNotificationHandlerAtk.h 2013-09-06 16:32:30 UTC (rev 155192)
@@ -22,10 +22,15 @@
#include <_javascript_Core/JSObjectRef.h>
#include <atk/atk.h>
+#include <wtf/PassRefPtr.h>
#include <wtf/RefCounted.h>
class AccessibilityNotificationHandler : public RefCounted<AccessibilityNotificationHandler> {
public:
+ static PassRefPtr<AccessibilityNotificationHandler> create()
+ {
+ return adoptRef(new AccessibilityNotificationHandler());
+ }
AccessibilityNotificationHandler(void);
~AccessibilityNotificationHandler();
Modified: trunk/Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp (155191 => 155192)
--- trunk/Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp 2013-09-06 16:13:52 UTC (rev 155191)
+++ trunk/Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp 2013-09-06 16:32:30 UTC (rev 155192)
@@ -1050,7 +1050,7 @@
if (m_notificationHandler)
return false;
- m_notificationHandler = new AccessibilityNotificationHandler();
+ m_notificationHandler = AccessibilityNotificationHandler::create();
m_notificationHandler->setPlatformElement(platformUIElement());
m_notificationHandler->setNotificationFunctionCallback(functionCallback);
