Bug 58429 – [GTK] Missing nullchecks in GTK's a11y wrapper

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Mario Sanchez Prada]

There are several places in the AccessibilityObjectWrapperAtk.cpp file where we are not null-checking the return of calling to coreObject->document() (coreObject as an instance of AccessibilityObject) and we're using it rightaway, assuming it won't ever be NULL, even though 'experience' tells us it can happen (I've observed some crashes recently because of that)

Hence, it would be good to add some extra checks to prevent these situations.

[Mario Sanchez Prada]

Created attachment 89363 [details]
Patch proposal

Attached patch proposal. 

No need to attach a test since it's just about adding some extra nullchecks to avoid potential problems. Other than that, it keeps passing all API and Layout tests up to date.

[Martin Robinson]

Comment on attachment 89363 [details]
Patch proposal

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

Looks good, but consider my comments before landing.

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1560
> +    Document* document = coreObject->document();
>      switch(coords) {
>      case ATK_XY_SCREEN:
> -        extents = coreObject->document()->view()->contentsToScreen(extents);
> +        if (document)
> +            extents = document->view()->contentsToScreen(extents);
>          break;

If you only use "document" in this if block you should probably do:

if (Document* document = coreObject->document())
    extents = document->view()->contentsToScreen(extents);

and remove the first declaration.

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1836
> +    Document* document = coreObject->document();
> +    if (!document)
> +        return;
> +
> +    if (!document->frame())
>          return;

Can't this just be:

if (!document || !document->frame())
    return;

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1867
> +    Document* document = coreObject->document();
> +    if (!document)
>          return;
> +
> +    if (!document->frame())
> +        return;
> +

Ditto.

[Mario Sanchez Prada]

Committed r83746: <http://trac.webkit.org/changeset/83746>