[webkit-reviews] review denied: [Bug 34005] Safari pegs CPU and drops tons of frames using HTML5 Vimeo player : [Attachment 54600] Part 1.1: QTMovieWin: The breakup of QTMovieWin
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Apr 30 18:03:24 PDT 2010
Sam Weinig <sam at webkit.org> has denied Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 34005: Safari pegs CPU and drops tons of frames using HTML5 Vimeo player
https://bugs.webkit.org/show_bug.cgi?id=34005
Attachment 54600: Part 1.1: QTMovieWin: The breakup of QTMovieWin
https://bugs.webkit.org/attachment.cgi?id=54600&action=review
------- Additional Comments from Sam Weinig <sam at webkit.org>
WebCore/platform/graphics/win/QTMovie.cpp:266
+ m_private->m_clients.remove(indexOfClient);
Is this correct? Do you really never want to remove the first client?
WebCore/platform/graphics/win/QTMovie.h:60
+ class QTMOVIEWIN_API QTMovie : public RefCounted<QTMovie> {
It seems like this class should be renamed something like QTMovieView or
QTMovieGWorld. If you don't want to rename it in this round, please add a
comment.
WebCore/platform/graphics/win/QTMovieTask.cpp:46
+ static QTMovieTask* s_sharedTask = new QTMovieTask();
This should use the DEFINE_GLOBAL macro.
r=me but please consider those
changes.WebCore/platform/graphics/win/QTMovieWin.h:75
+ RefPtr<QTMovie> movie() const;
We don't ever pass or return RefPtr's. Please see
http://webkit.org/coding/RefPtr.html for more on that.
More information about the webkit-reviews
mailing list