Bug 153969

Summary: Use modern SPI header idiom for NSScrollerImp and NSScrollerImpPair
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, commit-queue, jamesr, luiz, tonikitoo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mitz: review+

Description Sam Weinig 2016-02-07 18:37:54 PST
Use modern SPI header idiom for NSScrollerImp and NSScrollerImpPair
Comment 1 Sam Weinig 2016-02-07 18:45:34 PST
Created attachment 270835 [details]
Patch
Comment 2 WebKit Commit Bot 2016-02-07 18:48:02 PST
Attachment 270835 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:37:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:54:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:57:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:59:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:62:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:68:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:69:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:70:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:71:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:72:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:73:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:74:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:75:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:76:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:77:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:87:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:109:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:110:  Should have space between @property and attributes.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:111:  Should have space between @property and attributes.  [whitespace/property] [4]
Total errors found: 19 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 mitz 2016-02-07 18:53:19 PST
Comment on attachment 270835 [details]
Patch

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

> Source/WebCore/platform/mac/ScrollbarThemeMac.mm:144
>  static bool supportsExpandedScrollbars()
>  {
>      // FIXME: This is temporary until all platforms that support ScrollbarPainter support this part of the API.
> -    static bool globalSupportsExpandedScrollbars = [NSClassFromString(@"NSScrollerImp") instancesRespondToSelector:@selector(setExpanded:)];
> +    static bool globalSupportsExpandedScrollbars = [[NSScrollerImp class] instancesRespondToSelector:@selector(setExpanded:)];
>      return globalSupportsExpandedScrollbars;
>  }

I think this isn’t necessary anymore.

> Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:36
> +@interface NSScrollerImp (WebCoreIPI)

We now use class extensions rather than categories in such cases.

> Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:88
> +@interface NSScrollerImp (WebCoreIPI)
> +@property(getter=isTracking) BOOL tracking;
> +@end

Ditto.
Comment 4 Sam Weinig 2016-02-07 18:56:00 PST
(In reply to comment #3)
> Comment on attachment 270835 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=270835&action=review
> 
> > Source/WebCore/platform/mac/ScrollbarThemeMac.mm:144
> >  static bool supportsExpandedScrollbars()
> >  {
> >      // FIXME: This is temporary until all platforms that support ScrollbarPainter support this part of the API.
> > -    static bool globalSupportsExpandedScrollbars = [NSClassFromString(@"NSScrollerImp") instancesRespondToSelector:@selector(setExpanded:)];
> > +    static bool globalSupportsExpandedScrollbars = [[NSScrollerImp class] instancesRespondToSelector:@selector(setExpanded:)];
> >      return globalSupportsExpandedScrollbars;
> >  }
> 
> I think this isn’t necessary anymore.

Agreed. There are a few of these I will clean up in a second pass.

> 
> > Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:36
> > +@interface NSScrollerImp (WebCoreIPI)
> 
> We now use class extensions rather than categories in such cases.
> 
> > Source/WebCore/platform/spi/mac/NSScrollerImpSPI.h:88
> > +@interface NSScrollerImp (WebCoreIPI)
> > +@property(getter=isTracking) BOOL tracking;
> > +@end
> 
> Ditto.

Ok.
Comment 5 Sam Weinig 2016-02-07 19:01:08 PST
Committed r196246: <http://trac.webkit.org/changeset/196246>