[webkit-reviews] review denied: [Bug 17483] Implement Scrollbars on Windows (Cairo) : [Attachment 19440] Update based on aroben's comments

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied Brent Fulgham
<bfulgham at gmail.com>'s request for review:
Bug 17483: Implement Scrollbars on Windows (Cairo)
http://bugs.webkit.org/show_bug.cgi?id=17483

Attachment 19440: Update based on aroben's comments
http://bugs.webkit.org/attachment.cgi?id=19440&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
+static int cHorizontalWidth = 17;
+static int cHorizontalHeight = 17;
+static int cVerticalWidth = 17;
+static int cVerticalHeight = 17;
+static int cRealButtonLength = 28;
+static int cButtonInset = 14;
+static int cButtonHitInset = 3;

I don't think there's any reason to initialize these if you're just going to
get the real values later.

+    , m_ButtonLength (14)

There shouldn't be a space before the parentheses here.

+    cHorizontalHeight = ::GetSystemMetrics(SM_CYHSCROLL);
+    cHorizontalWidth = ::GetSystemMetrics(SM_CXHSCROLL);
+    cVerticalHeight = ::GetSystemMetrics(SM_CYVSCROLL);
+    cVerticalWidth = ::GetSystemMetrics(SM_CXVSCROLL);
+    cThumbWidth = ::GetSystemMetrics(SM_CXHTHUMB);
+    cThumbHeight = ::GetSystemMetrics(SM_CYVTHUMB);

These should only need to be initialized once (though really we should listen
for WM_SETTINGSCHANGE and refetch the values then, but that's for another day).


+    m_ButtonLength = ::GetSystemMetrics((orientation == VerticalScrollbar) ?
SM_CYVSCROLL : SM_CXHSCROLL);

I think you can just have two more static variables, cHorizontalButtonLength
and cVerticalButtonLength. Then you won't need this new member.

Once these are fixed I think this'll be good to go.