PlatformScreenMac should not rely on NSWindow for important bits of data

• Component: Platform
• Status: RESOLVED FIXED

[Beth Dakin]

PlatformScreenMac assumes it can reliably get an NSWindow from the platformWidget(), which is not true in WebKit2. It should be updated so that these functions work properly in WebKit2.

<rdar://problem/10741654>

[Beth Dakin]

Created attachment 124405 [details]
Patch

[Geoffrey Garen]

Comment on attachment 124405 [details]
Patch 

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

r=me

> Source/WebCore/platform/PlatformScreen.h:59
> +    FloatRect toUserSpace(const NSRect&, NSWindow *destination, float deviceScaleFactor);
> +    NSRect toDeviceSpace(const FloatRect&, NSWindow *source, float deviceScaleFactor);

Follow-up bug: It looks like these functions will do the wrong thing in WebKit2 in the case of multiple screens where the window is not on the primary screen, since the NSWindow argument will be NULL.

[Beth Dakin]

Thanks! I will definitely file a follow-up bug for the remaining issue. Committed change with http://trac.webkit.org/changeset/106271

[Adam Roben (:aroben)]

Comment on attachment 124405 [details]
Patch 

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

> Source/WebCore/platform/PlatformScreen.h:43
> +    class FrameView;

It's a layering violation for anything in Source/WebCore/platform to know about FrameView. I'm not sure what the best fix is though.

[Beth Dakin]

(In reply to comment #4)
> (From update of attachment 124405 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=124405&action=review
> 
> > Source/WebCore/platform/PlatformScreen.h:43
> > +    class FrameView;
> 
> It's a layering violation for anything in Source/WebCore/platform to know about FrameView. I'm not sure what the best fix is though.


We could add a virtual function on Widget to return the deviceScaleFactor. It would always return 1 from Widget.h, but FrameViews could fetch the real deviceScaleFactor. That would totally fix the layering violation, but it's a shame that it's such a circuitous path.

[Beth Dakin]

32-bit build fix: http://trac.webkit.org/changeset/106286