[webkit-reviews] review granted: [Bug 196601] [ATK] Cleanup accessible wrapper base class : [Attachment 366803] Rebased patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 5 03:00:42 PDT 2019

Mario Sanchez Prada <mario at webkit.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 196601: [ATK] Cleanup accessible wrapper base class

Attachment 366803: Rebased patch


--- Comment #4 from Mario Sanchez Prada <mario at webkit.org> ---
Comment on attachment 366803
  --> https://bugs.webkit.org/attachment.cgi?id=366803
Rebased patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366803&action=review

It looks good to me, only one comment on that there's a part of the code that
looks like could be removed altogether (either now or in a follow-up patch)

> +	   auto* atkParent = parent ? ATK_OBJECT(parent->wrapper()) : nullptr;

It seems like this will always yield nullptr to atkParent, since `parent ==
nullptr` already if you made it here... meaning that the for loop below won't
ever be executed.

Actually, it seems that the whole `(!parent && isRootObject(coreObject)) { ...
}` block can be removed, as all we can do if `parent == nullptr` at this point
is to return -1, which is what the if block right after this one does.

However, I think it's better to keep this for now as this patch is just about
cleaning things up and translating, so feel free to address that in a follow-up

More information about the webkit-reviews mailing list