Bug 94200 - REGRESSION (r125710): accessibility/accessibility-node-reparent.html, accessibility/accessibility-node-memory-management.html failing on GTK Linux
Summary: REGRESSION (r125710): accessibility/accessibility-node-reparent.html, accessi...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dominic Mazzoni
URL: http://build.webkit.org/results/GTK%2...
Keywords: LayoutTestFailure, MakingBotsRed, Regression
: 94204 (view as bug list)
Depends on: 94156
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-16 00:44 PDT by Zan Dobersek
Modified: 2012-08-27 02:15 PDT (History)
8 users (show)

See Also:


Attachments
Crash log (21.52 KB, application/octet-stream)
2012-08-17 00:57 PDT, Zan Dobersek
no flags Details
Patch (3.54 KB, patch)
2012-08-21 15:30 PDT, Dominic Mazzoni
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2012-08-16 00:44:05 PDT
accessibility/accessibility-node-reparent.html and accessibility/accessibility-node-memory-management.html started failing on GTK Linux 64-bit Release after introduced in r125710.
http://trac.webkit.org/changeset/125710

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=accessibility%2Faccessibility-node

--- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/accessibility/accessibility-node-memory-management-expected.txt 
+++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/accessibility/accessibility-node-memory-management-actual.txt 
@@ -5,7 +5,7 @@
 
 PASS expectedButtonRole != expectedDetachedRole is true
 PASS canvasButtonRole is expectedButtonRole
-PASS detachedCanvasButtonRole is expectedDetachedRole
+FAIL detachedCanvasButtonRole should be AXRole: unknown. Was AXRole: list item.
 PASS successfullyParsed is true
 
 TEST COMPLETE


--- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/accessibility/accessibility-node-reparent-expected.txt 
+++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/accessibility/accessibility-node-reparent-actual.txt 
@@ -6,7 +6,7 @@
 
 PASS expectedButtonRole != expectedDetachedRole is true
 PASS canvasButtonRole is expectedButtonRole
-PASS detachedCanvasButtonRole is expectedDetachedRole
+FAIL detachedCanvasButtonRole should be AXRole: unknown. Was AXRole: list item.
 PASS reparentedButtonRole is expectedButtonRole
 PASS successfullyParsed is true
Comment 1 Sergio Villar Senin 2012-08-16 01:25:57 PDT
Moving component to accessibility and adding a couple of guys to de Cc.

Also I'd like to point out the crash in accessibility/canvas-fallback-content.html:
http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r125750%20(35657)/accessibility/canvas-fallback-content-crash-log.txt

Program terminated with signal 11, Segmentation fault.
#0  0x00007fdf573c2c32 in WebCore::Node::isFocusable (this=0x317c400) at ../../Source/WebCore/dom/Node.cpp:927

Thread 1 (Thread 0x7fdf4c6ce900 (LWP 1856)):
#0  0x00007fdf573c2c32 in WebCore::Node::isFocusable (this=0x317c400) at ../../Source/WebCore/dom/Node.cpp:927
#1  0x00007fdf57049912 in WebCore::AccessibilityNodeObject::determineAccessibilityRole (this=0x31cdc60) at ../../Source/WebCore/accessibility/AccessibilityNodeObject.cpp:268
#2  0x00007fdf57048f4f in WebCore::AccessibilityNodeObject::init (this=0x31cdc60) at ../../Source/WebCore/accessibility/AccessibilityNodeObject.cpp:99
#3  0x00007fdf57048fa2 in WebCore::AccessibilityNodeObject::create (node=0x317c400) at ../../Source/WebCore/accessibility/AccessibilityNodeObject.cpp:105
#4  0x00007fdf57070e15 in WebCore::createFromNode (node=0x317c400) at ../../Source/WebCore/accessibility/AXObjectCache.cpp:283
#5  0x00007fdf570711a3 in WebCore::AXObjectCache::getOrCreate (this=0x315b8f0, node=0x317c400) at ../../Source/WebCore/accessibility/AXObjectCache.cpp:323
#6  0x00007fdf583eef94 in WebCore::AXObjectCache::handleFocusedUIElementChanged (this=0x315b8f0, oldFocusedNode=0x317cc20, newFocusedNode=0x317c400) at ../../Source/WebCore/accessibility/gtk/AXObjectCacheAtk.cpp:248
#7  0x00007fdf57332425 in WebCore::Document::setFocusedNode (this=0x313afc0, prpNewFocusedNode=...) at ../../Source/WebCore/dom/Document.cpp:3862
#8  0x00007fdf578684f1 in WebCore::FocusController::setFocusedNode (this=0x22df9e0, node=0x317c400, newFocusedFrame=...) at ../../Source/WebCore/page/FocusController.cpp:648
#9  0x00007fdf57382a2a in WebCore::Element::focus (this=0x317c400, restorePreviousSelection=true) at ../../Source/WebCore/dom/Element.cpp:1578
#10 0x00007fdf57fefc0b in WebCore::jsElementPrototypeFunctionFocus (exec=0x7fdf080b40d0) at DerivedSources/WebCore/JSElement.cpp:2023
Comment 2 Sergio Villar Senin 2012-08-16 01:26:07 PDT
*** Bug 94204 has been marked as a duplicate of this bug. ***
Comment 3 Zan Dobersek 2012-08-17 00:57:53 PDT
Created attachment 159033 [details]
Crash log

The same crash also occurs in fast/canvas/fallback-content.html
Also occurs on Apple Lion Debug WK2 bots:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&showAllRuns=true&tests=accessibility%2Fcanvas-fallback-content.html%2Cfast%2Fcanvas%2Ffallback-content.html

Attaching the complete crash log.
Comment 4 Dominic Mazzoni 2012-08-17 08:02:29 PDT
FYI, the canvas-fallback-content crash is fixed by the patch on 94156.

The GTK-specific failures will need some investigation, and I've updated the test expectations for now.
Comment 5 Dominic Mazzoni 2012-08-21 15:23:21 PDT
The crash has been fixed.

I debugged the test failures, and I have a patch. It was a bug in DumpRenderTree/gtk.
Comment 6 Dominic Mazzoni 2012-08-21 15:30:55 PDT
Created attachment 159781 [details]
Patch
Comment 7 Martin Robinson 2012-08-21 15:50:47 PDT
Looks good to me, but I think that Mario should double-check this one.
Comment 8 Mario Sanchez Prada 2012-08-22 00:57:05 PDT
(In reply to comment #7)
> Looks good to me, but I think that Mario should double-check this one.

Looks good to me too. Thanks for fixing these two, Dominic!
Comment 9 chris fleizach 2012-08-22 07:59:41 PDT
Comment on attachment 159781 [details]
Patch

r=me
Comment 10 WebKit Review Bot 2012-08-22 09:00:48 PDT
Comment on attachment 159781 [details]
Patch

Clearing flags on attachment: 159781

Committed r126309: <http://trac.webkit.org/changeset/126309>
Comment 11 WebKit Review Bot 2012-08-22 09:00:51 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Alejandro Piñeiro 2012-08-24 04:43:49 PDT
While working on other bug I run the tests, and for several of them I found several g_object_ref/g_object_unref on stderr. For example:

+accessibility/aria-hidden-updates-alldescendants.html	stderr

g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_unref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_unref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_unref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_unref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_unref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_unref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_unref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_unref: assertion `G_IS_OBJECT (object)' failed
g_object_ref: assertion `G_IS_OBJECT (object)' failed
g_object_unref: assertion `G_IS_OBJECT (object)' failed


After investigating a little I found that this started to happen after the patch on this bug. If I revert the patch I don't get those messages.

Should I reopen this bug?
Comment 13 Zan Dobersek 2012-08-24 04:55:10 PDT
(In reply to comment #12)
> While working on other bug I run the tests, and for several of them I found several g_object_ref/g_object_unref on stderr. For example:
> 
> +accessibility/aria-hidden-updates-alldescendants.html    stderr
> 
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_unref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_unref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_unref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_unref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_unref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_unref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_unref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_unref: assertion `G_IS_OBJECT (object)' failed
> g_object_ref: assertion `G_IS_OBJECT (object)' failed
> g_object_unref: assertion `G_IS_OBJECT (object)' failed
> 
> 
> After investigating a little I found that this started to happen after the patch on this bug. If I revert the patch I don't get those messages.
> 
> Should I reopen this bug?

Seems as null pointers are passed into AccessibilityUIElement constructors. Perhaps using a GRefPtr would fix this in style? That is, using 'typedef GRefPtr<AtkObject> PlatformUIElement;' in AccessibilityUIElement.h.
Comment 14 Dominic Mazzoni 2012-08-24 08:02:58 PDT
My vote would be a new bug.

Reverting the ref/unref is definitely wrong, it'd be a step backwards to not have that.

GRefPtr might help, but I think there are probably a couple of cases where a null check might be needed. It'd be best to actually debug these.
Comment 15 Alejandro Piñeiro 2012-08-27 02:15:43 PDT
(In reply to comment #14)
> My vote would be a new bug.

Done: https://bugs.webkit.org/show_bug.cgi?id=95062