[webkit-reviews] review denied: [Bug 20469] Close leak of PausedTimeouts, and clean up ownership : [Attachment 22908] First attempt at PausedTimeouts cleanup

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Eric Seidel <eric at webkit.org>'s
request for review:
Bug 20469: Close leak of PausedTimeouts, and clean up ownership
https://bugs.webkit.org/show_bug.cgi?id=20469

Attachment 22908: First attempt at PausedTimeouts cleanup
https://bugs.webkit.org/attachment.cgi?id=22908&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Good change!

+    void resumeTimeouts(OwnPtr<PausedTimeouts>&);

You're declaring this in ScriptController.h, but I don't see an implemention in
ScriptController.cpp.

-    size_t count = m_deferredFrames.size();
-    for (size_t i = 0; i < count; ++i)
+    for (size_t i = 0; i < m_deferredFrames.size(); ++i)

Why this change? It makes the loop potentially slower by putting the calls to
size() inside the loop instead of outside. Is there a benefit to the change?

+	 OwnPtr<PausedTimeouts> timeouts = m_pausedTimeouts[i].second;

+	     OwnPtr<PausedTimeouts> timeouts(m_pausedTimeouts.take(frame));

Why did you use assignment syntax in the first case, but not the second.
Consistent would be better.

I'm going to say review- because of the resumeTimeouts issue. I think the patch
won't build!