Bug 59741

Summary: Chromium Mac: Add scrollbar overlay drawing functions
Product: WebKit Reporter: Sailesh Agrawal <sail>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, jamesr, mihaip, rsesek, thakis
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 59728    
Attachments:
Description Flags
Patch
none
Patch
mihaip: review-
Addressed review comments
none
Fixed some bugs
none
Updated change log. none

Description Sailesh Agrawal 2011-04-28 14:36:53 PDT
Chromium Mac: Add scrollbar overlay drawing functions
Comment 1 Sailesh Agrawal 2011-04-28 14:38:05 PDT
Created attachment 91560 [details]
Patch
Comment 2 Sailesh Agrawal 2011-04-28 14:55:11 PDT
Created attachment 91562 [details]
Patch
Comment 3 Nico Weber 2011-04-28 15:04:36 PDT
Comment on attachment 91562 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=91562&action=review

I think this looks fine. I'm not a reviewer, though.

> Source/WebCore/platform/chromium/ScrollbarOverlayUtilitiesMac.mm:38
> +// we lookup at runtime. If the private APIs don't exist then the wkMake*

s/lookup/look up/
Comment 4 Mihai Parparita 2011-04-28 16:17:32 PDT
Comment on attachment 91562 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=91562&action=review

> Source/WebCore/platform/chromium/ScrollbarOverlayUtilitiesMac.mm:36
> +// This file contains utilities to draw overlay scrollbars. There are no plublic

s/plublic/public/.

> Source/WebCore/platform/chromium/ScrollbarOverlayUtilitiesMac.mm:106
> +static Class LookUpNSScrollerImpClass() {

There's a quite a few WebKit style issues here:
- opening brace should be on its own line for function definitions.
- The first letter does not need to be capitalized.
- Indents should be 4 spaces.
Comment 5 Sailesh Agrawal 2011-04-28 16:27:04 PDT
Created attachment 91585 [details]
Addressed review comments
Comment 6 Sailesh Agrawal 2011-04-28 16:27:45 PDT
Comment on attachment 91562 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=91562&action=review

>> Source/WebCore/platform/chromium/ScrollbarOverlayUtilitiesMac.mm:36
>> +// This file contains utilities to draw overlay scrollbars. There are no plublic
> 
> s/plublic/public/.

Fixed.

>> Source/WebCore/platform/chromium/ScrollbarOverlayUtilitiesMac.mm:38
>> +// we lookup at runtime. If the private APIs don't exist then the wkMake*
> 
> s/lookup/look up/

Fixed.

>> Source/WebCore/platform/chromium/ScrollbarOverlayUtilitiesMac.mm:106
>> +static Class LookUpNSScrollerImpClass() {
> 
> There's a quite a few WebKit style issues here:
> - opening brace should be on its own line for function definitions.
> - The first letter does not need to be capitalized.
> - Indents should be 4 spaces.

Fixed.
Comment 7 James Robinson 2011-04-28 16:38:37 PDT
Mark the patch review? if you would like it reviewed.  Also, when uploading a new patch you should obsolete any patches it replaces.

webkit-patch upload will take care of this for you.
Comment 8 Sailesh Agrawal 2011-04-29 14:00:15 PDT
Created attachment 91727 [details]
Fixed some bugs
Comment 9 Sailesh Agrawal 2011-04-29 14:14:00 PDT
Created attachment 91735 [details]
Updated change log.
Comment 10 WebKit Commit Bot 2011-05-02 10:56:01 PDT
Comment on attachment 91735 [details]
Updated change log.

Clearing flags on attachment: 91735

Committed r85492: <http://trac.webkit.org/changeset/85492>
Comment 11 WebKit Commit Bot 2011-05-02 10:56:07 PDT
All reviewed patches have been landed.  Closing bug.