Bug 150379 - clang-700.0.59.5 finds additional deprecated declarations which breaks the build
Summary: clang-700.0.59.5 finds additional deprecated declarations which breaks the build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Mac OS X 10.11
: P2 Normal
Assignee: Gordon Sheridan
URL:
Keywords:
Depends on: 150384
Blocks:
  Show dependency treegraph
 
Reported: 2015-10-20 17:21 PDT by Gordon Sheridan
Modified: 2015-10-22 15:37 PDT (History)
3 users (show)

See Also:


Attachments
Ignore two additional deprecated-declaration errors found by clang-700.0.59.5. (2.11 KB, patch)
2015-10-20 17:32 PDT, Gordon Sheridan
no flags Details | Formatted Diff | Diff
Fix build for clang-700.0.59.5 by replacing deprecated calls to convertScreenToBase: with convertRectFromScreen:. (2.01 KB, patch)
2015-10-20 18:22 PDT, Gordon Sheridan
no flags Details | Formatted Diff | Diff
Provide convenience functions to encapsulate differences between Mac and iOS to convert points between the screen and a window. (2.83 KB, patch)
2015-10-21 19:19 PDT, Gordon Sheridan
no flags Details | Formatted Diff | Diff
WAKWindow versions of methods to convert an NSRect between window and screen coordinates. (4.41 KB, patch)
2015-10-22 00:46 PDT, Gordon Sheridan
aestes: review-
aestes: commit-queue-
Details | Formatted Diff | Diff
WAKWindow versions of methods to convert an NSRect between window and screen coordinates. (4.59 KB, patch)
2015-10-22 15:21 PDT, Gordon Sheridan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gordon Sheridan 2015-10-20 17:21:59 PDT
There are two uses of -[NSWindow convertBaseToScreen:] in WebHTMLView.mm. This method has been deprecated since 10.7, but the code has been unchanged since 2005.
Comment 1 Gordon Sheridan 2015-10-20 17:32:32 PDT
Created attachment 263640 [details]
Ignore two additional deprecated-declaration errors found by clang-700.0.59.5.
Comment 2 Gordon Sheridan 2015-10-20 18:22:48 PDT
Created attachment 263647 [details]
Fix build for clang-700.0.59.5 by replacing deprecated calls to convertScreenToBase: with convertRectFromScreen:.
Comment 3 WebKit Commit Bot 2015-10-20 19:21:43 PDT
Comment on attachment 263647 [details]
Fix build for clang-700.0.59.5 by replacing deprecated calls to convertScreenToBase: with convertRectFromScreen:.

Clearing flags on attachment: 263647

Committed r191370: <http://trac.webkit.org/changeset/191370>
Comment 4 WebKit Commit Bot 2015-10-20 19:21:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Alexey Proskuryakov 2015-10-20 21:57:49 PDT
This broke iOS 9 build, rolling out.

https://build.webkit.org/builders/Apple%20iOS%209%20Simulator%20Release%20%28Build%29/builds/571/steps/compile-webkit/logs/stdio

EWS has detected this, please check its status in the future.
Comment 6 WebKit Commit Bot 2015-10-20 22:00:51 PDT
Re-opened since this is blocked by bug 150384
Comment 7 Andy Estes 2015-10-21 12:15:45 PDT
For iOS, we need to define -convertRectFromScreen: and -convertRectToScreen: in WAKWindow.h
Comment 8 Gordon Sheridan 2015-10-21 19:19:57 PDT
Created attachment 263782 [details]
Provide convenience functions to encapsulate differences between Mac and iOS to convert points between the screen and a window.
Comment 9 Andy Estes 2015-10-21 21:38:01 PDT
Comment on attachment 263782 [details]
Provide convenience functions to encapsulate differences between Mac and iOS to convert points between the screen and a window.

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

> Source/WebKit/mac/ChangeLog:10
> +        (convertPointFromScreen):
> +        Added convenience function to encapsulate differences in converting points on Mac vs. iOS.

This is ok as a build fix, but can you please add a FIXME saying that WAKWindow should instead implement -convertRectFromScreen:? After all, the whole point of WAKWindow is to avoid having to write code like this.

> Source/WebKit/mac/ChangeLog:13
> +        (convertPointToScreen):
> +        Ditto.

Ditto for -convertRectToScreen:.

> Source/WebKit/mac/WebView/WebHTMLView.mm:6119
> +    NSRect screenRect = { thePoint, NSZeroSize };

thePoint is in the window's coordinate space, so this should be called windowRect.
Comment 10 Andy Estes 2015-10-21 21:40:02 PDT
Comment on attachment 263782 [details]
Provide convenience functions to encapsulate differences between Mac and iOS to convert points between the screen and a window.

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

> Source/WebKit/mac/WebView/WebHTMLView.mm:6104
> +static NSPoint convertPointFromScreen(NSWindow *window, NSPoint thePoint)

Also, a better name for thePoint would be screenPoint.

> Source/WebKit/mac/WebView/WebHTMLView.mm:6114
> +static NSPoint convertPointToScreen(NSWindow *window, NSPoint thePoint)

And a better name here for thePoint would be windowPoint.
Comment 11 Gordon Sheridan 2015-10-22 00:46:07 PDT
Created attachment 263805 [details]
WAKWindow versions of methods to convert an NSRect between window and screen coordinates.

This patch provides WAKWindow versions of the non-deprecated methods for converting an NSRect between window and screen coordinates, which replace the deprecated methods that operated on an NSPoint.
Comment 12 Andy Estes 2015-10-22 14:13:25 PDT
Comment on attachment 263805 [details]
WAKWindow versions of methods to convert an NSRect between window and screen coordinates.

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

I like this a lot better, but I'm not sure the WAKWindow methods are correctly implemented.

> Source/WebCore/platform/ios/wak/WAKWindow.mm:163
> +- (NSRect)convertRectToScreen:(NSRect)windowRect
> +{
> +    windowRect.origin = [self convertBaseToScreen:windowRect.origin];
> +    return windowRect;
> +}

I'm not sure it's okay to convert a rect just by converting its origin. I would copy the implementation of -convertBaseToScreen:, but call -[CALayer convertRect:toLayer:] instead.

> Source/WebCore/platform/ios/wak/WAKWindow.mm:168
> +    screenRect.origin = [self convertScreenToBase:screenRect.origin];
> +    return screenRect;

Ditto for -[CALayer convertRect:fromLayer:].
Comment 13 Gordon Sheridan 2015-10-22 15:21:50 PDT
Created attachment 263873 [details]
WAKWindow versions of methods to convert an NSRect between window and screen coordinates.

Updated to incorporate Andy's suggestions for WAKWindow implementations of convertRectToScreen: and convertRectFromScreen:.
Comment 14 WebKit Commit Bot 2015-10-22 15:37:42 PDT
Comment on attachment 263873 [details]
WAKWindow versions of methods to convert an NSRect between window and screen coordinates.

Clearing flags on attachment: 263873

Committed r191484: <http://trac.webkit.org/changeset/191484>
Comment 15 WebKit Commit Bot 2015-10-22 15:37:49 PDT
All reviewed patches have been landed.  Closing bug.