[Webkit-unassigned] [Bug 121959] [ATK] Normalize checks in entry points for DRT and WKTR

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Mario Sanchez Prada <mario at webkit.org>  2013-09-26 13:34:11 PST ---
(In reply to comment #3)
> (From update of attachment 212717 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=212717&action=review
> 
> > Tools/ChangeLog:5
> > +
> 
> can you add a description of what and why this patch is needed

Strange. I had written one already but for it seems I just did it as a commit message and not in the proper ChangeLog, since the description is here:

https://bug-121959-attachments.webkit.org/attachment.cgi?id=212717

Anyway, I'll put it in the right place and submit the patch again.

> > Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:814
> > +    if (!ATK_IS_VALUE(m_element.get()))
> 
> I don't know much about this ATK_IS_* API, but it does seem strange to me that an m_element could be
> ATK_IS_VALUE or ATK_IS_OBJECT
> 
> The difference between ATK_IS_OBJECT and ATK_IS_COMPONENT is also pretty nebulous it seems.

Yes, I agree it's not very clear but it makes sense after all.

TL;DR:
------
AtkObject is an abstract class, while AtkComponent, AtkValue, AtkDocument... are interfaces that have to be implemented by instances of AtkObject, or derived class. Thus, checking ATK_IS_IMAGE() or ATK_COMPONENT() effectively implies in our case ATK_IS_OBJECT(), since those interfaces are only implemented by AtkObject.

Long version:
-------------
Welcome to GObject! :)

In a nutshell, AtkObject is an abstract class which I derived our concrete implementation WebKitAccessible, which is what you can see in WebKitAccessibleWrapperAtk, and what we use as our base for implementing accessible objects in ATK.

Then other things such as AtkValue, AtkComponent, AtkAction... are just interfaces that are implemented by subclasses of WebKitAccessible, we generate those classes/types dynamically according to tha nature of each wrapped AccessibilityObject, and we of course reuse those dynamically generated types when we found objects requiring to implement exactly the same interfaces that another one found earlier.

You can see a trace of those dynamically generated types in the description of this bug: "WAIType191" & "WAIType199". Each of those classes will be subclasses of WebKitAccessible (thus subclasses of AtkObject) implementing a different sets of interfaces.

So far so good, but this is plain C and so all this object oriented thang is not provided by the language, so we have GObject which is a library that provides us a whole system to implement OOP in C and that, besides forcing us to work in a certain way when writing the code (e.g. implementing something similar to C++ vtables ourselves),  includes macros such as ATK_OBJECT (just a cast to AtkObject*) or ATK_IS_OBJECT (to check if a pointer references a real instance of AtkObject), or WEBKIT_ACCESSIBLE and WEBKIT_IS_ACCESSIBLE (same thing but for objects from that subclass).

And turns out that, as we do neither things such as polymorphism in C, we ended up using those macros all the time, depending on which API we want to use: for instance, if we have an instance of WebKitAccessible and want to use some of the AtkObject API over it, we need to cast it first, so we make sure we only use the region of memory related to AtkObject among the whole lot of memory used for a WebKitAccessible:

  atk_object_get_name(ATK_OBJECT(webkitAccessible))

So, for the interfaces is exactly the same: we use macros such as ATK_IMAGE or ATK_COMPONENT whenever we want to use the API of a given interface over a specific AtkObject or, in the case of WebKitGTK, of WebKitAccessible:

  atk_table_get_n_columns(ATK_TABLE(webkitAccessible))

In a similar fashion, when we want to check if a pointer references a valid AtkObject we use ATK_IS_OBJECT(object), and when we want to check if a given AtkObject implements one interface such as AtkComponent, we do ATK_IS_COMPONENT(object). And as I mentioned in the TL;DR section above, as those interfaces are always implemented by AtkObject's only, they effectively imply the ATK_IS_OBJECT(object) check, thus it's not needed.

So, wrapping up: AtkObject is an abstrac class, while the rest of things (e.g. AtkImage, AtkDocument...) are interfaces. That is why something can be ATK_OBJECT and ATK_VALUE at the same time.

Hope is clearer now :)

> > Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:1079
> >      if (!ATK_IS_ACTION(m_element.get()))
> 
> ditto about the naming. maybe this is right, but it does seem strange that you can check that the element is ATK_IS_ACTION

In this specific case, I'm checking "if it's an AtkObject implementing the AtkAction interface". This will fail either if one of those is not true, or if it's just null.

Attaching a new patch with the right changelog soon...

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.