[webkit-reviews] review denied: [Bug 73668] Upstream BlackBerry API backing store files : [Attachment 118047] Updated patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 8 13:32:57 PST 2011


Daniel Bates <dbates at webkit.org> has denied Jacky Jiang
<zkjiang008 at gmail.com>'s request for review:
Bug 73668: Upstream BlackBerry API backing store files
https://bugs.webkit.org/show_bug.cgi?id=73668

Attachment 118047: Updated patch
https://bugs.webkit.org/attachment.cgi?id=118047&action=review

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=118047&action=review


> Source/WebKit/blackberry/Api/BackingStore.cpp:2437
> +#if OS(QNX)

Remove this if-define as it's unnecessary.

> Source/WebKit/blackberry/Api/BackingStore.cpp:2438
> +    // Store temps.

Remove this comment as it's inane.

> Source/WebKit/blackberry/Api/BackingStore.h:23
> +

Remove this empty line as it doesn't improve the readability of this file.

> Source/WebKit/blackberry/Api/BackingStore.h:25
> +#include <BlackBerryPlatformGraphics.h>
> +#include <BlackBerryPlatformPrimitives.h>

As far as I can tell we neither of these includes are necessary. Instead, we
should just forward declare BlackBerry::Platform::IntRect.

> Source/WebKit/blackberry/Api/BackingStore_p.h:27
> +

Remove this empty line as it doesn't improve the readability of this file.

> Source/WebKit/blackberry/Api/BackingStore_p.h:354
> +}

Please add an inline comment on this line: // namespace WebKit

> Source/WebKit/blackberry/Api/BackingStore_p.h:355
> +}

Please add an inline comment on this line: // namespace BlackBerry


More information about the webkit-reviews mailing list