109064 2013-02-06 10:02:11 -0800 [chromium] Default scrollbar theme should be shared with CC compositor. 2013-04-08 16:23:08 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 1 wjmaclean wjmaclean abarth aelias andersca dglazkov fishd jamesr rbyers schenney tkent+wkapi tonikitoo trchen webkit.review.bot oldest_to_newest 826519 0 wjmaclean 2013-02-06 10:02:11 -0800 [chromium] Default scrollbar theme should be shared with CC compositor. 826520 1 186873 wjmaclean 2013-02-06 10:03:18 -0800 Created attachment 186873 Patch 826522 2 webkit.review.bot 2013-02-06 10:12:58 -0800 Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. 826714 3 jamesr 2013-02-06 13:34:28 -0800 This seems strange. For WebKit-created scrollbars, we want to make sure the theming matches so it looks exactly like a WebKit rendered scrollbars. For scrollbars that aren't created and owned by WebKit, why would you want to use the same theme? 826723 4 wjmaclean 2013-02-06 13:44:45 -0800 (In reply to comment #3) > This seems strange. For WebKit-created scrollbars, we want to make sure the theming matches so it looks exactly like a WebKit rendered scrollbars. For scrollbars that aren't created and owned by WebKit, why would you want to use the same theme? We don't need the scrollbars to look identical to WebKit ones, so we have a custom painter. Two responses here: 1) We want the ability to know if the main-frame scrollbars (created by WebKit) will be overlay or not so we will know wether pinch-zoom scrollbars are needed (non-overlay => yes) or not (overlay => no) (here we just plumb through the boolean value). 2) We want to be able to render the pinch-zoom scrollbars with the WebKit default theme geometry, *and* to create a ScrollbarLayer we need a WebScrollbarThemeGeometry* that is built from a theme geometry, which in this case is the default theme geometry. This is my understanding of the approach you described, based on our conversation from two weeks ago (getting at the theme pointer and plumbing it through to CC). 826724 5 wjmaclean 2013-02-06 13:45:35 -0800 (In reply to comment #3) > This seems strange. For WebKit-created scrollbars, we want to make sure the theming matches so it looks exactly like a WebKit rendered scrollbars. For scrollbars that aren't created and owned by WebKit, why would you want to use the same theme? BTW, this CL is the companion to https://codereview.chromium.org/11550035/ 826774 6 jamesr 2013-02-06 14:47:49 -0800 (In reply to comment #4) > (In reply to comment #3) > > This seems strange. For WebKit-created scrollbars, we want to make sure the theming matches so it looks exactly like a WebKit rendered scrollbars. For scrollbars that aren't created and owned by WebKit, why would you want to use the same theme? > > We don't need the scrollbars to look identical to WebKit ones, so we have a custom painter. > > Two responses here: > > 1) We want the ability to know if the main-frame scrollbars (created by WebKit) will be overlay or not so we will know wether pinch-zoom scrollbars are needed (non-overlay => yes) or not (overlay => no) (here we just plumb through the boolean value). > How does the default theme make the determination to be overlay or not? If you want to mirror the setting I think you should look at where it's coming from. What would it mean to have a "non-overlay pinch-zoom scrollbar"? Pinch-zoom scrollbars can't take layout space (by definition), so doesn't that mean they are always overlay? > 2) We want to be able to render the pinch-zoom scrollbars with the WebKit default theme geometry Why? Why is WebKit's default theme geometry the geometry you want for pinch-zoom scrollbars, which aren't WebKit-related things? They don't seem related to me. 826795 7 wjmaclean 2013-02-06 15:19:14 -0800 (In reply to comment #6) > > > How does the default theme make the determination to be overlay or not? If you want to mirror the setting I think you should look at where it's coming from. It's set as an override to a virtual function. At present. ScrollbarThemeChromium (Default?) always says 'false' for usesOverlayScrollbars() whereas ScrollbarThemeChromiumAndroid always says 'true'. I figured if I plumbed the value, I'd pick up whichever was in use without having to assume anything other than constancy for a given platform (e.g. ScrollbarTheme won't give a different answer on subsequent calls). > What would it mean to have a "non-overlay pinch-zoom scrollbar"? Pinch-zoom scrollbars can't take layout space (by definition), so doesn't that mean they are always overlay? Pinch zoom scrollbars are always overlay. *But* they are only needed *if* the mainframe scrollbars are non-overlay, and they are not needed if the mainframe scrollbars are overlay. > > > 2) We want to be able to render the pinch-zoom scrollbars with the WebKit default theme geometry > > Why? Why is WebKit's default theme geometry the geometry you want for pinch-zoom scrollbars, which aren't WebKit-related things? They don't seem related to me. No, they're not related, but the WebKit theme suffices just fine for pinch-zoom scrollbars, so why not use it? This is a challenging conversation to have on the bug ... should we do this by VC tomorrow instead? Obviously I didn't understand what you wanted, so it would help me to get some clarification if we could talk in real time. 826943 8 186873 jamesr 2013-02-06 18:02:44 -0800 Comment on attachment 186873 Patch Let's do that before proceeding. I'm out the rest of this week. 186873 2013-02-06 10:03:18 -0800 2013-02-06 18:02:44 -0800 Patch bug-109064-20130206125955.patch text/plain 3556 wjmaclean U3VidmVyc2lvbiBSZXZpc2lvbjogMTQyMDAyCmRpZmYgLS1naXQgYS9Tb3VyY2UvUGxhdGZvcm0v Q2hhbmdlTG9nIGIvU291cmNlL1BsYXRmb3JtL0NoYW5nZUxvZwppbmRleCA3M2FiODgxMGNmYjcy NTFmYWZlMGIwMWQ3YWZhMWVhZTYxMWY5OGJlLi5kMzFiNzY0ZTkyMjIyNWFjMmE4YzQ4ODA5OTcw ZWFkYmI5YWQ5NmRiIDEwMDY0NAotLS0gYS9Tb3VyY2UvUGxhdGZvcm0vQ2hhbmdlTG9nCisrKyBi L1NvdXJjZS9QbGF0Zm9ybS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAxMy0wMi0wNiAg Vy4gSmFtZXMgTWFjTGVhbiAgPHdqbWFjbGVhbkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgW2No cm9taXVtXSBEZWZhdWx0IHNjcm9sbGJhciB0aGVtZSBzaG91bGQgYmUgc2hhcmVkIHdpdGggQ0Mg Y29tcG9zaXRvci4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTEwOTA2NAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgICogY2hyb21pdW0vcHVibGljL1dlYkNvbXBvc2l0b3JTdXBwb3J0Lmg6CisgICAgICAgIChX ZWJDb21wb3NpdG9yU3VwcG9ydCk6CisgICAgICAgIChXZWJLaXQ6OldlYkNvbXBvc2l0b3JTdXBw b3J0OjpzZXREZWZhdWx0V2ViU2Nyb2xsYmFyVGhlbWVHZW9tZXRyeSk6CisgICAgICAgIChXZWJL aXQ6OldlYkNvbXBvc2l0b3JTdXBwb3J0OjpzZXREZWZhdWx0VGhlbWVVc2VzT3ZlcmxheVNjcm9s bGJhcnMpOgorCiAyMDEzLTAyLTA2ICBUb21teSBXaWRlbmZseWNodCAgPHRvbW15d0Bnb29nbGUu Y29tPgogCiAgICAgICAgIE1lZGlhU3RyZWFtIEFQSTogSW1wbGVtZW50IERUTUYgc3VwcG9ydCBp biBSVENQZWVyQ29ubmVjdGlvbgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9n IGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCmluZGV4IDVlMTQxMzI5NmNmODJhMWFlNTgyM2Y3 OGQzYjY0OGU3YjMwYzYwYmYuLjk2Mjg1YTc1Yzk4ZDU3ODY1N2JlN2Y2ZmI2MzBmMWJhOGRkY2My NzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAxMy0wMi0wNiAgVy4gSmFtZXMgTWFj TGVhbiAgPHdqbWFjbGVhbkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgW2Nocm9taXVtXSBEZWZh dWx0IHNjcm9sbGJhciB0aGVtZSBzaG91bGQgYmUgc2hhcmVkIHdpdGggQ0MgY29tcG9zaXRvci4K KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwOTA2NAor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFBsdW1iaW5n LCBubyBuZXcgdGVzdHMuCisKKyAgICAgICAgKiBwYWdlL3Njcm9sbGluZy9jaHJvbWl1bS9TY3Jv bGxpbmdDb29yZGluYXRvckNocm9taXVtLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNjcm9sbGlu Z0Nvb3JkaW5hdG9yQ2hyb21pdW06OlNjcm9sbGluZ0Nvb3JkaW5hdG9yQ2hyb21pdW0pOgorCiAy MDEzLTAyLTA2ICBWc2V2b2xvZCBWbGFzb3YgIDx2c2V2aWtAY2hyb21pdW0ub3JnPgogCiAgICAg ICAgIFdlYiBJbnNwZWN0b3I6IFJlbW92ZSBpc1NuaXBwZXQgZmllbGQgZnJvbSBGaWxlRGVzY3Jp cHRvciBhbmQgVUlTb3VyY2VDb2RlLgpkaWZmIC0tZ2l0IGEvU291cmNlL1BsYXRmb3JtL2Nocm9t aXVtL3B1YmxpYy9XZWJDb21wb3NpdG9yU3VwcG9ydC5oIGIvU291cmNlL1BsYXRmb3JtL2Nocm9t aXVtL3B1YmxpYy9XZWJDb21wb3NpdG9yU3VwcG9ydC5oCmluZGV4IDVjN2Y3MzMwYTdlYWZhMDE3 OTViMGI5NGQwOWE3ZDI2NmZkNGViY2EuLjMyZGZhOGY0MjQ0YTBjZWUzNjI1ODUyNzIxZjdmMjc5 ZTg4NmI5YzggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9QbGF0Zm9ybS9jaHJvbWl1bS9wdWJsaWMvV2Vi Q29tcG9zaXRvclN1cHBvcnQuaAorKysgYi9Tb3VyY2UvUGxhdGZvcm0vY2hyb21pdW0vcHVibGlj L1dlYkNvbXBvc2l0b3JTdXBwb3J0LmgKQEAgLTEwNyw2ICsxMDcsMTEgQEAgcHVibGljOgogCiAg ICAgdmlydHVhbCBXZWJUcmFuc2Zvcm1PcGVyYXRpb25zKiBjcmVhdGVUcmFuc2Zvcm1PcGVyYXRp b25zKCkgeyByZXR1cm4gMDsgfQogCisgICAgLy8gV2ViU2Nyb2xsYmFyVGhlbWVHZW9tZXRyeQor CisgICAgdmlydHVhbCB2b2lkIHNldERlZmF1bHRXZWJTY3JvbGxiYXJUaGVtZUdlb21ldHJ5KFdl YlNjcm9sbGJhclRoZW1lR2VvbWV0cnkqKSB7IH0KKyAgICB2aXJ0dWFsIHZvaWQgc2V0RGVmYXVs dFRoZW1lVXNlc092ZXJsYXlTY3JvbGxiYXJzKGJvb2wpIHsgfQorCiBwcm90ZWN0ZWQ6CiAgICAg dmlydHVhbCB+V2ViQ29tcG9zaXRvclN1cHBvcnQoKSB7IH0KIH07CmRpZmYgLS1naXQgYS9Tb3Vy Y2UvV2ViQ29yZS9wYWdlL3Njcm9sbGluZy9jaHJvbWl1bS9TY3JvbGxpbmdDb29yZGluYXRvckNo cm9taXVtLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BhZ2Uvc2Nyb2xsaW5nL2Nocm9taXVtL1Njcm9s bGluZ0Nvb3JkaW5hdG9yQ2hyb21pdW0uY3BwCmluZGV4IGE3NDFmOGQwN2RhOTY4NTc3NDkwYWFh YjgxZTljNzg4MmM5YjAwNGQuLjQ4NDJmNzUyNGVmYWMyMGY1YTU0NDdiMjA0YjFmYTRlY2U0ODc2 NzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BhZ2Uvc2Nyb2xsaW5nL2Nocm9taXVtL1Nj cm9sbGluZ0Nvb3JkaW5hdG9yQ2hyb21pdW0uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BhZ2Uv c2Nyb2xsaW5nL2Nocm9taXVtL1Njcm9sbGluZ0Nvb3JkaW5hdG9yQ2hyb21pdW0uY3BwCkBAIC05 OSw2ICs5OSwxMiBAQCBTY3JvbGxpbmdDb29yZGluYXRvckNocm9taXVtOjpTY3JvbGxpbmdDb29y ZGluYXRvckNocm9taXVtKFBhZ2UqIHBhZ2UpCiAgICAgOiBTY3JvbGxpbmdDb29yZGluYXRvcihw YWdlKQogICAgICwgbV9wcml2YXRlKG5ldyBTY3JvbGxpbmdDb29yZGluYXRvclByaXZhdGUpCiB7 CisgICAgLy8gQ3JlYXRlIGEgV2ViU2Nyb2xsYmFyVGhlbWVHZW9tZXRyeSB0byBzaGFyZSB3aXRo IGNvbXBvc2l0b3IuCisgICAgU2Nyb2xsYmFyVGhlbWVDb21wb3NpdGUqIHRoZW1lID0gc3RhdGlj X2Nhc3Q8U2Nyb2xsYmFyVGhlbWVDb21wb3NpdGUqPihTY3JvbGxiYXJUaGVtZTo6dGhlbWUoKSk7 CisgICAgT3duUHRyPFdlYktpdDo6V2ViU2Nyb2xsYmFyVGhlbWVHZW9tZXRyeT4gZ2VvbWV0cnko V2ViS2l0OjpXZWJTY3JvbGxiYXJUaGVtZUdlb21ldHJ5TmF0aXZlOjpjcmVhdGUodGhlbWUpKTsK KworICAgIFdlYktpdDo6UGxhdGZvcm06OmN1cnJlbnQoKS0+Y29tcG9zaXRvclN1cHBvcnQoKS0+ c2V0RGVmYXVsdFdlYlNjcm9sbGJhclRoZW1lR2VvbWV0cnkoZ2VvbWV0cnkubGVha1B0cigpKTsK KyAgICBXZWJLaXQ6OlBsYXRmb3JtOjpjdXJyZW50KCktPmNvbXBvc2l0b3JTdXBwb3J0KCktPnNl dERlZmF1bHRUaGVtZVVzZXNPdmVybGF5U2Nyb2xsYmFycyh0aGVtZS0+dXNlc092ZXJsYXlTY3Jv bGxiYXJzKCkpOwogfQogCiBTY3JvbGxpbmdDb29yZGluYXRvckNocm9taXVtOjp+U2Nyb2xsaW5n Q29vcmRpbmF0b3JDaHJvbWl1bSgpCg==