[webkit-reviews] review granted: [Bug 21816] Clean up ImageBuffer.h so there do not have to be separate ifdefs for each platform : [Attachment 24713] Patch v4

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Brett Wilson (Google)
<brettw at chromium.org>'s request for review:
Bug 21816: Clean up ImageBuffer.h so there do not have to be separate ifdefs
for each platform
https://bugs.webkit.org/show_bug.cgi?id=21816

Attachment 24713: Patch v4
https://bugs.webkit.org/attachment.cgi?id=24713&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
> +    success = false;  // Make early return mean failure.

If you wanted to match the semantics of your earlier patch you could have done
this at the call site. I think it's fine the way it is, though.

> +    ASSERT((reinterpret_cast<size_t>(m_data.m_data) & 2) == 0);

You didn't write this code, but for the record and for else reading this patch,
the correct type to use here is uintptr_t (or intptr_t) rather than size_t. And
also, it's a very strange thing to test. Maybe the person writing it meant to
and with 1 or with 3. I don't understand why it's testing only the 2 bit.

> +#include <QPainter>
> +#include <QPixmap>
> +
> +#include "OwnPtr.h"

We'd normally put these includes into a single paragraph, not two paragraphs.

r=me