134406 2014-06-27 12:06:32 -0700 Custom scrollbars should not create ScrollbarPainters on Mac 2014-06-27 14:05:16 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 bdakin webkit-unassigned bdakin bunhere cdumez commit-queue esprehn+autocc glenn gyuyoung.kim kondapallykalyan sam sergio simon.fraser thorton oldest_to_newest 1019044 0 bdakin 2014-06-27 12:06:32 -0700 Pages with custom scrollbars are still creating NSScrollerImps for those scrollbars on Mac. This should not be happening. <rdar://problem/16178301> 1019049 1 234005 bdakin 2014-06-27 12:13:55 -0700 Created attachment 234005 Patch 1019050 2 commit-queue 2014-06-27 12:16:32 -0700 Attachment 234005 did not pass style-queue: ERROR: Source/WebCore/platform/Scrollbar.cpp:59: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/Scrollbar.cpp:82: Wrong number of spaces before statement. (expected: 25) [whitespace/indent] [4] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style. 1019054 3 bdakin 2014-06-27 12:24:36 -0700 Thanks Tim http://trac.webkit.org/changeset/170546 1019066 4 234005 darin 2014-06-27 13:29:34 -0700 Comment on attachment 234005 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=234005&action=review > Source/WebCore/platform/Scrollbar.h:201 > + bool m_isCustomScrollbar; Why protected instead of private? > Source/WebCore/rendering/RenderScrollbar.cpp:45 > + : Scrollbar(scrollableArea, orientation, RegularScrollbar, RenderScrollbarTheme::renderScrollbarTheme(), true) Makes me think we should use an enum instead of a bool for readability. 1019082 5 bdakin 2014-06-27 14:05:16 -0700 (In reply to comment #4) > (From update of attachment 234005 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234005&action=review > > > Source/WebCore/platform/Scrollbar.h:201 > > + bool m_isCustomScrollbar; > > Why protected instead of private? > Oh, that was very silly. This should definitely be private, I just failed to notice that all of the other member variables were protected when I put this one next to them. > > Source/WebCore/rendering/RenderScrollbar.cpp:45 > > + : Scrollbar(scrollableArea, orientation, RegularScrollbar, RenderScrollbarTheme::renderScrollbarTheme(), true) > > Makes me think we should use an enum instead of a bool for readability. That would definitely look nicer. I'll do a follow-up cleanup here. 234005 2014-06-27 12:13:55 -0700 2014-06-27 13:29:34 -0700 Patch for-review.txt text/plain 6229 bdakin SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3MDU0NCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDM3IEBACisyMDE0LTA2LTI3ICBCZXRoIERh a2luICA8YmRha2luQGFwcGxlLmNvbT4KKworICAgICAgICBDdXN0b20gc2Nyb2xsYmFycyBzaG91 bGQgbm90IGNyZWF0ZSBTY3JvbGxiYXJQYWludGVycyBvbiBNYWMKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzNDQwNgorICAgICAgICAtYW5kIGNvcnJl c3BvbmRpbmctCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xNjE3ODMwMT4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgc29sdXRpb24gaGVyZSBp cyB0byByZXR1cm4gZWFybHkgaW4gU2Nyb2xsYmFyVGhlbWVNYWM6OnJlZ2lzdGVyU2Nyb2xsYmFy KCkgaWYgCisgICAgICAgIHRoZSBzY3JvbGxiYXIgaXMgY3VzdG9tLiBIb3dldmVyLCBzaW5jZSB0 aGlzIGZ1bmN0aW9uIGlzIGNhbGxlZCBkdXJpbmcgdGhlIAorICAgICAgICBSZW5kZXJTY3JvbGxi YXIgYW5kIFNjcm9sbGJhciBjb25zdHJ1Y3Rvciwgd2UgbmVlZCB0byByZS1pbXBsZW1lbnQgCisg ICAgICAgIFNjcm9sbGJhcjo6aXNDdXN0b21TY3JvbGxiYXIoKSB0byByZXR1cm4gYSBtZW1iZXIg dmFyaWFibGUgdGhhdCBpcyBwYXNzZWQgaW50byB0aGUgCisgICAgICAgIGNvbnN0cnVjdG9yLiBP dGhlcndpc2UsIHdlIHdpbGwgZ2V0IFNjcm9sbGJhcuKAmXMgaW1wbGVtZW50YXRpb24gaXMgCisg ICAgICAgIGlzQ3VzdG9tU2Nyb2xsYmFyKCkgd3JvbmdmdWxseSByZXR1cm5pbmcgZmFsc2Ugc2lu Y2UgaW5zdGVhZCBvZiB0aGUgZGVyaXZlZAorICAgICAgICBjbGFzc+KAmXMgaW1wbGVtZW50YXRp b24uIAorCisgICAgICAgIFNjcm9sbGJhciBjb25zdHJ1Y3RvciBub3cgaGFzIGFuIG9wdGlvbiBw YXJhbWV0ZXIgaXNDdXN0b21TY3JvbGxiYXIgdGhhdCBkZWZhdWx0cyAKKyAgICAgICAgdG8gZmFs c2UuIFRoYXQgdmFsdWUgaXMgcmV0dXJuZWQgYnkgaXNDdXN0b21TY3JvbGxiYXIoKQorICAgICAg ICAqIHBsYXRmb3JtL1Njcm9sbGJhci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpTY3JvbGxiYXI6 OlNjcm9sbGJhcik6CisgICAgICAgICogcGxhdGZvcm0vU2Nyb2xsYmFyLmg6CisKKyAgICAgICAg UmV0dXJuIGVhcmx5IGlmIHRoaXMgaXMgYSBjdXN0b20gc2Nyb2xsYmFyIHNpbmNlIHRoZSByZXN0 IG9mIHRoZSBmdW5jdGlvbiBkZWFscyAKKyAgICAgICAgd2l0aCBjcmVhdGluZyBhIG5hdGl2ZSBz Y3JvbGxiYXIgYW5kIGdldHRpbmcgaXQgaW4gdGhlIHJpZ2h0IHN0YXRlLgorICAgICAgICAqIHBs YXRmb3JtL21hYy9TY3JvbGxiYXJUaGVtZU1hYy5tbToKKyAgICAgICAgKFdlYkNvcmU6OlNjcm9s bGJhclRoZW1lTWFjOjpyZWdpc3RlclNjcm9sbGJhcik6CisKKyAgICAgICAgU2VuZCB0cnVlIHRv IHRoZSBTY3JvbGxiYXIgY29uc3RydWN0b3IgdG8gaW5kaWNhdGUgdGhhdCB0aGlzIGlzIGEgY3Vz dG9tIAorICAgICAgICBzY3JvbGxiYXIuCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclNjcm9s bGJhci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJTY3JvbGxiYXI6OlJlbmRlclNjcm9s bGJhcik6CisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclNjcm9sbGJhci5oOgorCiAyMDE0LTA2 LTI3ICBNYXJ0aW4gSG9kb3ZhbiAgPG1ob2RvdmFuLnUtc3plZ2VkQHBhcnRuZXIuc2Ftc3VuZy5j b20+CiAKICAgICAgICAgUkVHUkVTU0lPTiAocjE2ODY4NSk6IGNzcyBjYWxjKCkgZXhwcmVzc2lv biBmYWlscyAKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL1Njcm9sbGJhci5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vU2Nyb2xsYmFyLmNwcAkocmV2aXNp b24gMTcwNTMzKQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vU2Nyb2xsYmFyLmNwcAkod29y a2luZyBjb3B5KQpAQCAtNTYsNyArNTYsNyBAQCBpbnQgU2Nyb2xsYmFyOjptYXhPdmVybGFwQmV0 d2VlblBhZ2VzKCkKIH0KIAogU2Nyb2xsYmFyOjpTY3JvbGxiYXIoU2Nyb2xsYWJsZUFyZWEqIHNj cm9sbGFibGVBcmVhLCBTY3JvbGxiYXJPcmllbnRhdGlvbiBvcmllbnRhdGlvbiwgU2Nyb2xsYmFy Q29udHJvbFNpemUgY29udHJvbFNpemUsCi0gICAgICAgICAgICAgICAgICAgICBTY3JvbGxiYXJU aGVtZSogdGhlbWUpCisgICAgICAgICAgICAgICAgICAgICBTY3JvbGxiYXJUaGVtZSogdGhlbWUs IGJvb2wgaXNDdXN0b21TY3JvbGxiYXIpCiAgICAgOiBtX3Njcm9sbGFibGVBcmVhKHNjcm9sbGFi bGVBcmVhKQogICAgICwgbV9vcmllbnRhdGlvbihvcmllbnRhdGlvbikKICAgICAsIG1fY29udHJv bFNpemUoY29udHJvbFNpemUpCkBAIC03OSw2ICs3OSw3IEBAIFNjcm9sbGJhcjo6U2Nyb2xsYmFy KFNjcm9sbGFibGVBcmVhKiBzY3IKICAgICAsIG1fb3ZlcmxhcHNSZXNpemVyKGZhbHNlKQogICAg ICwgbV9zdXBwcmVzc0ludmFsaWRhdGlvbihmYWxzZSkKICAgICAsIG1faXNBbHBoYUxvY2tlZChm YWxzZSkKKyAgICAsIG1faXNDdXN0b21TY3JvbGxiYXIoaXNDdXN0b21TY3JvbGxiYXIpCiB7CiAg ICAgaWYgKCFtX3RoZW1lKQogICAgICAgICBtX3RoZW1lID0gU2Nyb2xsYmFyVGhlbWU6OnRoZW1l KCk7CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9TY3JvbGxiYXIuaAo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9TY3JvbGxiYXIuaAkocmV2aXNpb24gMTcwNTMz KQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vU2Nyb2xsYmFyLmgJKHdvcmtpbmcgY29weSkK QEAgLTcyLDcgKzcyLDcgQEAgcHVibGljOgogCiAgICAgdmlydHVhbCBJbnRQb2ludCBjb252ZXJ0 RnJvbUNvbnRhaW5pbmdXaW5kb3coY29uc3QgSW50UG9pbnQmIHdpbmRvd1BvaW50KSBvdmVycmlk ZSB7IHJldHVybiBXaWRnZXQ6OmNvbnZlcnRGcm9tQ29udGFpbmluZ1dpbmRvdyh3aW5kb3dQb2lu dCk7IH0KIAotICAgIHZpcnR1YWwgYm9vbCBpc0N1c3RvbVNjcm9sbGJhcigpIGNvbnN0IG92ZXJy aWRlIHsgcmV0dXJuIGZhbHNlOyB9CisgICAgdmlydHVhbCBib29sIGlzQ3VzdG9tU2Nyb2xsYmFy KCkgY29uc3Qgb3ZlcnJpZGUgZmluYWwgeyByZXR1cm4gbV9pc0N1c3RvbVNjcm9sbGJhcjsgfQog ICAgIHZpcnR1YWwgU2Nyb2xsYmFyT3JpZW50YXRpb24gb3JpZW50YXRpb24oKSBjb25zdCBvdmVy cmlkZSB7IHJldHVybiBtX29yaWVudGF0aW9uOyB9CiAKICAgICB2aXJ0dWFsIGludCB2YWx1ZSgp IGNvbnN0IG92ZXJyaWRlIHsgcmV0dXJuIGxyb3VuZGYobV9jdXJyZW50UG9zKTsgfQpAQCAtMTU2 LDcgKzE1Niw3IEBAIHB1YmxpYzoKICAgICB2aXJ0dWFsIGJvb2wgc3VwcG9ydHNVcGRhdGVPblNl Y29uZGFyeVRocmVhZCgpIGNvbnN0OwogCiBwcm90ZWN0ZWQ6Ci0gICAgU2Nyb2xsYmFyKFNjcm9s bGFibGVBcmVhKiwgU2Nyb2xsYmFyT3JpZW50YXRpb24sIFNjcm9sbGJhckNvbnRyb2xTaXplLCBT Y3JvbGxiYXJUaGVtZSogPSAwKTsKKyAgICBTY3JvbGxiYXIoU2Nyb2xsYWJsZUFyZWEqLCBTY3Jv bGxiYXJPcmllbnRhdGlvbiwgU2Nyb2xsYmFyQ29udHJvbFNpemUsIFNjcm9sbGJhclRoZW1lKiA9 IDAsIGJvb2wgaXNDdXN0b21TY3JvbGxiYXIgPSBmYWxzZSk7CiAKICAgICB2b2lkIHVwZGF0ZVRo dW1iKCk7CiAgICAgdmlydHVhbCB2b2lkIHVwZGF0ZVRodW1iUG9zaXRpb24oKTsKQEAgLTE5OCw2 ICsxOTgsOCBAQCBwcm90ZWN0ZWQ6CiAKICAgICBib29sIG1faXNBbHBoYUxvY2tlZDsKIAorICAg IGJvb2wgbV9pc0N1c3RvbVNjcm9sbGJhcjsKKwogcHJpdmF0ZToKICAgICB2aXJ0dWFsIGJvb2wg aXNTY3JvbGxiYXIoKSBjb25zdCBvdmVycmlkZSB7IHJldHVybiB0cnVlOyB9CiB9OwpJbmRleDog U291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbWFjL1Njcm9sbGJhclRoZW1lTWFjLm1tCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21hYy9TY3JvbGxiYXJUaGVtZU1hYy5tbQko cmV2aXNpb24gMTcwNTMzKQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbWFjL1Njcm9sbGJh clRoZW1lTWFjLm1tCSh3b3JraW5nIGNvcHkpCkBAIC0xNTYsNiArMTU2LDkgQEAgc3RhdGljIE5T Q29udHJvbFNpemUgc2Nyb2xsYmFyQ29udHJvbFNpegogCiB2b2lkIFNjcm9sbGJhclRoZW1lTWFj OjpyZWdpc3RlclNjcm9sbGJhcihTY3JvbGxiYXJUaGVtZUNsaWVudCogc2Nyb2xsYmFyKQogewor ICAgIGlmIChzY3JvbGxiYXItPmlzQ3VzdG9tU2Nyb2xsYmFyKCkpCisgICAgICAgIHJldHVybjsK KwogICAgIGJvb2wgaXNIb3Jpem9udGFsID0gc2Nyb2xsYmFyLT5vcmllbnRhdGlvbigpID09IEhv cml6b250YWxTY3JvbGxiYXI7CiAgICAgU2Nyb2xsYmFyUGFpbnRlciBzY3JvbGxiYXJQYWludGVy ID0gW05TQ2xhc3NGcm9tU3RyaW5nKEAiTlNTY3JvbGxlckltcCIpIHNjcm9sbGVySW1wV2l0aFN0 eWxlOnJlY29tbWVuZGVkU2Nyb2xsZXJTdHlsZSgpIGNvbnRyb2xTaXplOnNjcm9sbGJhckNvbnRy b2xTaXplVG9OU0NvbnRyb2xTaXplKHNjcm9sbGJhci0+Y29udHJvbFNpemUoKSkgaG9yaXpvbnRh bDppc0hvcml6b250YWwgcmVwbGFjaW5nU2Nyb2xsZXJJbXA6bmlsXTsKICAgICBzY3JvbGxiYXJN YXAoKS0+YWRkKHNjcm9sbGJhciwgc2Nyb2xsYmFyUGFpbnRlcik7CkluZGV4OiBTb3VyY2UvV2Vi Q29yZS9yZW5kZXJpbmcvUmVuZGVyU2Nyb2xsYmFyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyU2Nyb2xsYmFyLmNwcAkocmV2aXNpb24gMTcwNTMzKQor KysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclNjcm9sbGJhci5jcHAJKHdvcmtpbmcg Y29weSkKQEAgLTQyLDcgKzQyLDcgQEAgUmVmUHRyPFNjcm9sbGJhcj4gUmVuZGVyU2Nyb2xsYmFy OjpjcmVhdAogfQogCiBSZW5kZXJTY3JvbGxiYXI6OlJlbmRlclNjcm9sbGJhcihTY3JvbGxhYmxl QXJlYSogc2Nyb2xsYWJsZUFyZWEsIFNjcm9sbGJhck9yaWVudGF0aW9uIG9yaWVudGF0aW9uLCBF bGVtZW50KiBvd25lckVsZW1lbnQsIEZyYW1lKiBvd25pbmdGcmFtZSkKLSAgICA6IFNjcm9sbGJh cihzY3JvbGxhYmxlQXJlYSwgb3JpZW50YXRpb24sIFJlZ3VsYXJTY3JvbGxiYXIsIFJlbmRlclNj cm9sbGJhclRoZW1lOjpyZW5kZXJTY3JvbGxiYXJUaGVtZSgpKQorICAgIDogU2Nyb2xsYmFyKHNj cm9sbGFibGVBcmVhLCBvcmllbnRhdGlvbiwgUmVndWxhclNjcm9sbGJhciwgUmVuZGVyU2Nyb2xs YmFyVGhlbWU6OnJlbmRlclNjcm9sbGJhclRoZW1lKCksIHRydWUpCiAgICAgLCBtX293bmVyRWxl bWVudChvd25lckVsZW1lbnQpCiAgICAgLCBtX293bmluZ0ZyYW1lKG93bmluZ0ZyYW1lKQogewpJ bmRleDogU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclNjcm9sbGJhci5oCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJTY3JvbGxiYXIuaAkocmV2aXNp b24gMTcwNTMzKQorKysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclNjcm9sbGJhci5o CSh3b3JraW5nIGNvcHkpCkBAIC03NCw4ICs3NCw2IEBAIHByaXZhdGU6CiAKICAgICB2aXJ0dWFs IHZvaWQgc3R5bGVDaGFuZ2VkKCkgb3ZlcnJpZGU7CiAKLSAgICB2aXJ0dWFsIGJvb2wgaXNDdXN0 b21TY3JvbGxiYXIoKSBjb25zdCBvdmVycmlkZSB7IHJldHVybiB0cnVlOyB9Ci0KICAgICB2b2lk IHVwZGF0ZVNjcm9sbGJhclBhcnRzKCk7CiAKICAgICB2b2lkIHVwZGF0ZVNjcm9sbGJhclBhcnQo U2Nyb2xsYmFyUGFydCk7Cg==