132793 2014-05-10 17:17:07 -0700 [WKWebView _updateScrollViewBackground] churns UI-and-CGColors while repainting 2014-05-11 10:53:20 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 thorton thorton andersca bdakin commit-queue darin sam simon.fraser webkit-bug-importer oldest_to_newest 1008342 0 thorton 2014-05-10 17:17:07 -0700 Every layer tree commit, we churn various different color objects in _updateScrollViewBackground. There's no reason for this, and it's a complete waste of ~2.5% of the UI process time while repainting quickly. 1008343 1 webkit-bug-importer 2014-05-10 17:17:31 -0700 <rdar://problem/16877870> 1008361 2 231245 thorton 2014-05-10 20:20:35 -0700 Created attachment 231245 patch 1008362 3 commit-queue 2014-05-10 20:22:21 -0700 Attachment 231245 did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:455: The parameter name "_currentScrollViewBackgroundColor" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 1008366 4 231245 mitz 2014-05-10 20:48:31 -0700 Comment on attachment 231245 patch View in context: https://bugs.webkit.org/attachment.cgi?id=231245&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:129 > + WebCore::Color _currentScrollViewBackgroundColor; What does the word “current” add here? > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:452 > + color = WebCore::Color(color.red(), color.green(), color.blue(), opacity * 255); opacity * 255 is not rounding correctly. If you use colorWithOverrideAlpha() from Color.h you’ll get the correct rounding behavior. Something like: color.setRGB(colorWithOverrideAlpha(color.rgb(), opacity); > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:455 > + if (_currentScrollViewBackgroundColor.isValid() && _currentScrollViewBackgroundColor == color) Color’s operator== compares validity, so I don’t understand what the isValid() check here is doing. 1008367 5 231247 thorton 2014-05-10 20:54:32 -0700 Created attachment 231247 patch 1008368 6 thorton 2014-05-10 20:57:52 -0700 http://trac.webkit.org/changeset/168594 1008422 7 darin 2014-05-11 10:40:06 -0700 Should we add a UIColor cache too, not just a CGColor cache? 1008425 8 thorton 2014-05-11 10:53:20 -0700 (In reply to comment #7) > Should we add a UIColor cache too, not just a CGColor cache? Maybe! Though, with this resolved, I don't see UIColor creation being slow anywhere in traces. 231245 2014-05-10 20:20:35 -0700 2014-05-10 20:54:32 -0700 patch color.diff text/plain 2794 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBkZjQyNGYxLi5lNTRlYzhlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcg QEAKKzIwMTQtMDUtMTAgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgW1dLV2ViVmlldyBfdXBkYXRlU2Nyb2xsVmlld0JhY2tncm91bmRdIGNodXJucyBV SS1hbmQtQ0dDb2xvcnMgd2hpbGUgcmVwYWludGluZworICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMyNzkzCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8x Njg3Nzg3MD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvV0tXZWJWaWV3Lm1tOgorICAgICAgICAoLVtXS1dlYlZp ZXcgX3VwZGF0ZVNjcm9sbFZpZXdCYWNrZ3JvdW5kXSk6CisgICAgICAgIERvbid0IGJ5cGFzcyB0 aGUgY2FjaGUgYW5kIG1ha2UgYSBjb3B5IG9mIHRoZSBDR0NvbG9yIGp1c3QgdG8gc2V0IGl0cyBh bHBoYS4KKyAgICAgICAgQ2FjaGUgdGhlIGxhc3QgY29sb3IgYW5kIGRvbid0IGJvdGhlciBjcmVh dGluZyBhIFVJQ29sb3Igbm9yIHVwZGF0aW5nIHRoZQorICAgICAgICBjb2xvciBvbiBvdXIgdmll d3MgaWYgaXQgaGFzbid0IGNoYW5nZWQuCisKIDIwMTQtMDUtMDkgIERhbiBCZXJuc3RlaW4gIDxt aXR6QGFwcGxlLmNvbT4KIAogICAgICAgICBbQ29jb2FdIFJlbW92ZSBhIGRlcHJlY2F0ZWQgYnVu ZGxlIGZvcm0gZGVsZWdhdGUgbWV0aG9kIG5vdyB0aGF0IG5vIG9uZSBpbXBsZW1lbnRzIGl0CmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL0NvY29hL1dLV2ViVmlldy5t bSBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvQ29jb2EvV0tXZWJWaWV3Lm1tCmluZGV4 IDVhNmE0ODIuLjBhNDU1YzggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9B UEkvQ29jb2EvV0tXZWJWaWV3Lm1tCisrKyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkv Q29jb2EvV0tXZWJWaWV3Lm1tCkBAIC0xMjUsNiArMTI1LDggQEAKICAgICBCT09MIF9hbGxvd3NC YWNrRm9yd2FyZE5hdmlnYXRpb25HZXN0dXJlczsKIAogICAgIFJldGFpblB0cjxVSVZpZXcgPFdL V2ViVmlld0NvbnRlbnRQcm92aWRlcj4+IF9jdXN0b21Db250ZW50VmlldzsKKworICAgIFdlYkNv cmU6OkNvbG9yIF9jdXJyZW50U2Nyb2xsVmlld0JhY2tncm91bmRDb2xvcjsKICNlbmRpZgogI2lm IFBMQVRGT1JNKE1BQykKICAgICBSZXRhaW5QdHI8V0tWaWV3PiBfd2tWaWV3OwpAQCAtNDQyLDE2 ICs0NDQsMjAgQEAgc3RhdGljIENHRmxvYXQgY29udGVudFpvb21TY2FsZShXS1dlYlZpZXcqIHdl YlZpZXcpCiAtICh2b2lkKV91cGRhdGVTY3JvbGxWaWV3QmFja2dyb3VuZAogewogICAgIFdlYkNv cmU6OkNvbG9yIGNvbG9yID0gX3BhZ2UtPnBhZ2VFeHRlbmRlZEJhY2tncm91bmRDb2xvcigpOwot ICAgIFJldGFpblB0cjxDR0NvbG9yUmVmPiBjZ0NvbG9yID0gY2FjaGVkQ0dDb2xvcihjb2xvciwg V2ViQ29yZTo6Q29sb3JTcGFjZURldmljZVJHQik7Ci0KICAgICBDR0Zsb2F0IHpvb21TY2FsZSA9 IGNvbnRlbnRab29tU2NhbGUoc2VsZik7CiAgICAgQ0dGbG9hdCBtaW5pbXVtWm9vbVNjYWxlID0g W19zY3JvbGxWaWV3IG1pbmltdW1ab29tU2NhbGVdOwogICAgIGlmICh6b29tU2NhbGUgPCBtaW5p bXVtWm9vbVNjYWxlKSB7CiAgICAgICAgIENHRmxvYXQgc2xvcGUgPSAxMjsKICAgICAgICAgQ0dG bG9hdCBvcGFjaXR5ID0gc3RkOjptYXg8Q0dGbG9hdD4oMSAtIHNsb3BlICogKG1pbmltdW1ab29t U2NhbGUgLSB6b29tU2NhbGUpLCAwKTsKLSAgICAgICAgY2dDb2xvciA9IGFkb3B0Q0YoQ0dDb2xv ckNyZWF0ZUNvcHlXaXRoQWxwaGEoY2dDb2xvci5nZXQoKSwgb3BhY2l0eSkpOworICAgICAgICBj b2xvciA9IFdlYkNvcmU6OkNvbG9yKGNvbG9yLnJlZCgpLCBjb2xvci5ncmVlbigpLCBjb2xvci5i bHVlKCksIG9wYWNpdHkgKiAyNTUpOwogICAgIH0KLSAgICBSZXRhaW5QdHI8VUlDb2xvcio+IHVp QmFja2dyb3VuZENvbG9yID0gYWRvcHROUyhbW1VJQ29sb3IgYWxsb2NdIGluaXRXaXRoQ0dDb2xv cjpjZ0NvbG9yLmdldCgpXSk7CisKKyAgICBpZiAoX2N1cnJlbnRTY3JvbGxWaWV3QmFja2dyb3Vu ZENvbG9yLmlzVmFsaWQoKSAmJiBfY3VycmVudFNjcm9sbFZpZXdCYWNrZ3JvdW5kQ29sb3IgPT0g Y29sb3IpCisgICAgICAgIHJldHVybjsKKworICAgIF9jdXJyZW50U2Nyb2xsVmlld0JhY2tncm91 bmRDb2xvciA9IGNvbG9yOworCisgICAgUmV0YWluUHRyPFVJQ29sb3IqPiB1aUJhY2tncm91bmRD b2xvciA9IGFkb3B0TlMoW1tVSUNvbG9yIGFsbG9jXSBpbml0V2l0aENHQ29sb3I6Y2FjaGVkQ0dD b2xvcihjb2xvciwgV2ViQ29yZTo6Q29sb3JTcGFjZURldmljZVJHQildKTsKICAgICBfbWFpbkV4 dGVuZGVkQmFja2dyb3VuZFZpZXcuYmFja2dyb3VuZENvbG9yID0gdWlCYWNrZ3JvdW5kQ29sb3Iu Z2V0KCk7CiAgICAgX2V4dGVuZGVkQmFja2dyb3VuZExheWVyVG9wSW5zZXQuYmFja2dyb3VuZENv bG9yID0gdWlCYWNrZ3JvdW5kQ29sb3IuZ2V0KCk7CiAgICAgX2V4dGVuZGVkQmFja2dyb3VuZExh eWVyQm90dG9tSW5zZXQuYmFja2dyb3VuZENvbG9yID0gdWlCYWNrZ3JvdW5kQ29sb3IuZ2V0KCk7 Cg== 231247 2014-05-10 20:54:32 -0700 2014-05-10 20:56:22 -0700 patch color.diff text/plain 3992 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA2ZWQ4NTRjLi42NjNkMGNiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQg QEAKKzIwMTQtMDUtMTAgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgW1dLV2ViVmlldyBfdXBkYXRlU2Nyb2xsVmlld0JhY2tncm91bmRdIGNodXJucyBV SS1hbmQtQ0dDb2xvcnMgd2hpbGUgcmVwYWludGluZworICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMyNzkzCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8x Njg3Nzg3MD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIFdlYkNvcmUuZXhwLmluOgorICAgICAgICBFeHBvcnQgYSBDb2xvciBjb252ZW5pZW5jZSBm dW5jdGlvbi4KKwogMjAxNC0wNS0wOSAgRGVhbiBKYWNrc29uICA8ZGlub0BhcHBsZS5jb20+CiAK ICAgICAgICAgLXdlYmtpdC1maWx0ZXIgcHJldmVudHMgcmVuZGVyaW5nIGF0IHJldGluYSBzY2Fs ZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvV2ViQ29yZS5leHAuaW4gYi9Tb3VyY2UvV2Vi Q29yZS9XZWJDb3JlLmV4cC5pbgppbmRleCA3ZmQzYzA1Li44ZDNkZTdlIDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViQ29yZS9XZWJDb3JlLmV4cC5pbgorKysgYi9Tb3VyY2UvV2ViQ29yZS9XZWJDb3Jl LmV4cC5pbgpAQCAtOTMzLDYgKzkzMyw3IEBAIF9fWk43V2ViQ29yZTIyV2hlZWxFdmVudERlbHRh VHJhY2tlcjE3ZW5kVHJhY2tpbmdEZWx0YXNFdgogX19aTjdXZWJDb3JlMjJXaGVlbEV2ZW50RGVs dGFUcmFja2VyMTliZWdpblRyYWNraW5nRGVsdGFzRXYKIF9fWk43V2ViQ29yZTIyV2hlZWxFdmVu dERlbHRhVHJhY2tlcjIxcmVjb3JkV2hlZWxFdmVudERlbHRhRVJLTlNfMThQbGF0Zm9ybVdoZWVs RXZlbnRFCiBfX1pON1dlYkNvcmUyMldoZWVsRXZlbnREZWx0YVRyYWNrZXJDMUV2CitfX1pON1dl YkNvcmUyMmNvbG9yV2l0aE92ZXJyaWRlQWxwaGFFamYKIF9fWk43V2ViQ29yZTIyY291bnRlclZh bHVlRm9yRWxlbWVudEVQTlNfN0VsZW1lbnRFCiBfX1pON1dlYkNvcmUyMmNyZWF0ZUZyYWdtZW50 RnJvbVRleHRFUk5TXzVSYW5nZUVSS04zV1RGNlN0cmluZ0UKIF9fWk43V2ViQ29yZTIyZXh0ZXJu YWxSZXByZXNlbnRhdGlvbkVQTlNfNUZyYW1lRWoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQy L0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwppbmRleCBkZjQyNGYxLi5lNTRl YzhlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dl YktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTQtMDUtMTAgIFRpbSBIb3J0b24g IDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisKKyAgICAgICAgW1dLV2ViVmlldyBfdXBkYXRl U2Nyb2xsVmlld0JhY2tncm91bmRdIGNodXJucyBVSS1hbmQtQ0dDb2xvcnMgd2hpbGUgcmVwYWlu dGluZworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMy NzkzCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xNjg3Nzg3MD4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvV0tX ZWJWaWV3Lm1tOgorICAgICAgICAoLVtXS1dlYlZpZXcgX3VwZGF0ZVNjcm9sbFZpZXdCYWNrZ3Jv dW5kXSk6CisgICAgICAgIERvbid0IGJ5cGFzcyB0aGUgY2FjaGUgYW5kIG1ha2UgYSBjb3B5IG9m IHRoZSBDR0NvbG9yIGp1c3QgdG8gc2V0IGl0cyBhbHBoYS4KKyAgICAgICAgQ2FjaGUgdGhlIGxh c3QgY29sb3IgYW5kIGRvbid0IGJvdGhlciBjcmVhdGluZyBhIFVJQ29sb3Igbm9yIHVwZGF0aW5n IHRoZQorICAgICAgICBjb2xvciBvbiBvdXIgdmlld3MgaWYgaXQgaGFzbid0IGNoYW5nZWQuCisK IDIwMTQtMDUtMDkgIERhbiBCZXJuc3RlaW4gIDxtaXR6QGFwcGxlLmNvbT4KIAogICAgICAgICBb Q29jb2FdIFJlbW92ZSBhIGRlcHJlY2F0ZWQgYnVuZGxlIGZvcm0gZGVsZWdhdGUgbWV0aG9kIG5v dyB0aGF0IG5vIG9uZSBpbXBsZW1lbnRzIGl0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9V SVByb2Nlc3MvQVBJL0NvY29hL1dLV2ViVmlldy5tbSBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9BUEkvQ29jb2EvV0tXZWJWaWV3Lm1tCmluZGV4IDVhNmE0ODIuLjUwZGZlMWYgMTAwNjQ0Ci0t LSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvQ29jb2EvV0tXZWJWaWV3Lm1tCisrKyBi L1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvQ29jb2EvV0tXZWJWaWV3Lm1tCkBAIC0xMjUs NiArMTI1LDggQEAKICAgICBCT09MIF9hbGxvd3NCYWNrRm9yd2FyZE5hdmlnYXRpb25HZXN0dXJl czsKIAogICAgIFJldGFpblB0cjxVSVZpZXcgPFdLV2ViVmlld0NvbnRlbnRQcm92aWRlcj4+IF9j dXN0b21Db250ZW50VmlldzsKKworICAgIFdlYkNvcmU6OkNvbG9yIF9zY3JvbGxWaWV3QmFja2dy b3VuZENvbG9yOwogI2VuZGlmCiAjaWYgUExBVEZPUk0oTUFDKQogICAgIFJldGFpblB0cjxXS1Zp ZXc+IF93a1ZpZXc7CkBAIC00NDIsMTYgKzQ0NCwyMCBAQCBzdGF0aWMgQ0dGbG9hdCBjb250ZW50 Wm9vbVNjYWxlKFdLV2ViVmlldyogd2ViVmlldykKIC0gKHZvaWQpX3VwZGF0ZVNjcm9sbFZpZXdC YWNrZ3JvdW5kCiB7CiAgICAgV2ViQ29yZTo6Q29sb3IgY29sb3IgPSBfcGFnZS0+cGFnZUV4dGVu ZGVkQmFja2dyb3VuZENvbG9yKCk7Ci0gICAgUmV0YWluUHRyPENHQ29sb3JSZWY+IGNnQ29sb3Ig PSBjYWNoZWRDR0NvbG9yKGNvbG9yLCBXZWJDb3JlOjpDb2xvclNwYWNlRGV2aWNlUkdCKTsKLQog ICAgIENHRmxvYXQgem9vbVNjYWxlID0gY29udGVudFpvb21TY2FsZShzZWxmKTsKICAgICBDR0Zs b2F0IG1pbmltdW1ab29tU2NhbGUgPSBbX3Njcm9sbFZpZXcgbWluaW11bVpvb21TY2FsZV07CiAg ICAgaWYgKHpvb21TY2FsZSA8IG1pbmltdW1ab29tU2NhbGUpIHsKICAgICAgICAgQ0dGbG9hdCBz bG9wZSA9IDEyOwogICAgICAgICBDR0Zsb2F0IG9wYWNpdHkgPSBzdGQ6Om1heDxDR0Zsb2F0Pigx IC0gc2xvcGUgKiAobWluaW11bVpvb21TY2FsZSAtIHpvb21TY2FsZSksIDApOwotICAgICAgICBj Z0NvbG9yID0gYWRvcHRDRihDR0NvbG9yQ3JlYXRlQ29weVdpdGhBbHBoYShjZ0NvbG9yLmdldCgp LCBvcGFjaXR5KSk7CisgICAgICAgIGNvbG9yID0gV2ViQ29yZTo6Y29sb3JXaXRoT3ZlcnJpZGVB bHBoYShjb2xvci5yZ2IoKSwgb3BhY2l0eSk7CiAgICAgfQotICAgIFJldGFpblB0cjxVSUNvbG9y Kj4gdWlCYWNrZ3JvdW5kQ29sb3IgPSBhZG9wdE5TKFtbVUlDb2xvciBhbGxvY10gaW5pdFdpdGhD R0NvbG9yOmNnQ29sb3IuZ2V0KCldKTsKKworICAgIGlmIChfc2Nyb2xsVmlld0JhY2tncm91bmRD b2xvciA9PSBjb2xvcikKKyAgICAgICAgcmV0dXJuOworCisgICAgX3Njcm9sbFZpZXdCYWNrZ3Jv dW5kQ29sb3IgPSBjb2xvcjsKKworICAgIFJldGFpblB0cjxVSUNvbG9yKj4gdWlCYWNrZ3JvdW5k Q29sb3IgPSBhZG9wdE5TKFtbVUlDb2xvciBhbGxvY10gaW5pdFdpdGhDR0NvbG9yOmNhY2hlZENH Q29sb3IoY29sb3IsIFdlYkNvcmU6OkNvbG9yU3BhY2VEZXZpY2VSR0IpXSk7CiAgICAgX21haW5F eHRlbmRlZEJhY2tncm91bmRWaWV3LmJhY2tncm91bmRDb2xvciA9IHVpQmFja2dyb3VuZENvbG9y LmdldCgpOwogICAgIF9leHRlbmRlZEJhY2tncm91bmRMYXllclRvcEluc2V0LmJhY2tncm91bmRD b2xvciA9IHVpQmFja2dyb3VuZENvbG9yLmdldCgpOwogICAgIF9leHRlbmRlZEJhY2tncm91bmRM YXllckJvdHRvbUluc2V0LmJhY2tncm91bmRDb2xvciA9IHVpQmFja2dyb3VuZENvbG9yLmdldCgp Owo=