[webkit-reviews] review denied: [Bug 105553] [chromium] Convert WebTransformOperations into pure virtual : [Attachment 182575] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Ali Juma <ajuma at chromium.org>'s
request for review:
Bug 105553: [chromium] Convert WebTransformOperations into pure virtual
https://bugs.webkit.org/show_bug.cgi?id=105553

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=182575&action=review


Since this has a chromium-side dep, make sure that lands and is rolled into
Source/WebKit/chromium/DEPS and leave a comment on this bug indicating what the
dependency is so folks know.  This patch has some minor issues, but is looking
close.

> Source/WebCore/platform/graphics/chromium/AnimationTranslationUtil.cpp:60
> +WebTransformOperations* toWebTransformOperations(const TransformOperations&
transformOperations, const FloatSize& boxSize)

since the caller is going to put this in an OwnPtr<>, you should make the
return type of this function PassOwnPtr<> and put the adoptPtr() call in the
initialization of the webTransformOperations local (which you should make
OwnPtr<>).  whenever using smart pointers you want to put it into the smart
pointer as close to the allocation point as you can so less code is dealing
with raw pointers

> Source/WebCore/platform/graphics/chromium/AnimationTranslationUtil.cpp:63
> +    WebTransformOperations* webTransformOperations =
Platform::current()->compositorSupport()->createTransformOperations();

you should check if this call returns 0 and, if so, return 0 from this function
before proceeding. you can't promise here that the platform's
compositingSupport will give you something