Bug 129438 – Add highDPI support to DumpRenderTree/WebKitTestRunner without the need of reloading the test case.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

ssia

[alan]

Created attachment 225397 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 225397 [details]
Patch

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

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:1445
> +static void changeOffscreenWindowScaleIfNeeded()

I don't think the "offscreen" here is useful.

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:1448
> +    bool needsHighDPIOffscreenWindow = (gTestRunner->testPathOrURL().find("highDPI-") != string::npos);

This should share code with changeOffscreenWindowScaleIfNeeded(). Please don't do the patch check in two places.

> Tools/WebKitTestRunner/TestInvocation.cpp:132
> +    bool needsHighDPIOffscreenWindow = strstr(pathOrURL, "highDPI-");

I think you should do a case insensitive check. Many will be running on case-insensitive file systems.

> Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:256
> +    // Changing the scaling factor on the window does not trigger NSWindowDidChangeBackingPropertiesNotification. We need to send the notification off manually.

s/off//

> Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:257
> +    NSMutableDictionary *notificationUserInfo = [[NSMutableDictionary alloc] initWithCapacity:1];

RetainPtr?

[alan]

Created attachment 225409 [details]
Patch

[alan]

(In reply to comment #2)
> (From update of attachment 225397 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=225397&action=review
> 
> > Tools/DumpRenderTree/mac/DumpRenderTree.mm:1445
> > +static void changeOffscreenWindowScaleIfNeeded()
> 
> I don't think the "offscreen" here is useful.
done.

> 
> > Tools/DumpRenderTree/mac/DumpRenderTree.mm:1448
> > +    bool needsHighDPIOffscreenWindow = (gTestRunner->testPathOrURL().find("highDPI-") != string::npos);
> 
> This should share code with changeOffscreenWindowScaleIfNeeded(). Please don't do the patch check in two places.
DumpRenderTree/WebKitTestRunner.

> 
> > Tools/WebKitTestRunner/TestInvocation.cpp:132
> > +    bool needsHighDPIOffscreenWindow = strstr(pathOrURL, "highDPI-");
> 
> I think you should do a case insensitive check. Many will be running on case-insensitive file systems.
> 
done.

> > Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:256
> > +    // Changing the scaling factor on the window does not trigger NSWindowDidChangeBackingPropertiesNotification. We need to send the notification off manually.
> 
> s/off//
done.

> 
> > Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:257
> > +    NSMutableDictionary *notificationUserInfo = [[NSMutableDictionary alloc] initWithCapacity:1];
> 
> RetainPtr?
done.

[alan]

Created attachment 225419 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 225419 [details]
Patch

Clearing flags on attachment: 225419

Committed r164859: <http://trac.webkit.org/changeset/164859>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 129483

[alan]

Created attachment 225478 [details]
Patch

[alan]

Comment on attachment 225478 [details]
Patch

EWS testing

[WebKit Commit Bot]

Comment on attachment 225478 [details]
Patch

Clearing flags on attachment: 225478

Committed r164882: <http://trac.webkit.org/changeset/164882>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.