131976 2014-04-22 01:47:12 -0700 WKThumbnailView doesn't quite work with topContentInset 2014-04-22 08:52:29 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 thorton thorton bdakin simon.fraser oldest_to_newest 1002836 0 thorton 2014-04-22 01:47:12 -0700 <rdar://problem/16642127> 1002837 1 229871 thorton 2014-04-22 01:48:31 -0700 Created attachment 229871 patch 1002892 2 229871 darin 2014-04-22 07:45:04 -0700 Comment on attachment 229871 patch View in context: https://bugs.webkit.org/attachment.cgi?id=229871&action=review > Source/WebKit2/UIProcess/API/Cocoa/_WKThumbnailView.mm:90 > + _lastSnapshotScale = 0; Could consider NaN instead for uses like this. > Source/WebKit2/UIProcess/API/Cocoa/_WKThumbnailView.mm:126 > - IntRect snapshotRect(IntPoint(), _webPageProxy->viewSize()); > + IntRect snapshotRect(IntPoint(), IntSize(_webPageProxy->viewSize().width(), _webPageProxy->viewSize().height() - _webPageProxy->topContentInset())); Is there a way to write this math simpler? Like maybe this: IntRect snapshotRect(IntPoint(), _webPageProxy->viewSize() - IntSize(0, _webPageProxy->topContentInset())); 1002901 3 thorton 2014-04-22 08:37:51 -0700 (In reply to comment #2) > (From update of attachment 229871 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=229871&action=review > > > Source/WebKit2/UIProcess/API/Cocoa/_WKThumbnailView.mm:90 > > + _lastSnapshotScale = 0; > > Could consider NaN instead for uses like this. I had really never considered using NaN for a comparison-that-always-fails case like this. That's neat. > > Source/WebKit2/UIProcess/API/Cocoa/_WKThumbnailView.mm:126 > > - IntRect snapshotRect(IntPoint(), _webPageProxy->viewSize()); > > + IntRect snapshotRect(IntPoint(), IntSize(_webPageProxy->viewSize().width(), _webPageProxy->viewSize().height() - _webPageProxy->topContentInset())); > > Is there a way to write this math simpler? Like maybe this: > > IntRect snapshotRect(IntPoint(), _webPageProxy->viewSize() - IntSize(0, _webPageProxy->topContentInset())); Sure. Thanks, Darin! 1002907 4 thorton 2014-04-22 08:52:29 -0700 http://trac.webkit.org/changeset/167662 229871 2014-04-22 01:48:31 -0700 2014-04-22 07:45:04 -0700 patch tci-wkthumb.diff text/plain 4476 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBiYWQwMTVmLi43NmFiZmRiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMzEg QEAKKzIwMTQtMDQtMjIgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgV0tUaHVtYm5haWxWaWV3IGRvZXNuJ3QgcXVpdGUgd29yayB3aXRoIHRvcENvbnRl bnRJbnNldAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTMxOTc2CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xNjY0MjEyNz4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2Ev X1dLVGh1bWJuYWlsVmlldy5tbToKKyAgICAgICAgKC1bX1dLVGh1bWJuYWlsVmlldyBpbml0V2l0 aEZyYW1lOmZyb21XS1ZpZXc6XSk6CisgICAgICAgIF9XS1RodW1ibmFpbFZpZXcgYWx3YXlzIG5l ZWRzIGEgbGF5ZXIuCisgICAgICAgIFdlIGRvbid0IHdhbnQgdG8gdXNlIHNuYXBzaG90cyBieSBk ZWZhdWx0LCB0aGlzIHdhcyBkZWJ1Z2dpbmcgYWNjaWRlbnRhbGx5IGxhbmRlZCBpbiB0aGUgbGFz dCBjaGFuZ2UuCisKKyAgICAgICAgKC1bX1dLVGh1bWJuYWlsVmlldyBfcmVxdWVzdFNuYXBzaG90 SWZOZWVkZWRdKToKKyAgICAgICAgRG9uJ3QgcmVxdWVzdCBhIG5ldyBzbmFwc2hvdCBpZiB0aGUg c2NhbGUgaXMgdGhlIHNhbWUgYXMgdGhlIGV4aXN0aW5nIG9uZS4KKyAgICAgICAgUmVkdWNlIHRo ZSBoZWlnaHQgb2YgdGhlIHNuYXBzaG90IGJ5IHRoZSB0b3BDb250ZW50SW5zZXQuCisgICAgICAg IEhhdmluZyB0aGUgb3JpZ2luIGF0ICgwLCAwKSBpcyBzdGlsbCBjb3JyZWN0LCBhcyB0aGUgaW5z ZXQgY29udGVudCBpcyBpbiBuZWdhdGl2ZSBZIHNwYWNlLAorICAgICAgICBhbmQgKDAsIDApIGlz IHRoZSB0cnVlIG9yaWdpbiBvZiB0aGUgY29udGVudC4KKworICAgICAgICAoLVtfV0tUaHVtYm5h aWxWaWV3IHNldFNjYWxlOl0pOgorICAgICAgICBXaGVuIG5vdCBzbmFwc2hvdHRpbmcsIHdlJ3Zl IGRlY2lkZWQgdGhhdCBXS1RodW1ibmFpbFZpZXcgc2hvdWxkIGFwcGx5IHRoZSBzY2FsZSB0byB0 aGUgV0tWaWV3IGxheWVyIHRyZWUsCisgICAgICAgIGluc3RlYWQgb2YgZGVwZW5kaW5nIG9uIGNs aWVudHMgdG8gZG8gc28uCisKKyAgICAgICAgKC1bX1dLVGh1bWJuYWlsVmlldyBzZXRVc2VzU25h cHNob3Q6XSk6CisgICAgICAgIFdlICpkbyogbmVlZCB0byBhcHBseSB0aHVtYm5haWwgc2NhbGUg aW4gdGhlIHdlYiBwcm9jZXNzIGV2ZW4gd2hlbiBzbmFwc2hvdHRpbmcsCisgICAgICAgIGJlY2F1 c2Ugd2UgdXNlIHRoZSBsaXZlIGxheWVyIHRyZWUgdGVtcG9yYXJpbHkgd2hpbGUgd2FpdGluZyBm b3IgdGhlIGFzeW5jaHJvbm91cworICAgICAgICBzbmFwc2hvdCB0byByZXR1cm4uIFRoaXMgd2Fz IGEgbWlzc3RlcCBpbiB0aGUgbGFzdCB0aHVtYm5haWwgdmlldyBwYXRjaC4KKwogMjAxNC0wNC0y MSAgVGltIEhvcnRvbiAgPHRpbW90aHlfaG9ydG9uQGFwcGxlLmNvbT4KIAogICAgICAgICBbd2sy XSBEb24ndCBzZW5kIGludHJpbnNpYyBjb250ZW50IHNpemUgdXBkYXRlcyB3aXRoIGRpcnR5IGxh eW91dApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9fV0tU aHVtYm5haWxWaWV3Lm1tIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9fV0tU aHVtYm5haWxWaWV3Lm1tCmluZGV4IDFkYjFjNWQuLmUwOGQ3NGUgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvQ29jb2EvX1dLVGh1bWJuYWlsVmlldy5tbQorKysgYi9T b3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL0NvY29hL19XS1RodW1ibmFpbFZpZXcubW0KQEAg LTU1LDYgKzU1LDcgQEAgdXNpbmcgbmFtZXNwYWNlIFdlYktpdDsKICAgICBCT09MIF9zaG91bGRB cHBseVRodW1ibmFpbFNjYWxlOwogCiAgICAgQk9PTCBfc25hcHNob3RXYXNEZWZlcnJlZDsKKyAg ICBkb3VibGUgX2xhc3RTbmFwc2hvdFNjYWxlOwogfQogCiBAc3ludGhlc2l6ZSBfd2FpdGluZ0Zv clNuYXBzaG90ID0gX3dhaXRpbmdGb3JTbmFwc2hvdDsKQEAgLTY0LDYgKzY1LDggQEAgdXNpbmcg bmFtZXNwYWNlIFdlYktpdDsKICAgICBpZiAoIShzZWxmID0gW3N1cGVyIGluaXRXaXRoRnJhbWU6 ZnJhbWVdKSkKICAgICAgICAgcmV0dXJuIG5pbDsKIAorICAgIHNlbGYud2FudHNMYXllciA9IFlF UzsKKwogICAgIF93a1ZpZXcgPSB3a1ZpZXc7CiAgICAgX3dlYlBhZ2VQcm94eSA9IHRvSW1wbChb X3drVmlldyBwYWdlUmVmXSk7CiAgICAgX3NjYWxlID0gMTsKQEAgLTczLDggKzc2LDYgQEAgdXNp bmcgbmFtZXNwYWNlIFdlYktpdDsKIAogICAgIF9zaG91bGRBcHBseVRodW1ibmFpbFNjYWxlID0g IV9vcmlnaW5hbFNvdXJjZVZpZXdJc0luV2luZG93OwogCi0gICAgc2VsZi51c2VzU25hcHNob3Qg PSBZRVM7Ci0KICAgICByZXR1cm4gc2VsZjsKIH0KIApAQCAtODYsNiArODcsNyBAQCB1c2luZyBu YW1lc3BhY2UgV2ViS2l0OwogICAgIFtfd2tWaWV3IF9zZXRUaHVtYm5haWxWaWV3Om5pbF07CiAK ICAgICBzZWxmLmxheWVyLmNvbnRlbnRzID0gbmlsOworICAgIF9sYXN0U25hcHNob3RTY2FsZSA9 IDA7CiAKICAgICBfd2ViUGFnZVByb3h5LT5zZXRNYXlTdGFydE1lZGlhV2hlbkluV2luZG93KF9v cmlnaW5hbE1heVN0YXJ0TWVkaWFXaGVuSW5XaW5kb3cpOwogfQpAQCAtMTEwLDYgKzExMiw5IEBA IHVzaW5nIG5hbWVzcGFjZSBXZWJLaXQ7CiAgICAgaWYgKCFfdXNlc1NuYXBzaG90KQogICAgICAg ICByZXR1cm47CiAKKyAgICBpZiAoc2VsZi5sYXllci5jb250ZW50cyAmJiBfbGFzdFNuYXBzaG90 U2NhbGUgPT0gX3NjYWxlKQorICAgICAgICByZXR1cm47CisKICAgICBpZiAoX3dhaXRpbmdGb3JT bmFwc2hvdCkgewogICAgICAgICBfc25hcHNob3RXYXNEZWZlcnJlZCA9IFlFUzsKICAgICAgICAg cmV0dXJuOwpAQCAtMTE4LDEwICsxMjMsMTEgQEAgdXNpbmcgbmFtZXNwYWNlIFdlYktpdDsKICAg ICBfd2FpdGluZ0ZvclNuYXBzaG90ID0gWUVTOwogCiAgICAgUmV0YWluUHRyPF9XS1RodW1ibmFp bFZpZXc+IHRodW1ibmFpbFZpZXcgPSBzZWxmOwotICAgIEludFJlY3Qgc25hcHNob3RSZWN0KElu dFBvaW50KCksIF93ZWJQYWdlUHJveHktPnZpZXdTaXplKCkpOworICAgIEludFJlY3Qgc25hcHNo b3RSZWN0KEludFBvaW50KCksIEludFNpemUoX3dlYlBhZ2VQcm94eS0+dmlld1NpemUoKS53aWR0 aCgpLCBfd2ViUGFnZVByb3h5LT52aWV3U2l6ZSgpLmhlaWdodCgpIC0gX3dlYlBhZ2VQcm94eS0+ dG9wQ29udGVudEluc2V0KCkpKTsKICAgICBTbmFwc2hvdE9wdGlvbnMgb3B0aW9ucyA9IFNuYXBz aG90T3B0aW9uc1Jlc3BlY3REcmF3aW5nQXJlYVRyYW5zZm9ybSB8IFNuYXBzaG90T3B0aW9uc0lu Vmlld0Nvb3JkaW5hdGVzOwogICAgIEludFNpemUgYml0bWFwU2l6ZSA9IHNuYXBzaG90UmVjdC5z aXplKCk7CiAgICAgYml0bWFwU2l6ZS5zY2FsZShfc2NhbGUgKiBfd2ViUGFnZVByb3h5LT5kZXZp Y2VTY2FsZUZhY3RvcigpKTsKKyAgICBfbGFzdFNuYXBzaG90U2NhbGUgPSBfc2NhbGU7CiAgICAg X3dlYlBhZ2VQcm94eS0+dGFrZVNuYXBzaG90KHNuYXBzaG90UmVjdCwgYml0bWFwU2l6ZSwgb3B0 aW9ucywgW3RodW1ibmFpbFZpZXddKGJvb2wsIGNvbnN0IFNoYXJlYWJsZUJpdG1hcDo6SGFuZGxl JiBpbWFnZUhhbmRsZSkgewogICAgICAgICBSZWZQdHI8U2hhcmVhYmxlQml0bWFwPiBiaXRtYXAg PSBTaGFyZWFibGVCaXRtYXA6OmNyZWF0ZShpbWFnZUhhbmRsZSwgU2hhcmVkTWVtb3J5OjpSZWFk T25seSk7CiAgICAgICAgIFJldGFpblB0cjxDR0ltYWdlUmVmPiBjZ0ltYWdlID0gYml0bWFwLT5t YWtlQ0dJbWFnZSgpOwpAQCAtMTYzLDYgKzE2OSw4IEBAIHVzaW5nIG5hbWVzcGFjZSBXZWJLaXQ7 CiAKICAgICBpZiAoX3VzZXNTbmFwc2hvdCkKICAgICAgICAgW3NlbGYgX3JlcXVlc3RTbmFwc2hv dElmTmVlZGVkXTsKKworICAgIHNlbGYubGF5ZXIuc3VibGF5ZXJUcmFuc2Zvcm0gPSBDQVRyYW5z Zm9ybTNETWFrZVNjYWxlKF9zY2FsZSwgX3NjYWxlLCAxKTsKIH0KIAogLSAodm9pZClzZXRVc2Vz U25hcHNob3Q6KEJPT0wpdXNlc1NuYXBzaG90CkBAIC0xNzEsNyArMTc5LDYgQEAgdXNpbmcgbmFt ZXNwYWNlIFdlYktpdDsKICAgICAgICAgcmV0dXJuOwogCiAgICAgX3VzZXNTbmFwc2hvdCA9IHVz ZXNTbmFwc2hvdDsKLSAgICBfc2hvdWxkQXBwbHlUaHVtYm5haWxTY2FsZSA9IF91c2VzU25hcHNo b3QgPyBmYWxzZSA6ICFfb3JpZ2luYWxTb3VyY2VWaWV3SXNJbldpbmRvdzsKIAogICAgIGlmICgh c2VsZi53aW5kb3cpCiAgICAgICAgIHJldHVybjsK