[webkit-reviews] review denied: [Bug 92160] [BlackBerry] Support async spellcheck for the blackberry port : [Attachment 154355] Patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Nima Ghanavatian
<nima.ghanavatian at gmail.com>'s request for review:
Bug 92160: [BlackBerry] Support async spellcheck for the blackberry port
https://bugs.webkit.org/show_bug.cgi?id=92160

Attachment 154355: Patch
https://bugs.webkit.org/attachment.cgi?id=154355&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=154355&action=review


ChangeLogs are fine now. Please have a look at my comments and also the patch
needs to be reapplied against trunk so the style checker can run.

> Source/WebCore/ChangeLog:6
> +	   2012-07-24  Nima Ghanavatian  <nghanavatian at rim.com>

You do not need this line.

> Source/WebKit/blackberry/ChangeLog:6
> +	   2012-07-24  Nima Ghanavatian  <nghanavatian at rim.com>

Ditto.

> Source/WebCore/platform/text/TextChecking.h:40
> +#if PLATFORM(MAC) && !defined(BUILDING_ON_LEOPARD) || PLATFORM(BLACKBERRY)

Better add some braces to make more clear how this should be evaluated.

> Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp:533
> +	   free(checkingString);

I am not sure if it needs to be freed since malloc failed. But what about when
it succeeded, does it need to be freed in that case? Are we leaking
checkingString?