Bug 71856 – WebKit should use new NSWindowDidChangeBackingPropertiesNotification instead of old NSWindowDidChangeResolutionNotification

• Component: Platform
• Status: RESOLVED FIXED

[Beth Dakin]

WebKit currently uses NSWindowDidChangeResolutionNotification to determine when a WebView/WKView's window's backing scale factor has changed. Going forward, we should use a new notification called NSWindowDidChangeBackingPropertiesNotification instead.

<rdar://problem/10317253>

[Beth Dakin]

Created attachment 114154 [details]
Patch

[Timothy Hatcher]

Comment on attachment 114154 [details]
Patch

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

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1892
> +    CGFloat oldBackingScaleFactor = [[notification.userInfo objectForKey:backingPropertyOldScaleFactorKey] doubleValue]; 
> +    CGFloat newBackingScaleFactor = [self _intrinsicDeviceScaleFactor]; 
> +    if (oldBackingScaleFactor == newBackingScaleFactor) 
> +        return; 
> +
> +    _data->_page->setIntrinsicDeviceScaleFactor(newBackingScaleFactor);

Wouldn't Page::setIntrinsicDeviceScaleFactor know the old scale factor, meaning you wouldn't need to check here?

> Source/WebKit/mac/WebView/WebView.mm:3382
> +    CGFloat oldBackingScaleFactor = [[notification.userInfo objectForKey:backingPropertyOldScaleFactorKey] doubleValue]; 
> +    CGFloat newBackingScaleFactor = [self _deviceScaleFactor];
> +    if (oldBackingScaleFactor == newBackingScaleFactor) 
> +        return; 
> +
> +    _private->page->setDeviceScaleFactor(newBackingScaleFactor);

Wouldn't Page::setDeviceScaleFactor know the old scale factor, meaning you wouldn't need to check here?

[Beth Dakin]

(In reply to comment #2)
> (From update of attachment 114154 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=114154&action=review
> 
> > Source/WebKit2/UIProcess/API/mac/WKView.mm:1892
> > +    CGFloat oldBackingScaleFactor = [[notification.userInfo objectForKey:backingPropertyOldScaleFactorKey] doubleValue]; 
> > +    CGFloat newBackingScaleFactor = [self _intrinsicDeviceScaleFactor]; 
> > +    if (oldBackingScaleFactor == newBackingScaleFactor) 
> > +        return; 
> > +
> > +    _data->_page->setIntrinsicDeviceScaleFactor(newBackingScaleFactor);
> 
> Wouldn't Page::setIntrinsicDeviceScaleFactor know the old scale factor, meaning you wouldn't need to check here?
> 
> > Source/WebKit/mac/WebView/WebView.mm:3382
> > +    CGFloat oldBackingScaleFactor = [[notification.userInfo objectForKey:backingPropertyOldScaleFactorKey] doubleValue]; 
> > +    CGFloat newBackingScaleFactor = [self _deviceScaleFactor];
> > +    if (oldBackingScaleFactor == newBackingScaleFactor) 
> > +        return; 
> > +
> > +    _private->page->setDeviceScaleFactor(newBackingScaleFactor);
> 
> Wouldn't Page::setDeviceScaleFactor know the old scale factor, meaning you wouldn't need to check here?

Good question. It is true in both WebKit 1 and WebKit 2 that these setters know the old deviceScaleFactor, so it's not really necessary to return early before we call the setters. It's a little nice in the WebKit 1 case in particular, because doing the early return prevents WebKit from calling down into WebCore, which is admittedly not a really big deal, but it seems like a reasonable thing to avoid. Hmm. I don't know. I think I will leave the early returns in for the time being, but you do have me wondering if they are excessive…

[Beth Dakin]

Committed change with revision 99617.