61800 2011-05-31 12:56:54 -0700 Ready Chromium port for Skia on Mac 2011-06-01 10:20:00 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 caryclark caryclark commit-queue eric fishd oldest_to_newest 412809 0 caryclark 2011-05-31 12:56:54 -0700 Ready Chromium port for Skia on Mac 412810 1 95466 caryclark 2011-05-31 13:04:19 -0700 Created attachment 95466 Patch 413088 2 95466 eric 2011-05-31 19:35:11 -0700 Comment on attachment 95466 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95466&action=review Seems OK, but could be cleaner. > Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:412 > + SkCanvas* canvas = context->platformContext()->canvas(); Seems like we want a direct accessor here somehow. Or maybe a skia(context) helper function. > Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:438 > + gfx::SkiaBitLocker bitLocker(drawingContext->platformContext()->canvas()); We might want a WebCore helper which knows how to bitlock directly from a GraphicsContext instead of having to do this canvas lookup every time. We could also use an abstraction around the drawingContext which would lock in teh skia case, and otherwise just give you back the CGContext. That would remove a bunch of these #if USE(SKIA) branches. 413309 3 95466 caryclark 2011-06-01 05:40:16 -0700 Comment on attachment 95466 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95466&action=review >> Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:438 >> + gfx::SkiaBitLocker bitLocker(drawingContext->platformContext()->canvas()); > > We might want a WebCore helper which knows how to bitlock directly from a GraphicsContext instead of having to do this canvas lookup every time. > > We could also use an abstraction around the drawingContext which would lock in teh skia case, and otherwise just give you back the CGContext. That would remove a bunch of these #if USE(SKIA) branches. The bitlock/canvas lookup combination only happens four times in the sources. Also, the #if USE(SKIA) scaffolding is temporary; once the Chrome Mac port uses Skia, it will be removed. 413411 4 95466 eric 2011-06-01 08:57:11 -0700 Comment on attachment 95466 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95466&action=review >>> Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:438 >>> + gfx::SkiaBitLocker bitLocker(drawingContext->platformContext()->canvas()); >> >> We might want a WebCore helper which knows how to bitlock directly from a GraphicsContext instead of having to do this canvas lookup every time. >> >> We could also use an abstraction around the drawingContext which would lock in teh skia case, and otherwise just give you back the CGContext. That would remove a bunch of these #if USE(SKIA) branches. > > The bitlock/canvas lookup combination only happens four times in the sources. Also, the #if USE(SKIA) scaffolding is temporary; once the Chrome Mac port uses Skia, it will be removed. I don't think "temporary" is every a very good excuse for ugly code. But I think this patch is OK. Best of luck. :) 413465 5 95466 commit-queue 2011-06-01 10:19:55 -0700 Comment on attachment 95466 Patch Clearing flags on attachment: 95466 Committed r87821: <http://trac.webkit.org/changeset/87821> 413466 6 commit-queue 2011-06-01 10:20:00 -0700 All reviewed patches have been landed. Closing bug. 95466 2011-05-31 13:04:19 -0700 2011-06-01 10:19:55 -0700 Patch bug-61800-20110531160417.patch text/plain 4831 caryclark SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg3NzQ3KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjUgQEAKKzIwMTEtMDUtMzEgIENhcnkgQ2xh cmsgIDxjYXJ5Y2xhcmtAZ29vZ2xlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBSZWFkeSBDaHJvbWl1bSBwb3J0IGZvciBTa2lhIG9uIE1hYwor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjE4MDAKKwor ICAgICAgICBTa2lhIG9uIE1hYyBpcyBub3QgZW5hYmxlZC4gVGhlIGV4ZWN1dGluZworICAgICAg ICBjb2RlIGlzIHVuY2hhbmdlZCwgc28gdGhlcmUgYXJlIG5vIG5ldyB0ZXN0cy4KKworICAgICAg ICAqIHBsYXRmb3JtL2Nocm9taXVtL0RyYWdJbWFnZVJlZi5oOgorICAgICAgICBVc2UgU2tpYSBm b3IgRHJhZ0ltYWdlUmVmIGluc3RlYWQgb2YgQ0cuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9jaHJv bWl1bS9TY3JvbGxiYXJUaGVtZUNocm9taXVtTWFjLm1tOgorICAgICAgICAoV2ViQ29yZTo6U2Ny b2xsYmFyVGhlbWVDaHJvbWl1bU1hYzo6cGFpbnQpOgorICAgICAgICBHZXQgdGhlIHRvdGFsIG1h dHJpeCBmcm9tIFNraWEgZm9yIHRoZSBzY3JvbGxiYXIuCisgICAgICAgIENvbnZlcnQgdGhlIFNr Q2FudmFzIGludG8gYSBDR0NvbnRleHQgdG8gZHJhdy4KKworICAgICAgICAqIHBsYXRmb3JtL2No cm9taXVtL1RoZW1lQ2hyb21pdW1NYWMubW06CisgICAgICAgIChXZWJDb3JlOjpwYWludFN0ZXBw ZXIpOgorICAgICAgICBEaXR0by4KKwogMjAxMS0wNS0zMSAgUm9iIEJ1aXMgIDxyYnVpc0ByaW0u Y29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERpcmsgU2NodWx6ZS4KSW5kZXg6IFNvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2Nocm9taXVtL0RyYWdJbWFnZVJlZi5oCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9taXVtL0RyYWdJbWFnZVJlZi5oCShyZXZpc2lvbiA4 Nzc0MCkKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9taXVtL0RyYWdJbWFnZVJlZi5o CSh3b3JraW5nIGNvcHkpCkBAIC0yOSw3ICsyOSw3IEBACiAjaWZuZGVmIERyYWdJbWFnZVJlZl9o CiAjZGVmaW5lIERyYWdJbWFnZVJlZl9oCiAKLSNpZiBPUyhEQVJXSU4pCisjaWYgT1MoREFSV0lO KSAmJiAhVVNFKFNLSUFfT05fTUFDX0NIUk9NRSkKIHR5cGVkZWYgc3RydWN0IENHSW1hZ2UqIENH SW1hZ2VSZWY7CiAjZWxzZQogY2xhc3MgU2tCaXRtYXA7CkBAIC0zNyw3ICszNyw3IEBAIGNsYXNz IFNrQml0bWFwOwogCiBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKLSNpZiBPUyhEQVJXSU4pCisjaWYg T1MoREFSV0lOKSAmJiAhVVNFKFNLSUFfT05fTUFDX0NIUk9NRSkKIHR5cGVkZWYgQ0dJbWFnZVJl ZiBEcmFnSW1hZ2VSZWY7CiAjZWxzZQogdHlwZWRlZiBTa0JpdG1hcCogRHJhZ0ltYWdlUmVmOwpJ bmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY2hyb21pdW0vU2Nyb2xsYmFyVGhlbWVDaHJv bWl1bU1hYy5tbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1 bS9TY3JvbGxiYXJUaGVtZUNocm9taXVtTWFjLm1tCShyZXZpc2lvbiA4Nzc0MCkKKysrIFNvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9taXVtL1Njcm9sbGJhclRoZW1lQ2hyb21pdW1NYWMubW0J KHdvcmtpbmcgY29weSkKQEAgLTM2LDYgKzM2LDEwIEBACiAjaW5jbHVkZSA8d3RmL1N0ZExpYkV4 dHJhcy5oPgogI2luY2x1ZGUgPHd0Zi9VbnVzZWRQYXJhbS5oPgogCisjaWYgVVNFKFNLSUEpCisj aW5jbHVkZSAiUGxhdGZvcm1Db250ZXh0U2tpYS5oIgorI2luY2x1ZGUgInNraWEvZXh0L3NraWFf dXRpbHNfbWFjLmgiCisjZW5kaWYKIAogLy8gRklYTUU6IFRoZXJlIGFyZSByZXBhaW50aW5nIHBy b2JsZW1zIGR1ZSB0byBBcXVhIHNjcm9sbCBiYXIgYnV0dG9ucycgdmlzdWFsIG92ZXJmbG93Lgog CkBAIC00MDQsNyArNDA4LDEyIEBAIGJvb2wgU2Nyb2xsYmFyVGhlbWVDaHJvbWl1bU1hYzo6cGFp bnQoU2MKICAgICAgICAgdHJhY2tJbmZvLmVuYWJsZVN0YXRlID0ga1RoZW1lVHJhY2tOb3RoaW5n VG9TY3JvbGw7CiAgICAgdHJhY2tJbmZvLnRyYWNrSW5mby5zY3JvbGxiYXIucHJlc3NTdGF0ZSA9 IHNjcm9sbGJhclBhcnRUb0hJUHJlc3NlZFN0YXRlKHNjcm9sbGJhci0+cHJlc3NlZFBhcnQoKSk7 CiAKKyNpZiBVU0UoU0tJQSkKKyAgICBTa0NhbnZhcyogY2FudmFzID0gY29udGV4dC0+cGxhdGZv cm1Db250ZXh0KCktPmNhbnZhcygpOworICAgIENHQWZmaW5lVHJhbnNmb3JtIGN1cnJlbnRDVE0g PSBnZng6OlNrTWF0cml4VG9DR0FmZmluZVRyYW5zZm9ybShjYW52YXMtPmdldFRvdGFsTWF0cml4 KCkpOworI2Vsc2UKICAgICBDR0FmZmluZVRyYW5zZm9ybSBjdXJyZW50Q1RNID0gQ0dDb250ZXh0 R2V0Q1RNKGNvbnRleHQtPnBsYXRmb3JtQ29udGV4dCgpKTsKKyNlbmRpZgogCiAgICAgLy8gVGhl IEFxdWEgc2Nyb2xsYmFyIGlzIGJ1Z2d5IHdoZW4gcm90YXRlZCBhbmQgc2NhbGVkLiAgV2Ugd2ls bCBqdXN0IGRyYXcgaW50byBhIGJpdG1hcCBpZiB3ZSBkZXRlY3QgYSBzY2FsZSBvciByb3RhdGlv bi4KICAgICBib29sIGNhbkRyYXdEaXJlY3RseSA9IGN1cnJlbnRDVE0uYSA9PSAxLjBmICYmIGN1 cnJlbnRDVE0uYiA9PSAwLjBmICYmIGN1cnJlbnRDVE0uYyA9PSAwLjBmICYmIChjdXJyZW50Q1RN LmQgPT0gMS4wZiB8fCBjdXJyZW50Q1RNLmQgPT0gLTEuMGYpOwpAQCAtNDI1LDcgKzQzNCwxMyBA QCBib29sIFNjcm9sbGJhclRoZW1lQ2hyb21pdW1NYWM6OnBhaW50KFNjCiAgICAgfQogCiAgICAg Ly8gRHJhdyB0aHVtYmxlc3MuCi0gICAgSElUaGVtZURyYXdUcmFjaygmdHJhY2tJbmZvLCAwLCBk cmF3aW5nQ29udGV4dC0+cGxhdGZvcm1Db250ZXh0KCksIGtISVRoZW1lT3JpZW50YXRpb25Ob3Jt YWwpOworI2lmIFVTRShTS0lBKQorICAgIGdmeDo6U2tpYUJpdExvY2tlciBiaXRMb2NrZXIoZHJh d2luZ0NvbnRleHQtPnBsYXRmb3JtQ29udGV4dCgpLT5jYW52YXMoKSk7CisgICAgQ0dDb250ZXh0 UmVmIGNnQ29udGV4dCA9IGJpdExvY2tlci5jZ0NvbnRleHQoKTsKKyNlbHNlCisgICAgQ0dDb250 ZXh0UmVmIGNnQ29udGV4dCA9IGRyYXdpbmdDb250ZXh0LT5wbGF0Zm9ybUNvbnRleHQoKTsKKyNl bmRpZgorICAgIEhJVGhlbWVEcmF3VHJhY2soJnRyYWNrSW5mbywgMCwgY2dDb250ZXh0LCBrSElU aGVtZU9yaWVudGF0aW9uTm9ybWFsKTsKIAogICAgIFZlY3RvcjxJbnRSZWN0PiB0aWNrbWFya3M7 CiAgICAgc2Nyb2xsYmFyLT5zY3JvbGxhYmxlQXJlYSgpLT5nZXRUaWNrbWFya3ModGlja21hcmtz KTsKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9taXVtL1RoZW1lQ2hyb21pdW1N YWMubW0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY2hyb21pdW0vVGhl bWVDaHJvbWl1bU1hYy5tbQkocmV2aXNpb24gODc3NDApCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9jaHJvbWl1bS9UaGVtZUNocm9taXVtTWFjLm1tCSh3b3JraW5nIGNvcHkpCkBAIC0zNiw2 ICszNiwxMSBAQAogI2luY2x1ZGUgPHd0Zi9TdGRMaWJFeHRyYXMuaD4KICNpbXBvcnQgPG9iamMv cnVudGltZS5oPgogCisjaWYgVVNFKFNLSUEpCisjaW5jbHVkZSAiUGxhdGZvcm1Db250ZXh0U2tp YS5oIgorI2luY2x1ZGUgInNraWEvZXh0L3NraWFfdXRpbHNfbWFjLmgiCisjZW5kaWYKKwogdXNp bmcgbmFtZXNwYWNlIHN0ZDsKIAogLy8gVGhpcyBmaWxlIChhbmQgaXRzIGFzc29jaWF0ZWQgLmgg ZmlsZSkgaXMgYSBjbG9uZSBvZiBUaGVtZU1hYy5tbS4KQEAgLTY1Miw3ICs2NTcsMTMgQEAgc3Rh dGljIHZvaWQgcGFpbnRTdGVwcGVyKENvbnRyb2xTdGF0ZXMgcwogICAgICAgICBib3VuZHMub3Jp Z2luLnggKz0gYm91bmRzLm9yaWdpbi54IC0gYmFja2dyb3VuZEJvdW5kcy5vcmlnaW4ueDsKICAg ICBpZiAoYm91bmRzLm9yaWdpbi55ICE9IGJhY2tncm91bmRCb3VuZHMub3JpZ2luLnkpCiAgICAg ICAgIGJvdW5kcy5vcmlnaW4ueSArPSBib3VuZHMub3JpZ2luLnkgLSBiYWNrZ3JvdW5kQm91bmRz Lm9yaWdpbi55OwotICAgIEhJVGhlbWVEcmF3QnV0dG9uKCZib3VuZHMsICZkcmF3SW5mbywgY29u dGV4dC0+cGxhdGZvcm1Db250ZXh0KCksIGtISVRoZW1lT3JpZW50YXRpb25Ob3JtYWwsIDApOwor I2lmIFVTRShTS0lBKQorICAgIGdmeDo6U2tpYUJpdExvY2tlciBiaXRMb2NrZXIoY29udGV4dC0+ cGxhdGZvcm1Db250ZXh0KCktPmNhbnZhcygpKTsKKyAgICBDR0NvbnRleHRSZWYgY2dDb250ZXh0 ID0gYml0TG9ja2VyLmNnQ29udGV4dCgpOworI2Vsc2UKKyAgICBDR0NvbnRleHRSZWYgY2dDb250 ZXh0ID0gY29udGV4dC0+cGxhdGZvcm1Db250ZXh0KCk7CisjZW5kaWYKKyAgICBISVRoZW1lRHJh d0J1dHRvbigmYm91bmRzLCAmZHJhd0luZm8sIGNnQ29udGV4dCwga0hJVGhlbWVPcmllbnRhdGlv bk5vcm1hbCwgMCk7CiAgICAgY29udGV4dC0+cmVzdG9yZSgpOwogfQogCg==