[webkit-reviews] review granted: [Bug 17172] Refactor platform checks in ScrollView.h : [Attachment 18957] updated patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jan Alonzo <jmalonzo at gmail.com>'s
request for review:
Bug 17172: Refactor platform checks in ScrollView.h
http://bugs.webkit.org/show_bug.cgi?id=17172

Attachment 18957: updated patch
http://bugs.webkit.org/attachment.cgi?id=18957&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
r=me

+IntRect ScrollView::contentsToWindow(const IntRect& rect) const
+{
+    return IntRect(contentsToWindow(rect.location()), rect.size());
+}
+
+IntRect ScrollView::windowToContents(const IntRect& rect) const
+{
+    return IntRect(windowToContents(rect.location()), rect.size());
+}

It's unfortunate we now have 3 copies of these functions. I would have
preferred a single copy either in a shared ScrollView.cpp file with a proper
ifdef or as an inline in the header file (not in the class definition, though).
I feel the same way about other copied and pasted code that's in multiple
platform files.

-    return &m_data->children;
+    return &(m_data->children);

I personally don't think this change is an improvement.