79811 2012-02-28 11:09:55 -0800 Reimplement pathFromFont() in SimpleFontDataMac.mm 2012-02-28 14:22:09 -0800 1 1 1 Unclassified WebKit Text 528+ (Nightly build) Mac OS X 10.6 RESOLVED WONTFIX P2 Enhancement --- 79833 0 ned webkit-unassigned adamk mitz webkit.review.bot oldest_to_newest 566881 0 ned 2012-02-28 11:09:55 -0800 It certainly can't hurt to fix this debugging function. 566895 1 129296 ned 2012-02-28 11:14:48 -0800 Created attachment 129296 Proposed changes. 566913 2 129296 mitz 2012-02-28 11:30:53 -0800 Comment on attachment 129296 Proposed changes. View in context: https://bugs.webkit.org/attachment.cgi?id=129296&action=review OK. I think you could have just as well removed this function and changed its callers. > Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm:85 > +static NSString* pathFromFont(NSFont* font) Spaces should go before the star in Objective-C objects (I know this was already wrong in some of this code). > Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm:87 > + return [[[font fontDescriptor] objectForKey:(NSString*)kCTFontURLAttribute] path]; Needs a space before the *. 566922 3 129299 ned 2012-02-28 11:35:54 -0800 Created attachment 129299 Changes per review. I fixed the spaces, but I could always remove it if you think it's not valuable. 566968 4 129299 webkit.review.bot 2012-02-28 12:29:08 -0800 Comment on attachment 129299 Changes per review. Clearing flags on attachment: 129299 Committed r109137: <http://trac.webkit.org/changeset/109137> 566969 5 webkit.review.bot 2012-02-28 12:29:12 -0800 All reviewed patches have been landed. Closing bug. 567043 6 adamk 2012-02-28 13:40:09 -0800 This breaks the Chromium Mac builders, I think because kCTFontURLAttribute is not available on Leopard. Sample failure: http://build.chromium.org/p/chromium.webkit/builders/Mac%20Builder%20%28dbg%29/builds/4938/steps/compile/logs/stdio Given that this is just for debugging I'd prefer to simply roll this out for now, please speak up if you disagree. 567045 7 mitz 2012-02-28 13:43:03 -0800 (In reply to comment #6) > This breaks the Chromium Mac builders, I think because kCTFontURLAttribute is not available on Leopard. Sorry about that. > Sample failure: > > http://build.chromium.org/p/chromium.webkit/builders/Mac%20Builder%20%28dbg%29/builds/4938/steps/compile/logs/stdio > > Given that this is just for debugging I'd prefer to simply roll this out for now, please speak up if you disagree. Yes, feel free to revert this change. 567059 8 adamk 2012-02-28 13:51:04 -0800 Rolled out in http://trac.webkit.org/changeset/109146 567070 9 ned 2012-02-28 14:02:38 -0800 Leopard? Hmm. Given the earlier comments, what say I just rip it out altogether? 567076 10 mitz 2012-02-28 14:10:39 -0800 I’m still ok with that. Or just not touching it. 567090 11 ned 2012-02-28 14:22:09 -0800 Okay, I'll let sleeping dogs lie. 129296 2012-02-28 11:14:48 -0800 2012-02-28 11:35:54 -0800 Proposed changes. pathFromFont.txt text/plain 2086 ned SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwOTEyMykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDEyLTAyLTI4ICBOZWQgSG9s YnJvb2sgIDxuaG9sYnJvb2tAYXBwbGUuY29tPgorCisgICAgICAgIFJlaW1wbGVtZW50IHBhdGhG cm9tRm9udCgpIGluIFNpbXBsZUZvbnREYXRhTWFjLm1tCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03OTgxMQorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIERlYnVnLW9ubHkgZnVuY3Rpb24sIHNvIG5vIG5ldyB0 ZXN0cy4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL21hYy9TaW1wbGVGb250RGF0YU1h Yy5tbToKKyAgICAgICAgKFdlYkNvcmU6OnBhdGhGcm9tRm9udCk6IFJlaW1wbGVtZW50ZWQuCisK IDIwMTItMDItMjggIEFkYW0gS2xlaW4gIDxhZGFta0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAg VW5yZXZpZXdlZCwgc3BlY3VsYXRpdmUgdGVzdCBmaXggYWZ0ZXIgcjEwOTAxNi4KSW5kZXg6IFNv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL21hYy9TaW1wbGVGb250RGF0YU1hYy5tbQo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9tYWMvU2ltcGxl Rm9udERhdGFNYWMubW0JKHJldmlzaW9uIDEwOTA4MCkKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2dyYXBoaWNzL21hYy9TaW1wbGVGb250RGF0YU1hYy5tbQkod29ya2luZyBjb3B5KQpAQCAt ODIsMzEgKzgyLDEwIEBAIHN0YXRpYyBOU1N0cmluZyAqd2ViRmFsbGJhY2tGb250RmFtaWx5KHYK IH0KIAogI2lmICFFUlJPUl9ESVNBQkxFRAotI2lmIGRlZmluZWQoX19MUDY0X18pIHx8ICghZGVm aW5lZChCVUlMRElOR19PTl9MRU9QQVJEKSAmJiAhZGVmaW5lZChCVUlMRElOR19PTl9TTk9XX0xF T1BBUkQpKQotc3RhdGljIE5TU3RyaW5nKiBwYXRoRnJvbUZvbnQoTlNGb250KikKK3N0YXRpYyBO U1N0cmluZyogcGF0aEZyb21Gb250KE5TRm9udCogZm9udCkKIHsKLSAgICAvLyBGTUdldEFUU0Zv bnRSZWZGcm9tRm9udCBpcyBub3QgYXZhaWxhYmxlLiBBcyBwYXRoRnJvbUZvbnQgaXMgb25seSB1 c2VkIGZvciBkZWJ1Z2dpbmcgcHVycG9zZXMsCi0gICAgLy8gcmV0dXJuaW5nIG5pbCBpcyBhY2Nl cHRhYmxlLgotICAgIHJldHVybiBuaWw7CisgICAgcmV0dXJuIFtbW2ZvbnQgZm9udERlc2NyaXB0 b3JdIG9iamVjdEZvcktleTooTlNTdHJpbmcqKWtDVEZvbnRVUkxBdHRyaWJ1dGVdIHBhdGhdOwog fQotI2Vsc2UKLXN0YXRpYyBOU1N0cmluZyogcGF0aEZyb21Gb250KE5TRm9udCAqZm9udCkKLXsK LSAgICBBVFNGb250UmVmIGF0c0ZvbnQgPSBGTUdldEFUU0ZvbnRSZWZGcm9tRm9udChDVEZvbnRH ZXRQbGF0Zm9ybUZvbnQodG9DVEZvbnRSZWYoZm9udCksIDApKTsKLSAgICBGU1JlZiBmaWxlUmVm OwotCi0gICAgT1NTdGF0dXMgc3RhdHVzID0gQVRTRm9udEdldEZpbGVSZWZlcmVuY2UoYXRzRm9u dCwgJmZpbGVSZWYpOwotICAgIGlmIChzdGF0dXMgIT0gbm9FcnIpCi0gICAgICAgIHJldHVybiBu aWw7Ci0KLSAgICBVSW50OCBmaWxlUGF0aEJ1ZmZlcltQQVRIX01BWF07Ci0gICAgc3RhdHVzID0g RlNSZWZNYWtlUGF0aCgmZmlsZVJlZiwgZmlsZVBhdGhCdWZmZXIsIFBBVEhfTUFYKTsKLSAgICBp ZiAoc3RhdHVzID09IG5vRXJyKQotICAgICAgICByZXR1cm4gW05TU3RyaW5nIHN0cmluZ1dpdGhV VEY4U3RyaW5nOihjb25zdCBjaGFyKilmaWxlUGF0aEJ1ZmZlcl07Ci0KLSAgICByZXR1cm4gbmls OwotfQotI2VuZGlmIC8vIF9fTFA2NF9fCiAjZW5kaWYgLy8gIUVSUk9SX0RJU0FCTEVECiAKIHZv aWQgU2ltcGxlRm9udERhdGE6OnBsYXRmb3JtSW5pdCgpCg== 129299 2012-02-28 11:35:54 -0800 2012-02-28 12:29:08 -0800 Changes per review. pathFromFont2.txt text/plain 2087 ned SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwOTEyMykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDEyLTAyLTI4ICBOZWQgSG9s YnJvb2sgIDxuaG9sYnJvb2tAYXBwbGUuY29tPgorCisgICAgICAgIFJlaW1wbGVtZW50IHBhdGhG cm9tRm9udCgpIGluIFNpbXBsZUZvbnREYXRhTWFjLm1tCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03OTgxMQorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIERlYnVnLW9ubHkgZnVuY3Rpb24sIHNvIG5vIG5ldyB0 ZXN0cy4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL21hYy9TaW1wbGVGb250RGF0YU1h Yy5tbToKKyAgICAgICAgKFdlYkNvcmU6OnBhdGhGcm9tRm9udCk6IFJlaW1wbGVtZW50ZWQuCisK IDIwMTItMDItMjggIEFkYW0gS2xlaW4gIDxhZGFta0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAg VW5yZXZpZXdlZCwgc3BlY3VsYXRpdmUgdGVzdCBmaXggYWZ0ZXIgcjEwOTAxNi4KSW5kZXg6IFNv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL21hYy9TaW1wbGVGb250RGF0YU1hYy5tbQo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9tYWMvU2ltcGxl Rm9udERhdGFNYWMubW0JKHJldmlzaW9uIDEwOTA4MCkKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2dyYXBoaWNzL21hYy9TaW1wbGVGb250RGF0YU1hYy5tbQkod29ya2luZyBjb3B5KQpAQCAt ODIsMzEgKzgyLDEwIEBAIHN0YXRpYyBOU1N0cmluZyAqd2ViRmFsbGJhY2tGb250RmFtaWx5KHYK IH0KIAogI2lmICFFUlJPUl9ESVNBQkxFRAotI2lmIGRlZmluZWQoX19MUDY0X18pIHx8ICghZGVm aW5lZChCVUlMRElOR19PTl9MRU9QQVJEKSAmJiAhZGVmaW5lZChCVUlMRElOR19PTl9TTk9XX0xF T1BBUkQpKQotc3RhdGljIE5TU3RyaW5nKiBwYXRoRnJvbUZvbnQoTlNGb250KikKK3N0YXRpYyBO U1N0cmluZyAqcGF0aEZyb21Gb250KE5TRm9udCAqZm9udCkKIHsKLSAgICAvLyBGTUdldEFUU0Zv bnRSZWZGcm9tRm9udCBpcyBub3QgYXZhaWxhYmxlLiBBcyBwYXRoRnJvbUZvbnQgaXMgb25seSB1 c2VkIGZvciBkZWJ1Z2dpbmcgcHVycG9zZXMsCi0gICAgLy8gcmV0dXJuaW5nIG5pbCBpcyBhY2Nl cHRhYmxlLgotICAgIHJldHVybiBuaWw7CisgICAgcmV0dXJuIFtbW2ZvbnQgZm9udERlc2NyaXB0 b3JdIG9iamVjdEZvcktleTooTlNTdHJpbmcgKilrQ1RGb250VVJMQXR0cmlidXRlXSBwYXRoXTsK IH0KLSNlbHNlCi1zdGF0aWMgTlNTdHJpbmcqIHBhdGhGcm9tRm9udChOU0ZvbnQgKmZvbnQpCi17 Ci0gICAgQVRTRm9udFJlZiBhdHNGb250ID0gRk1HZXRBVFNGb250UmVmRnJvbUZvbnQoQ1RGb250 R2V0UGxhdGZvcm1Gb250KHRvQ1RGb250UmVmKGZvbnQpLCAwKSk7Ci0gICAgRlNSZWYgZmlsZVJl ZjsKLQotICAgIE9TU3RhdHVzIHN0YXR1cyA9IEFUU0ZvbnRHZXRGaWxlUmVmZXJlbmNlKGF0c0Zv bnQsICZmaWxlUmVmKTsKLSAgICBpZiAoc3RhdHVzICE9IG5vRXJyKQotICAgICAgICByZXR1cm4g bmlsOwotCi0gICAgVUludDggZmlsZVBhdGhCdWZmZXJbUEFUSF9NQVhdOwotICAgIHN0YXR1cyA9 IEZTUmVmTWFrZVBhdGgoJmZpbGVSZWYsIGZpbGVQYXRoQnVmZmVyLCBQQVRIX01BWCk7Ci0gICAg aWYgKHN0YXR1cyA9PSBub0VycikKLSAgICAgICAgcmV0dXJuIFtOU1N0cmluZyBzdHJpbmdXaXRo VVRGOFN0cmluZzooY29uc3QgY2hhciopZmlsZVBhdGhCdWZmZXJdOwotCi0gICAgcmV0dXJuIG5p bDsKLX0KLSNlbmRpZiAvLyBfX0xQNjRfXwogI2VuZGlmIC8vICFFUlJPUl9ESVNBQkxFRAogCiB2 b2lkIFNpbXBsZUZvbnREYXRhOjpwbGF0Zm9ybUluaXQoKQo=