67728 2011-09-07 12:58:13 -0700 Refactor hyphen measurement 2011-09-08 12:17:53 -0700 1 1 1 Unclassified WebKit Text 528+ (Nightly build) All All RESOLVED FIXED P2 Enhancement --- 0 ned webkit-unassigned webkit.review.bot oldest_to_newest 463589 0 ned 2011-09-07 12:58:13 -0700 It would be nice to avoid duplicated code in RenderBlockLineLayout.cpp by adding a helper function to measure the hyphen string. 463593 1 106623 ned 2011-09-07 13:00:19 -0700 Created attachment 106623 Proposed changes. 463637 2 106623 darin 2011-09-07 14:02:14 -0700 Comment on attachment 106623 Proposed changes. View in context: https://bugs.webkit.org/attachment.cgi?id=106623&action=review > Source/WebCore/rendering/RenderBlockLineLayout.cpp:485 > + RenderStyle* style = renderer->style(); > + const AtomicString& hyphenString = style->hyphenString(); > + return font.width(RenderBlock::constructTextRun(renderer, font, hyphenString.string(), style)); Not sure these local variables help make this more readable. 463704 3 106660 ned 2011-09-07 15:33:38 -0700 Created attachment 106660 Changes per review. How about we split the difference so as to avoid doubly dereferencing |renderer|? 464241 4 106660 webkit.review.bot 2011-09-08 12:17:49 -0700 Comment on attachment 106660 Changes per review. Clearing flags on attachment: 106660 Committed r94778: <http://trac.webkit.org/changeset/94778> 464242 5 webkit.review.bot 2011-09-08 12:17:53 -0700 All reviewed patches have been landed. Closing bug. 106623 2011-09-07 13:00:19 -0700 2011-09-07 15:33:38 -0700 Proposed changes. measureHyphenWidth.txt text/plain 3587 ned SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk0NzAyKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTggQEAKKzIwMTEtMDktMDcgIE5lZCBIb2xi cm9vayAgPG5ob2xicm9va0BhcHBsZS5jb20+CisKKyAgICAgICAgUmVmYWN0b3IgaHlwaGVuIG1l YXN1cmVtZW50CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD02NzcyOAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IE5vIG5ldyB0ZXN0cywgcHVyZWx5IHJlZmFjdG9yaW5nLgorCisgICAgICAgICogcmVuZGVyaW5n L1JlbmRlckJsb2NrTGluZUxheW91dC5jcHA6CisgICAgICAgIChXZWJDb3JlOjptZWFzdXJlSHlw aGVuV2lkdGgpOiBBZGRlZC4KKyAgICAgICAgKFdlYkNvcmU6OnNldExvZ2ljYWxXaWR0aEZvclRl eHRSdW4pOiBVc2UgbWVhc3VyZUh5cGhlbldpZHRoKCkuCisgICAgICAgIChXZWJDb3JlOjp0cnlI eXBoZW5hdGluZyk6IERpdHRvLgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyQmxvY2s6OkxpbmVC cmVha2VyOjpuZXh0TGluZUJyZWFrKTogRGl0dG8uCisKIDIwMTEtMDktMDYgIE9saXZlciBIdW50 ICA8b2xpdmVyQGFwcGxlLmNvbT4KIAogICAgICAgICBSZW1vdmUgSlNPYmplY3RXaXRoR2xvYmFs T2JqZWN0CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyQmxvY2tMaW5lTGF5 b3V0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyQmxv Y2tMaW5lTGF5b3V0LmNwcAkocmV2aXNpb24gOTQ1NzMpCisrKyBTb3VyY2UvV2ViQ29yZS9yZW5k ZXJpbmcvUmVuZGVyQmxvY2tMaW5lTGF5b3V0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDc4LDYg KzQ3OCwxMyBAQCB2b2lkIFJlbmRlckJsb2NrOjpzZXRNYXJnaW5zRm9yUnVieVJ1bihCCiAgICAg c2V0TWFyZ2luRW5kRm9yQ2hpbGQocmVuZGVyZXIsIC1lbmRPdmVyaGFuZyk7CiB9CiAKK3N0YXRp YyBpbmxpbmUgZmxvYXQgbWVhc3VyZUh5cGhlbldpZHRoKFJlbmRlclRleHQqIHJlbmRlcmVyLCBj b25zdCBGb250JiBmb250KQoreworICAgIFJlbmRlclN0eWxlKiBzdHlsZSA9IHJlbmRlcmVyLT5z dHlsZSgpOworICAgIGNvbnN0IEF0b21pY1N0cmluZyYgaHlwaGVuU3RyaW5nID0gc3R5bGUtPmh5 cGhlblN0cmluZygpOworICAgIHJldHVybiBmb250LndpZHRoKFJlbmRlckJsb2NrOjpjb25zdHJ1 Y3RUZXh0UnVuKHJlbmRlcmVyLCBmb250LCBoeXBoZW5TdHJpbmcuc3RyaW5nKCksIHN0eWxlKSk7 Cit9CisKIHN0YXRpYyBpbmxpbmUgdm9pZCBzZXRMb2dpY2FsV2lkdGhGb3JUZXh0UnVuKFJvb3RJ bmxpbmVCb3gqIGxpbmVCb3gsIEJpZGlSdW4qIHJ1biwgUmVuZGVyVGV4dCogcmVuZGVyZXIsIGZs b2F0IHhQb3MsIGNvbnN0IExpbmVJbmZvJiBsaW5lSW5mbywKICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgR2x5cGhPdmVyZmxvd0FuZEZhbGxiYWNrRm9udHNNYXAmIHRleHRCb3hE YXRhTWFwLCBWZXJ0aWNhbFBvc2l0aW9uQ2FjaGUmIHZlcnRpY2FsUG9zaXRpb25DYWNoZSkKIHsK QEAgLTUwMCw5ICs1MDcsOCBAQCBzdGF0aWMgaW5saW5lIHZvaWQgc2V0TG9naWNhbFdpZHRoRm9y VGV4CiAgICAgCiAgICAgaW50IGh5cGhlbldpZHRoID0gMDsKICAgICBpZiAodG9JbmxpbmVUZXh0 Qm94KHJ1bi0+bV9ib3gpLT5oYXNIeXBoZW4oKSkgewotICAgICAgICBjb25zdCBBdG9taWNTdHJp bmcmIGh5cGhlblN0cmluZyA9IHJlbmRlcmVyLT5zdHlsZSgpLT5oeXBoZW5TdHJpbmcoKTsKICAg ICAgICAgY29uc3QgRm9udCYgZm9udCA9IHJlbmRlcmVyLT5zdHlsZShsaW5lSW5mby5pc0ZpcnN0 TGluZSgpKS0+Zm9udCgpOwotICAgICAgICBoeXBoZW5XaWR0aCA9IGZvbnQud2lkdGgoUmVuZGVy QmxvY2s6OmNvbnN0cnVjdFRleHRSdW4ocmVuZGVyZXIsIGZvbnQsIGh5cGhlblN0cmluZy5zdHJp bmcoKSwgcmVuZGVyZXItPnN0eWxlKCkpKTsKKyAgICAgICAgaHlwaGVuV2lkdGggPSBtZWFzdXJl SHlwaGVuV2lkdGgocmVuZGVyZXIsIGZvbnQpOwogICAgIH0KICAgICBydW4tPm1fYm94LT5zZXRM b2dpY2FsV2lkdGgocmVuZGVyZXItPndpZHRoKHJ1bi0+bV9zdGFydCwgcnVuLT5tX3N0b3AgLSBy dW4tPm1fc3RhcnQsIHhQb3MsIGxpbmVJbmZvLmlzRmlyc3RMaW5lKCksICZmYWxsYmFja0ZvbnRz LCAmZ2x5cGhPdmVyZmxvdykgKyBoeXBoZW5XaWR0aCk7CiAgICAgaWYgKCFmYWxsYmFja0ZvbnRz LmlzRW1wdHkoKSkgewpAQCAtMTY5MCw4ICsxNjk2LDcgQEAgc3RhdGljIHZvaWQgdHJ5SHlwaGVu YXRpbmcoUmVuZGVyVGV4dCogdAogICAgIGlmIChwb3MgLSBsYXN0U3BhY2UgPD0gbWluaW11bVN1 ZmZpeExlbmd0aCkKICAgICAgICAgcmV0dXJuOwogCi0gICAgY29uc3QgQXRvbWljU3RyaW5nJiBo eXBoZW5TdHJpbmcgPSB0ZXh0LT5zdHlsZSgpLT5oeXBoZW5TdHJpbmcoKTsKLSAgICBpbnQgaHlw aGVuV2lkdGggPSBmb250LndpZHRoKFJlbmRlckJsb2NrOjpjb25zdHJ1Y3RUZXh0UnVuKHRleHQs IGZvbnQsIGh5cGhlblN0cmluZy5zdHJpbmcoKSwgdGV4dC0+c3R5bGUoKSkpOworICAgIGludCBo eXBoZW5XaWR0aCA9IG1lYXN1cmVIeXBoZW5XaWR0aCh0ZXh0LCBmb250KTsKIAogICAgIGZsb2F0 IG1heFByZWZpeFdpZHRoID0gYXZhaWxhYmxlV2lkdGggLSB4UG9zIC0gaHlwaGVuV2lkdGggLSBs YXN0U3BhY2VXb3JkU3BhY2luZzsKICAgICAvLyBJZiB0aGUgbWF4aW11bSB3aWR0aCBhdmFpbGFi bGUgZm9yIHRoZSBwcmVmaXggYmVmb3JlIHRoZSBoeXBoZW4gaXMgc21hbGwsIHRoZW4gaXQgaXMg dmVyeSB1bmxpa2VseQpAQCAtMjE5MSw4ICsyMTk2LDcgQEAgSW5saW5lSXRlcmF0b3IgUmVuZGVy QmxvY2s6OkxpbmVCcmVha2VyOgogICAgICAgICAgICAgICAgICAgICBsaW5lSW5mby5zZXRFbXB0 eShmYWxzZSk7CiAKICAgICAgICAgICAgICAgICBpZiAoYyA9PSBzb2Z0SHlwaGVuICYmIGF1dG9X cmFwICYmICFoeXBoZW5XaWR0aCAmJiBzdHlsZS0+aHlwaGVucygpICE9IEh5cGhlbnNOb25lKSB7 Ci0gICAgICAgICAgICAgICAgICAgIGNvbnN0IEF0b21pY1N0cmluZyYgaHlwaGVuU3RyaW5nID0g c3R5bGUtPmh5cGhlblN0cmluZygpOwotICAgICAgICAgICAgICAgICAgICBoeXBoZW5XaWR0aCA9 IGYud2lkdGgoY29uc3RydWN0VGV4dFJ1bih0LCBmLCBoeXBoZW5TdHJpbmcuc3RyaW5nKCksIGN1 cnJlbnQubV9vYmotPnN0eWxlKCkpKTsKKyAgICAgICAgICAgICAgICAgICAgaHlwaGVuV2lkdGgg PSBtZWFzdXJlSHlwaGVuV2lkdGgodCwgZik7CiAgICAgICAgICAgICAgICAgICAgIHdpZHRoLmFk ZFVuY29tbWl0dGVkV2lkdGgoaHlwaGVuV2lkdGgpOwogICAgICAgICAgICAgICAgIH0KIAo= 106660 2011-09-07 15:33:38 -0700 2011-09-08 12:17:49 -0700 Changes per review. measureHyphenWidth.txt text/plain 3533 ned SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk0NzAyKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTggQEAKKzIwMTEtMDktMDcgIE5lZCBIb2xi cm9vayAgPG5ob2xicm9va0BhcHBsZS5jb20+CisKKyAgICAgICAgUmVmYWN0b3IgaHlwaGVuIG1l YXN1cmVtZW50CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD02NzcyOAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IE5vIG5ldyB0ZXN0cywgcHVyZWx5IHJlZmFjdG9yaW5nLgorCisgICAgICAgICogcmVuZGVyaW5n L1JlbmRlckJsb2NrTGluZUxheW91dC5jcHA6CisgICAgICAgIChXZWJDb3JlOjptZWFzdXJlSHlw aGVuV2lkdGgpOiBBZGRlZC4KKyAgICAgICAgKFdlYkNvcmU6OnNldExvZ2ljYWxXaWR0aEZvclRl eHRSdW4pOiBVc2UgbWVhc3VyZUh5cGhlbldpZHRoKCkuCisgICAgICAgIChXZWJDb3JlOjp0cnlI eXBoZW5hdGluZyk6IERpdHRvLgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyQmxvY2s6OkxpbmVC cmVha2VyOjpuZXh0TGluZUJyZWFrKTogRGl0dG8uCisKIDIwMTEtMDktMDYgIE9saXZlciBIdW50 ICA8b2xpdmVyQGFwcGxlLmNvbT4KIAogICAgICAgICBSZW1vdmUgSlNPYmplY3RXaXRoR2xvYmFs T2JqZWN0CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyQmxvY2tMaW5lTGF5 b3V0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyQmxv Y2tMaW5lTGF5b3V0LmNwcAkocmV2aXNpb24gOTQ1NzMpCisrKyBTb3VyY2UvV2ViQ29yZS9yZW5k ZXJpbmcvUmVuZGVyQmxvY2tMaW5lTGF5b3V0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDc4LDYg KzQ3OCwxMiBAQCB2b2lkIFJlbmRlckJsb2NrOjpzZXRNYXJnaW5zRm9yUnVieVJ1bihCCiAgICAg c2V0TWFyZ2luRW5kRm9yQ2hpbGQocmVuZGVyZXIsIC1lbmRPdmVyaGFuZyk7CiB9CiAKK3N0YXRp YyBpbmxpbmUgZmxvYXQgbWVhc3VyZUh5cGhlbldpZHRoKFJlbmRlclRleHQqIHJlbmRlcmVyLCBj b25zdCBGb250JiBmb250KQoreworICAgIFJlbmRlclN0eWxlKiBzdHlsZSA9IHJlbmRlcmVyLT5z dHlsZSgpOworICAgIHJldHVybiBmb250LndpZHRoKFJlbmRlckJsb2NrOjpjb25zdHJ1Y3RUZXh0 UnVuKHJlbmRlcmVyLCBmb250LCBzdHlsZS0+aHlwaGVuU3RyaW5nKCkuc3RyaW5nKCksIHN0eWxl KSk7Cit9CisKIHN0YXRpYyBpbmxpbmUgdm9pZCBzZXRMb2dpY2FsV2lkdGhGb3JUZXh0UnVuKFJv b3RJbmxpbmVCb3gqIGxpbmVCb3gsIEJpZGlSdW4qIHJ1biwgUmVuZGVyVGV4dCogcmVuZGVyZXIs IGZsb2F0IHhQb3MsIGNvbnN0IExpbmVJbmZvJiBsaW5lSW5mbywKICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgR2x5cGhPdmVyZmxvd0FuZEZhbGxiYWNrRm9udHNNYXAmIHRleHRC b3hEYXRhTWFwLCBWZXJ0aWNhbFBvc2l0aW9uQ2FjaGUmIHZlcnRpY2FsUG9zaXRpb25DYWNoZSkK IHsKQEAgLTUwMCw5ICs1MDYsOCBAQCBzdGF0aWMgaW5saW5lIHZvaWQgc2V0TG9naWNhbFdpZHRo Rm9yVGV4CiAgICAgCiAgICAgaW50IGh5cGhlbldpZHRoID0gMDsKICAgICBpZiAodG9JbmxpbmVU ZXh0Qm94KHJ1bi0+bV9ib3gpLT5oYXNIeXBoZW4oKSkgewotICAgICAgICBjb25zdCBBdG9taWNT dHJpbmcmIGh5cGhlblN0cmluZyA9IHJlbmRlcmVyLT5zdHlsZSgpLT5oeXBoZW5TdHJpbmcoKTsK ICAgICAgICAgY29uc3QgRm9udCYgZm9udCA9IHJlbmRlcmVyLT5zdHlsZShsaW5lSW5mby5pc0Zp cnN0TGluZSgpKS0+Zm9udCgpOwotICAgICAgICBoeXBoZW5XaWR0aCA9IGZvbnQud2lkdGgoUmVu ZGVyQmxvY2s6OmNvbnN0cnVjdFRleHRSdW4ocmVuZGVyZXIsIGZvbnQsIGh5cGhlblN0cmluZy5z dHJpbmcoKSwgcmVuZGVyZXItPnN0eWxlKCkpKTsKKyAgICAgICAgaHlwaGVuV2lkdGggPSBtZWFz dXJlSHlwaGVuV2lkdGgocmVuZGVyZXIsIGZvbnQpOwogICAgIH0KICAgICBydW4tPm1fYm94LT5z ZXRMb2dpY2FsV2lkdGgocmVuZGVyZXItPndpZHRoKHJ1bi0+bV9zdGFydCwgcnVuLT5tX3N0b3Ag LSBydW4tPm1fc3RhcnQsIHhQb3MsIGxpbmVJbmZvLmlzRmlyc3RMaW5lKCksICZmYWxsYmFja0Zv bnRzLCAmZ2x5cGhPdmVyZmxvdykgKyBoeXBoZW5XaWR0aCk7CiAgICAgaWYgKCFmYWxsYmFja0Zv bnRzLmlzRW1wdHkoKSkgewpAQCAtMTY5MCw4ICsxNjk1LDcgQEAgc3RhdGljIHZvaWQgdHJ5SHlw aGVuYXRpbmcoUmVuZGVyVGV4dCogdAogICAgIGlmIChwb3MgLSBsYXN0U3BhY2UgPD0gbWluaW11 bVN1ZmZpeExlbmd0aCkKICAgICAgICAgcmV0dXJuOwogCi0gICAgY29uc3QgQXRvbWljU3RyaW5n JiBoeXBoZW5TdHJpbmcgPSB0ZXh0LT5zdHlsZSgpLT5oeXBoZW5TdHJpbmcoKTsKLSAgICBpbnQg aHlwaGVuV2lkdGggPSBmb250LndpZHRoKFJlbmRlckJsb2NrOjpjb25zdHJ1Y3RUZXh0UnVuKHRl eHQsIGZvbnQsIGh5cGhlblN0cmluZy5zdHJpbmcoKSwgdGV4dC0+c3R5bGUoKSkpOworICAgIGlu dCBoeXBoZW5XaWR0aCA9IG1lYXN1cmVIeXBoZW5XaWR0aCh0ZXh0LCBmb250KTsKIAogICAgIGZs b2F0IG1heFByZWZpeFdpZHRoID0gYXZhaWxhYmxlV2lkdGggLSB4UG9zIC0gaHlwaGVuV2lkdGgg LSBsYXN0U3BhY2VXb3JkU3BhY2luZzsKICAgICAvLyBJZiB0aGUgbWF4aW11bSB3aWR0aCBhdmFp bGFibGUgZm9yIHRoZSBwcmVmaXggYmVmb3JlIHRoZSBoeXBoZW4gaXMgc21hbGwsIHRoZW4gaXQg aXMgdmVyeSB1bmxpa2VseQpAQCAtMjE5MSw4ICsyMTk1LDcgQEAgSW5saW5lSXRlcmF0b3IgUmVu ZGVyQmxvY2s6OkxpbmVCcmVha2VyOgogICAgICAgICAgICAgICAgICAgICBsaW5lSW5mby5zZXRF bXB0eShmYWxzZSk7CiAKICAgICAgICAgICAgICAgICBpZiAoYyA9PSBzb2Z0SHlwaGVuICYmIGF1 dG9XcmFwICYmICFoeXBoZW5XaWR0aCAmJiBzdHlsZS0+aHlwaGVucygpICE9IEh5cGhlbnNOb25l KSB7Ci0gICAgICAgICAgICAgICAgICAgIGNvbnN0IEF0b21pY1N0cmluZyYgaHlwaGVuU3RyaW5n ID0gc3R5bGUtPmh5cGhlblN0cmluZygpOwotICAgICAgICAgICAgICAgICAgICBoeXBoZW5XaWR0 aCA9IGYud2lkdGgoY29uc3RydWN0VGV4dFJ1bih0LCBmLCBoeXBoZW5TdHJpbmcuc3RyaW5nKCks IGN1cnJlbnQubV9vYmotPnN0eWxlKCkpKTsKKyAgICAgICAgICAgICAgICAgICAgaHlwaGVuV2lk dGggPSBtZWFzdXJlSHlwaGVuV2lkdGgodCwgZik7CiAgICAgICAgICAgICAgICAgICAgIHdpZHRo LmFkZFVuY29tbWl0dGVkV2lkdGgoaHlwaGVuV2lkdGgpOwogICAgICAgICAgICAgICAgIH0KIAo=