Bug 119333

Summary: [EFL] REGRESSION after r153432: some accessibility tests crash.
Product: WebKit Reporter: Krzysztof Czech <k.czech>
Component: WebKit EFLAssignee: Mario Sanchez Prada <mario>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, gustavo, lucas.de.marchi, mario
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch proposal
none
Patch proposal none

Krzysztof Czech
Reported 2013-07-31 03:48:47 PDT
Crash log: 0xb70267b7 0xb717b288 0xb3fcf208 g_object_ref_sink 0xaf19b444 0xaf19d023 WTR::AccessibilityUIElement::allAttributes() 0xaf18ec57 WTR::JSAccessibilityUIElement::allAttributes(OpaqueJSContext const*, OpaqueJSVal ue*, OpaqueJSValue*, unsigned int, OpaqueJSValue const* const*, OpaqueJSValue const**) 0xb6c6b32b long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(JSC::ExecState *) 0xb70182d7 0xb701f648 0xb7025f33 0xb6e402bb JSC::JITCode::execute(JSC::JSStack*, JSC::ExecState*, JSC::VM*)
Attachments
Patch proposal (2.44 KB, patch)
2013-07-31 04:09 PDT, Mario Sanchez Prada
no flags
Patch proposal (1.80 KB, patch)
2013-08-07 13:42 PDT, Mario Sanchez Prada
no flags
Krzysztof Czech
Comment 1 2013-07-31 03:50:47 PDT
It seems AccessibilityUIElement does not have parent.
Mario Sanchez Prada
Comment 2 2013-07-31 03:56:08 PDT
Oops! I forgot to add that null check, sorry about that I'll be posting a patch right away
Mario Sanchez Prada
Comment 3 2013-07-31 04:09:37 PDT
Created attachment 207832 [details] Patch proposal Let's hope I have not made more mistakes
Mario Sanchez Prada
Comment 4 2013-07-31 04:10:18 PDT
Adding Gustavo as reviewer for this -almost- one liner patch
Mario Sanchez Prada
Comment 5 2013-08-02 08:27:22 PDT
Comment on attachment 207832 [details] Patch proposal Thanks for the review. Adding it to the commit queue...
WebKit Commit Bot
Comment 6 2013-08-02 08:51:28 PDT
Comment on attachment 207832 [details] Patch proposal Clearing flags on attachment: 207832 Committed r153651: <http://trac.webkit.org/changeset/153651>
WebKit Commit Bot
Comment 7 2013-08-02 08:51:31 PDT
All reviewed patches have been landed. Closing bug.
Krzysztof Czech
Comment 8 2013-08-05 02:30:42 PDT
I'm reopening this bug. Crash still occurs, I guess even on gtk port as well.
Krzysztof Czech
Comment 9 2013-08-05 03:31:53 PDT
The crash seems to appear while calling parent->platformUIElement(). This method returns PlatformUIElement type which is a GRefPtr<AtkObject>. I believe in this context: AtkObject* atkParent = parent ? parent->platformUIElement().get() : 0; platformUIElement().get(), before get() is called, the temporary PlatformUIElement is created and refGPtr(ptr) is called where ptr is a GRefPtr<AtkObject>. I think, that's way g_object_ref_sink protests. I guess platformUIElement could be specialized for ATK so that it could return AtkObject*.
Krzysztof Czech
Comment 10 2013-08-05 03:41:35 PDT
While testing this approach with specialized platformUIElement (I called it platformUIElementAtk()), I did find this crash, but another one appeared: 1 0xb7033767 2 0xb7186288 3 0xb3ed5627 atk_object_get_role 4 0xafb9b568 5 0xafb9d173 WTR::AccessibilityUIElement::allAttributes() 6 0xafb8ec47 WTR::JSAccessibilityUIElement::allAttributes(OpaqueJSContext const*, OpaqueJSVal ue*, OpaqueJSValue*, unsigned int, OpaqueJSValue const* const*, OpaqueJSValue const**) 7 0xb6c79292 long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(JSC::ExecState *) 8 0xb7025035 9 0xb702c448 10 0xb7032ee3 11 0xb6e4d3cb JSC::JITCode::execute(JSC::JSStack*, JSC::ExecState*, JSC::VM*)
Krzysztof Czech
Comment 11 2013-08-05 03:42:40 PDT
(In reply to comment #10) > ... I did find this crash ... I did not find this crash
Mario Sanchez Prada
Comment 12 2013-08-05 08:19:18 PDT
Thanks Krzysztof for reporting that the issue has not been properly fixed yet. I can take a look to it tomorrow if you want, but please confirm that will be fine since I see you have been already doing some investigation and I don't want to collide with your efforts, should you were planning to work on this.
Krzysztof Czech
Comment 13 2013-08-05 08:27:10 PDT
(In reply to comment #12) > Thanks Krzysztof for reporting that the issue has not been properly fixed yet. I can take a look to it tomorrow if you want, but please confirm that will be fine since I see you have been already doing some investigation and I don't want to collide with your efforts, should you were planning to work on this. Yes, I will be fine if you take a look at this issue. I wrote some suppositions, but I'm not sure whether they hit the point, they may be wrong. I just looked at this briefly.
Mario Sanchez Prada
Comment 14 2013-08-05 08:42:39 PDT
(In reply to comment #13) > (In reply to comment #12) > > Thanks Krzysztof for reporting that the issue has not been properly fixed yet. I can take a look to it tomorrow if you want, but please confirm that will be fine since I see you have been already doing some investigation and I don't want to collide with your efforts, should you were planning to work on this. > Yes, I will be fine if you take a look at this issue. I wrote some suppositions, but I'm not sure whether they hit the point, they may be wrong. I just looked at this briefly. Ok, fair enough. I'll work tomorrow on that then. Today I'm just almost dead because of this GUADEC conference :)
Mario Sanchez Prada
Comment 15 2013-08-07 13:42:33 PDT
Created attachment 208297 [details] Patch proposal The problem seems to be more simple in the end: We just need to store a RefPtr for the parent instead of the raw pointer.
WebKit Commit Bot
Comment 16 2013-08-07 14:12:31 PDT
Comment on attachment 208297 [details] Patch proposal Clearing flags on attachment: 208297 Committed r153798: <http://trac.webkit.org/changeset/153798>
WebKit Commit Bot
Comment 17 2013-08-07 14:12:34 PDT
All reviewed patches have been landed. Closing bug.
Krzysztof Czech
Comment 18 2013-08-08 00:17:06 PDT
Thanks Mario
Note You need to log in before you can comment on or make changes to this bug.