[Webkit-unassigned] [Bug 105553] [chromium] Convert WebTransformOperations into pure virtual

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=105553


James Robinson <jamesr at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #182575|review?                     |review-
               Flag|                            |




--- Comment #13 from James Robinson <jamesr at chromium.org>  2013-01-15 20:03:58 PST ---
(From update of attachment 182575)
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

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.