[webkit-reviews] review denied: [Bug 32870] Add WebAnimationController to Chromium WebKit API : [Attachment 45435] Same as previous, minus tabs that got in

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 23 11:45:04 PST 2009


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Yaar Schnitman
<yaar at chromium.org>'s request for review:
Bug 32870: Add WebAnimationController to Chromium WebKit API
https://bugs.webkit.org/show_bug.cgi?id=32870

Attachment 45435: Same as previous, minus tabs that got in
https://bugs.webkit.org/attachment.cgi?id=45435&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> +++ b/WebKit/chromium/src/WebFrameImpl.cpp
...
> +WebAnimationController* WebFrameImpl::animationController()
> +{
> +    if (!m_animationController.get())
> +	   m_animationController.set(new WebAnimationControllerImpl(this));
> +    return m_animationController.get();
> +}
...
> @@ -1495,6 +1503,7 @@ WebFrameImpl::WebFrameImpl(WebFrameClient* client)
>      , m_framesScopingCount(-1)
>      , m_scopingComplete(false)
>      , m_nextInvalidateAfter(0)
> +    , m_animationController(0)

As previously mentioned, there's no need to initialize an OwnPtr explicitly.
You can just delete this line.	R- because of this.

But, come to think of it, why is m_animationController heap allocated?	Why
not just make it be allocated as an instance variable of WebFrameImpl?


More information about the webkit-reviews mailing list