122152 2013-10-01 03:37:38 -0700 [mac] 1x emulation on Retina hardware shouldn't use SPI 2013-10-01 13:10:50 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 122006 1 thorton thorton andersca ap rniwa webkit-bug-importer oldest_to_newest 934958 0 thorton 2013-10-01 03:37:38 -0700 There's a NSHighResolutionCapable Info.plist key that is much more official than the SPI we're using. This will un-break some things re: snapshotting down the road. 934959 1 webkit-bug-importer 2013-10-01 03:37:48 -0700 <rdar://problem/15119384> 935030 2 ap 2013-10-01 08:57:29 -0700 Will using it break the few HiDPI tests we have? 935035 3 thorton 2013-10-01 09:02:39 -0700 (In reply to comment #2) > Will using it break the few HiDPI tests we have? It shouldn't; they change the deviceScaleFactor at a purely WebKit level. 935107 4 213102 thorton 2013-10-01 11:39:56 -0700 Created attachment 213102 patch 935112 5 thorton 2013-10-01 11:45:43 -0700 http://trac.webkit.org/changeset/156724 935148 6 213102 ap 2013-10-01 12:51:26 -0700 Comment on attachment 213102 patch View in context: https://bugs.webkit.org/attachment.cgi?id=213102&action=review > Tools/DumpRenderTree/mac/DumpRenderTree.mm:954 > + NSDictionary *defaults = [[NSDictionary alloc] initWithObjectsAndKeys:[NSNumber numberWithBool:YES], @"AppleMagnifiedMode", nil]; @{@"AppleMagnifiedMode": @YES} > Tools/DumpRenderTree/mac/DumpRenderTree.mm:955 > + [[NSUserDefaults standardUserDefaults] registerDefaults:defaults]; Registering defaults is not what we need here. A persistent preference stuck somewhere would override a registered default, and it's inconsistent with what we do in resetDefaultsToConsistentValues(). We just use regular setBool:forKey:. Is it important to set this default before entering NSApplication? Otherwise, you could just add this to resetDefaultsToConsistentValues(). > Tools/WebKitTestRunner/mac/main.mm:38 > + NSDictionary *defaults = [[NSDictionary alloc] initWithObjectsAndKeys:[NSNumber numberWithBool:YES], @"AppleMagnifiedMode", nil]; > + [[NSUserDefaults standardUserDefaults] registerDefaults:defaults]; > + [defaults release]; Same comment about registerDefaults:. We already had two styles of setting defaults in this function, which was one more than we should have. Do we only want AppleMagnifiedMode set in UI process? 935155 7 thorton 2013-10-01 12:59:24 -0700 (In reply to comment #6) > (From update of attachment 213102 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=213102&action=review > > > Tools/DumpRenderTree/mac/DumpRenderTree.mm:954 > > + NSDictionary *defaults = [[NSDictionary alloc] initWithObjectsAndKeys:[NSNumber numberWithBool:YES], @"AppleMagnifiedMode", nil]; > > @{@"AppleMagnifiedMode": @YES} I wasn't sure if I could use this here, but Anders said yes, so I will switch it up. > > Tools/DumpRenderTree/mac/DumpRenderTree.mm:955 > > + [[NSUserDefaults standardUserDefaults] registerDefaults:defaults]; > > Registering defaults is not what we need here. A persistent preference stuck somewhere would override a registered default, and it's inconsistent with what we do in resetDefaultsToConsistentValues(). We just use regular setBool:forKey:. > > Is it important to set this default before entering NSApplication? Otherwise, you could just add this to resetDefaultsToConsistentValues(). > > > Tools/WebKitTestRunner/mac/main.mm:38 > > + NSDictionary *defaults = [[NSDictionary alloc] initWithObjectsAndKeys:[NSNumber numberWithBool:YES], @"AppleMagnifiedMode", nil]; > > + [[NSUserDefaults standardUserDefaults] registerDefaults:defaults]; > > + [defaults release]; > > Same comment about registerDefaults:. We already had two styles of setting defaults in this function, which was one more than we should have. Yeah, Dan pointed this out. Apparently setVolatileDomain is the way to go for temporary defaults that override ones up the chain, though I didn't know this at the time. Is there any reason to actually use setBool:forKey: instead of that? > Do we only want AppleMagnifiedMode set in UI process? We only need it set in the UI process. 935163 8 ap 2013-10-01 13:10:50 -0700 > Is there any reason to actually use setBool:forKey: instead of that? You can only set any volatile domain once, I think (I assume that repeated calls will replace its content). In fact, I'm not sure if what we are doing between tests in WebKitTestRunner is right. 213102 2013-10-01 11:39:56 -0700 2013-10-01 12:51:26 -0700 patch reso.diff text/plain 5077 thorton ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCAxM2Y2 YjA4Li4yMWQ0NmFhIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjIgQEAKKzIwMTMtMTAtMDEgIFRpbSBIb3J0b24gIDx0aW1vdGh5 X2hvcnRvbkBhcHBsZS5jb20+CisKKyAgICAgICAgW21hY10gMXggZW11bGF0aW9uIG9uIFJldGlu YSBoYXJkd2FyZSBzaG91bGRuJ3QgdXNlIFNQSQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTIyMTUyCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xNTEx OTM4ND4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAq IER1bXBSZW5kZXJUcmVlL21hYy9EdW1wUmVuZGVyVHJlZS5tbToKKyAgICAgICAgKGNyZWF0ZVdl YlZpZXdBbmRPZmZzY3JlZW5XaW5kb3cpOgorICAgICAgICAobWFpbik6CisgICAgICAgICogRHVt cFJlbmRlclRyZWUvbWFjL0R1bXBSZW5kZXJUcmVlV2luZG93Lmg6CisgICAgICAgICogRHVtcFJl bmRlclRyZWUvbWFjL0R1bXBSZW5kZXJUcmVlV2luZG93Lm1tOgorICAgICAgICAqIFdlYktpdFRl c3RSdW5uZXIvbWFjL1BsYXRmb3JtV2ViVmlld01hYy5tbToKKyAgICAgICAgKFdUUjo6UGxhdGZv cm1XZWJWaWV3OjpQbGF0Zm9ybVdlYlZpZXcpOgorICAgICAgICAqIFdlYktpdFRlc3RSdW5uZXIv bWFjL21haW4ubW06CisgICAgICAgIChtYWluKToKKyAgICAgICAgUmVnaXN0ZXIgZm9yIEFwcGxl TWFnbmlmaWVkTW9kZSBpbnN0ZWFkIG9mIHVzaW5nIF9zZXRXaW5kb3dSZXNvbHV0aW9uOmRpc3Bs YXlJZkNoYW5nZWQ6LgorCiAyMDEzLTA5LTMwICBTYW0gV2VpbmlnICA8c2FtQHdlYmtpdC5vcmc+ CiAKICAgICAgICAgY2hlY2std2Via2l0LXN0eWxlIHNob3VsZCBub3Qgd2FybiBhYm91dCB1c2Ug b2YgQysrMTEg4oCcPj7igJ0gIGluc3RlYWQgb2Yg4oCcPiA+IgpkaWZmIC0tZ2l0IGEvVG9vbHMv RHVtcFJlbmRlclRyZWUvbWFjL0R1bXBSZW5kZXJUcmVlLm1tIGIvVG9vbHMvRHVtcFJlbmRlclRy ZWUvbWFjL0R1bXBSZW5kZXJUcmVlLm1tCmluZGV4IDI1ZTNmNWQuLjNmNmZjZTYgMTAwNjQ0Ci0t LSBhL1Rvb2xzL0R1bXBSZW5kZXJUcmVlL21hYy9EdW1wUmVuZGVyVHJlZS5tbQorKysgYi9Ub29s cy9EdW1wUmVuZGVyVHJlZS9tYWMvRHVtcFJlbmRlclRyZWUubW0KQEAgLTU3Myw3ICs1NzMsNiBA QCBXZWJWaWV3ICpjcmVhdGVXZWJWaWV3QW5kT2Zmc2NyZWVuV2luZG93KCkKICAgICBbW3dpbmRv dyBjb250ZW50Vmlld10gYWRkU3Vidmlldzp3ZWJWaWV3XTsKICAgICBbd2luZG93IG9yZGVyQmFj azpuaWxdOwogICAgIFt3aW5kb3cgc2V0QXV0b2Rpc3BsYXk6Tk9dOwotICAgIFt3aW5kb3cgX3Nl dFdpbmRvd1Jlc29sdXRpb246MSBkaXNwbGF5SWZDaGFuZ2VkOllFU107CiAKICAgICBbd2luZG93 IHN0YXJ0TGlzdGVuaW5nRm9yQWNjZWxlcmF0ZWRDb21wb3NpdGluZ0NoYW5nZXNdOwogICAgIApA QCAtOTUxLDYgKzk1MCwxMSBAQCBpbnQgbWFpbihpbnQgYXJnYywgY29uc3QgY2hhciAqYXJndltd KQogewogICAgIE5TQXV0b3JlbGVhc2VQb29sICpwb29sID0gW1tOU0F1dG9yZWxlYXNlUG9vbCBh bGxvY10gaW5pdF07CiAgICAgW0R1bXBSZW5kZXJUcmVlQXBwbGljYXRpb24gc2hhcmVkQXBwbGlj YXRpb25dOyAvLyBGb3JjZSBBcHBLaXQgdG8gaW5pdCBpdHNlbGYKKyAgICAKKyAgICBOU0RpY3Rp b25hcnkgKmRlZmF1bHRzID0gW1tOU0RpY3Rpb25hcnkgYWxsb2NdIGluaXRXaXRoT2JqZWN0c0Fu ZEtleXM6W05TTnVtYmVyIG51bWJlcldpdGhCb29sOllFU10sIEAiQXBwbGVNYWduaWZpZWRNb2Rl IiwgbmlsXTsKKyAgICBbW05TVXNlckRlZmF1bHRzIHN0YW5kYXJkVXNlckRlZmF1bHRzXSByZWdp c3RlckRlZmF1bHRzOmRlZmF1bHRzXTsKKyAgICBbZGVmYXVsdHMgcmVsZWFzZV07CisgICAgCiAg ICAgZHVtcFJlbmRlclRyZWUoYXJnYywgYXJndik7CiAgICAgW1dlYkNvcmVTdGF0aXN0aWNzIGdh cmJhZ2VDb2xsZWN0SmF2YVNjcmlwdE9iamVjdHNdOwogICAgIFtXZWJDb3JlU3RhdGlzdGljcyBl bXB0eUNhY2hlXTsgLy8gT3RoZXJ3aXNlIFNWR0ltYWdlcyB0cmlnZ2VyIGZhbHNlIHBvc2l0aXZl cyBmb3IgRnJhbWUvTm9kZSBjb3VudHMKZGlmZiAtLWdpdCBhL1Rvb2xzL0R1bXBSZW5kZXJUcmVl L21hYy9EdW1wUmVuZGVyVHJlZVdpbmRvdy5oIGIvVG9vbHMvRHVtcFJlbmRlclRyZWUvbWFjL0R1 bXBSZW5kZXJUcmVlV2luZG93LmgKaW5kZXggM2IxZDc3ZS4uYTIyOWQyMCAxMDA2NDQKLS0tIGEv VG9vbHMvRHVtcFJlbmRlclRyZWUvbWFjL0R1bXBSZW5kZXJUcmVlV2luZG93LmgKKysrIGIvVG9v bHMvRHVtcFJlbmRlclRyZWUvbWFjL0R1bXBSZW5kZXJUcmVlV2luZG93LmgKQEAgLTMyLDEyICsz Miw2IEBACiAKIEBjbGFzcyBXZWJWaWV3OwogCi1AaW50ZXJmYWNlIE5TV2luZG93IChEZXRhaWxz KQotCi0tICh2b2lkKV9zZXRXaW5kb3dSZXNvbHV0aW9uOihDR0Zsb2F0KXJlc29sdXRpb24gZGlz cGxheUlmQ2hhbmdlZDooQk9PTClkaXNwbGF5SWZDaGFuZ2VkOwotCi1AZW5kCi0KIEBpbnRlcmZh Y2UgRHVtcFJlbmRlclRyZWVXaW5kb3cgOiBOU1dpbmRvdwogewogfQpkaWZmIC0tZ2l0IGEvVG9v bHMvRHVtcFJlbmRlclRyZWUvbWFjL0R1bXBSZW5kZXJUcmVlV2luZG93Lm1tIGIvVG9vbHMvRHVt cFJlbmRlclRyZWUvbWFjL0R1bXBSZW5kZXJUcmVlV2luZG93Lm1tCmluZGV4IDRhNmZkNmIuLjE3 YTExNGIgMTAwNjQ0Ci0tLSBhL1Rvb2xzL0R1bXBSZW5kZXJUcmVlL21hYy9EdW1wUmVuZGVyVHJl ZVdpbmRvdy5tbQorKysgYi9Ub29scy9EdW1wUmVuZGVyVHJlZS9tYWMvRHVtcFJlbmRlclRyZWVX aW5kb3cubW0KQEAgLTEyMCw5ICsxMjAsNCBAQCBzdGF0aWMgQ0ZBcnJheUNhbGxCYWNrcyBOb25S ZXRhaW5pbmdBcnJheUNhbGxiYWNrcyA9IHsKICAgICAgICAgW3NlbGYgc2V0QXV0b2Rpc3BsYXk6 WUVTXTsKIH0KIAotLSAoQ0dGbG9hdCliYWNraW5nU2NhbGVGYWN0b3IKLXsKLSAgICByZXR1cm4g MTsKLX0KLQogQGVuZApkaWZmIC0tZ2l0IGEvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9tYWMvUGxh dGZvcm1XZWJWaWV3TWFjLm1tIGIvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9tYWMvUGxhdGZvcm1X ZWJWaWV3TWFjLm1tCmluZGV4IGJhOGQ3ZjUuLmFlYjgyNDQgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1dl YktpdFRlc3RSdW5uZXIvbWFjL1BsYXRmb3JtV2ViVmlld01hYy5tbQorKysgYi9Ub29scy9XZWJL aXRUZXN0UnVubmVyL21hYy9QbGF0Zm9ybVdlYlZpZXdNYWMubW0KQEAgLTExMSwxNyArMTExLDYg QEAgdXNpbmcgbmFtZXNwYWNlIFdUUjsKICAgICByZXR1cm4gTlNNYWtlUmVjdChfZmFrZU9yaWdp bi54LCBfZmFrZU9yaWdpbi55LCBjdXJyZW50RnJhbWUuc2l6ZS53aWR0aCwgY3VycmVudEZyYW1l LnNpemUuaGVpZ2h0KTsKIH0KIAotLSAoQ0dGbG9hdCliYWNraW5nU2NhbGVGYWN0b3IKLXsKLSAg ICByZXR1cm4gMTsKLX0KLQotQGVuZAotCi1AaW50ZXJmYWNlIE5TV2luZG93IChEZXRhaWxzKQot Ci0tICh2b2lkKV9zZXRXaW5kb3dSZXNvbHV0aW9uOihDR0Zsb2F0KXJlc29sdXRpb24gZGlzcGxh eUlmQ2hhbmdlZDooQk9PTClkaXNwbGF5SWZDaGFuZ2VkOwotCiBAZW5kCiAKIG5hbWVzcGFjZSBX VFIgewpAQCAtMTUwLDcgKzEzOSw2IEBAIFBsYXRmb3JtV2ViVmlldzo6UGxhdGZvcm1XZWJWaWV3 KFdLQ29udGV4dFJlZiBjb250ZXh0UmVmLCBXS1BhZ2VHcm91cFJlZiBwYWdlR3JvCiAgICAgW1tt X3dpbmRvdyBjb250ZW50Vmlld10gYWRkU3VidmlldzptX3ZpZXddOwogICAgIFttX3dpbmRvdyBv cmRlckJhY2s6bmlsXTsKICAgICBbbV93aW5kb3cgc2V0UmVsZWFzZWRXaGVuQ2xvc2VkOk5PXTsK LSAgICBbbV93aW5kb3cgX3NldFdpbmRvd1Jlc29sdXRpb246MSBkaXNwbGF5SWZDaGFuZ2VkOllF U107CiB9CiAKIHZvaWQgUGxhdGZvcm1XZWJWaWV3OjpyZXNpemVUbyh1bnNpZ25lZCB3aWR0aCwg dW5zaWduZWQgaGVpZ2h0KQpkaWZmIC0tZ2l0IGEvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9tYWMv bWFpbi5tbSBiL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvbWFjL21haW4ubW0KaW5kZXggODYzN2Nm Yi4uZjIyOTZkNCAxMDA2NDQKLS0tIGEvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9tYWMvbWFpbi5t bQorKysgYi9Ub29scy9XZWJLaXRUZXN0UnVubmVyL21hYy9tYWluLm1tCkBAIC0zMiw2ICszMiwx MCBAQCBpbnQgbWFpbihpbnQgYXJnYywgY29uc3QgY2hhciogYXJndltdKQogICAgIE5TQXV0b3Jl bGVhc2VQb29sICpwb29sID0gW1tOU0F1dG9yZWxlYXNlUG9vbCBhbGxvY10gaW5pdF07CiAgICAg W05TQXBwbGljYXRpb24gc2hhcmVkQXBwbGljYXRpb25dOwogICAgIFtbTlNVc2VyRGVmYXVsdHMg c3RhbmRhcmRVc2VyRGVmYXVsdHNdIHNldFZvbGF0aWxlRG9tYWluOltOU0RpY3Rpb25hcnkgZGlj dGlvbmFyeVdpdGhPYmplY3Q6W05TTnVtYmVyIG51bWJlcldpdGhCb29sOk5PXSBmb3JLZXk6QCJX ZWJLaXRLZXJuaW5nQW5kTGlnYXR1cmVzRW5hYmxlZEJ5RGVmYXVsdCJdIGZvck5hbWU6TlNBcmd1 bWVudERvbWFpbl07CisgICAgCisgICAgTlNEaWN0aW9uYXJ5ICpkZWZhdWx0cyA9IFtbTlNEaWN0 aW9uYXJ5IGFsbG9jXSBpbml0V2l0aE9iamVjdHNBbmRLZXlzOltOU051bWJlciBudW1iZXJXaXRo Qm9vbDpZRVNdLCBAIkFwcGxlTWFnbmlmaWVkTW9kZSIsIG5pbF07CisgICAgW1tOU1VzZXJEZWZh dWx0cyBzdGFuZGFyZFVzZXJEZWZhdWx0c10gcmVnaXN0ZXJEZWZhdWx0czpkZWZhdWx0c107Cisg ICAgW2RlZmF1bHRzIHJlbGVhc2VdOwogCiAgICAgW1tOU1VzZXJEZWZhdWx0cyBzdGFuZGFyZFVz ZXJEZWZhdWx0c10gc2V0Qm9vbDpOTyBmb3JLZXk6QCJXZWJBdXRvbWF0aWNRdW90ZVN1YnN0aXR1 dGlvbkVuYWJsZWQiXTsKICAgICBbW05TVXNlckRlZmF1bHRzIHN0YW5kYXJkVXNlckRlZmF1bHRz XSBzZXRCb29sOk5PIGZvcktleTpAIldlYkF1dG9tYXRpY0Rhc2hTdWJzdGl0dXRpb25FbmFibGVk Il07Cg==