Bug 119333 – [EFL] REGRESSION after r153432: some accessibility tests crash.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Krzysztof Czech]

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*)

[Krzysztof Czech]

It seems AccessibilityUIElement does not have parent.

[Mario Sanchez Prada]

Oops! I forgot to add that null check, sorry about that

I'll be posting a patch right away

[Mario Sanchez Prada]

Created attachment 207832 [details]
Patch proposal

Let's hope I have not made more mistakes

[Mario Sanchez Prada]

Adding Gustavo as reviewer for this -almost- one liner patch

[Mario Sanchez Prada]

Comment on attachment 207832 [details]
Patch proposal

Thanks for the review. Adding it to the commit queue...

[WebKit Commit Bot]

Comment on attachment 207832 [details]
Patch proposal

Clearing flags on attachment: 207832

Committed r153651: <http://trac.webkit.org/changeset/153651>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Krzysztof Czech]

I'm reopening this bug. Crash still occurs, I guess even on gtk port as well.

[Krzysztof Czech]

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]

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]

(In reply to comment #10)
> ... I did find this crash ...
I did not find this crash

[Mario Sanchez Prada]

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]

(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]

(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]

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 on attachment 208297 [details]
Patch proposal

Clearing flags on attachment: 208297

Committed r153798: <http://trac.webkit.org/changeset/153798>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Krzysztof Czech]

Thanks Mario