63519 2011-06-28 01:10:40 -0700 Refactor RenderTextControl::text(). 2011-06-28 16:47:40 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) All All RESOLVED FIXED P3 Minor --- 1 tkent tkent morrita webkit.review.bot oldest_to_newest 428704 0 tkent 2011-06-28 01:10:40 -0700 Refactor RenderTextControl::text(). 428706 1 98878 tkent 2011-06-28 01:15:03 -0700 Created attachment 98878 Patch 428715 2 98878 morrita 2011-06-28 01:25:44 -0700 Comment on attachment 98878 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98878&action=review > Source/WebCore/rendering/RenderTextControl.cpp:337 > +static String finishText(StringBuilder& result) Another possible idea would be introducing a small class which wraps or subclasses StringBuilder, and make it build these two types of text. This kind of out-parameter looks kind of horrible for me. What do you think? 428719 3 98878 tkent 2011-06-28 01:31:59 -0700 Comment on attachment 98878 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98878&action=review >> Source/WebCore/rendering/RenderTextControl.cpp:337 >> +static String finishText(StringBuilder& result) > > Another possible idea would be introducing a small class > which wraps or subclasses StringBuilder, and make it build these two types of text. > This kind of out-parameter looks kind of horrible for me. > What do you think? I feel it's overkill in this case. 428723 4 98878 morrita 2011-06-28 01:40:39 -0700 Comment on attachment 98878 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98878&action=review >>> Source/WebCore/rendering/RenderTextControl.cpp:337 >>> +static String finishText(StringBuilder& result) >> >> Another possible idea would be introducing a small class >> which wraps or subclasses StringBuilder, and make it build these two types of text. >> This kind of out-parameter looks kind of horrible for me. >> What do you think? > > I feel it's overkill in this case. Okay, let's just land this. 428757 5 98878 webkit.review.bot 2011-06-28 02:31:11 -0700 Comment on attachment 98878 Patch Clearing flags on attachment: 98878 Committed r89913: <http://trac.webkit.org/changeset/89913> 428758 6 webkit.review.bot 2011-06-28 02:31:16 -0700 All reviewed patches have been landed. Closing bug. 429219 7 ap 2011-06-28 16:06:16 -0700 It would be really good to have a goal of refactoring mentioned in ChangeLog. What were you trying to achieve? 429266 8 tkent 2011-06-28 16:47:40 -0700 (In reply to comment #7) > It would be really good to have a goal of refactoring mentioned in ChangeLog. What were you trying to achieve? Thank you for pointing it out. The trigger of the change was I found text() was called twice in setInnerText(). It was obviously unnecessary. Changes for text() and finishText() are just code cleanup. I should have written such information to ChangeLog. 98878 2011-06-28 01:15:03 -0700 2011-06-28 02:31:11 -0700 Patch bug-63519-20110628171502.patch text/plain 6631 tkent U3VidmVyc2lvbiBSZXZpc2lvbjogODk4OTEKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBkOTJiZjc4YmJlMTkyMjlh NDJkMzg1ZDU0OWYyZGY4NDI1ZGE4ZWVkLi5kZWUxYTI3MzFmMTM2YzkyZjllZTk2MGI0MWYyOTE2 MjgzNjNiMzlkIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjQgQEAKKzIwMTEtMDYtMjggIEtlbnQg VGFtdXJhICA8dGtlbnRAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFJlZmFjdG9yIFJlbmRlclRleHRDb250cm9sOjp0ZXh0KCku CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MzUxOQor CisgICAgICAgIE5vIG5ldyB0ZXN0cyBiZWNhdXNlIHRoaXMgaXMganVzdCBhIHJlZmFjdG9yaW5n LgorCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclRleHRDb250cm9sLmNwcDoKKyAgICAgICAg KFdlYkNvcmU6OlJlbmRlclRleHRDb250cm9sOjpzZXRJbm5lclRleHRWYWx1ZSk6CisgICAgICAg ICBSZXN1Y2UgdGhlIG51bWJlciBvZiB0ZXh0KCkgY2FsbHMuCisgICAgICAgIChXZWJDb3JlOjpm aW5pc2hUZXh0KToKKyAgICAgICAgICAtIFVzZSBTdHJpbmdCdWlsZGVyLgorICAgICAgICAgIC0g TWFrZSB0aGlzIGEgc3RhdGljIGZ1bmN0aW9uLgorICAgICAgICAgICAgVGhpcyBmdW5jdGlvbiBk b2Vzbid0IGFjY2VzcyBhbnl0aGluZyBvZiBSZW5kZXJUZXh0Q29udHJvbC4KKyAgICAgICAgKFdl YkNvcmU6OlJlbmRlclRleHRDb250cm9sOjp0ZXh0KToKKyAgICAgICAgIFVzZSBlbXB0eVN0cmlu ZygpIGFuZCBTdHJpbmdCdWlsZGVyLgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyVGV4dENvbnRy b2w6OnRleHRXaXRoSGFyZExpbmVCcmVha3MpOiBkaXR0by4KKyAgICAgICAgKiByZW5kZXJpbmcv UmVuZGVyVGV4dENvbnRyb2wuaDogUmVtb3ZlIGZpbmlzaFRleHQoKS4KKwogMjAxMS0wNi0yNyAg R3l1eW91bmcgS2ltICA8Z3l1eW91bmcua2ltQHNhbXN1bmcuY29tPgogCiAgICAgICAgIFVucmV2 aWV3ZWQuIEZpeCBzdHlsZSBlcnJvciByZWxhdGVkIHRvIHdyb25nIGluY2x1ZGUgb3JkZXIuCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGV4dENvbnRyb2wuY3Bw IGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRleHRDb250cm9sLmNwcAppbmRleCA1 ZDI5MjAwOGM3ODhhNmE5MjUzZTNkZmEyYzdmNDEyYzM3ZmMxYmNjLi5lZmQ3M2QzZmRmMzBmMGNm OTc5MjU5YmMxYTk1ZWQzMTRhNGY4ZWIyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5k ZXJpbmcvUmVuZGVyVGV4dENvbnRyb2wuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmlu Zy9SZW5kZXJUZXh0Q29udHJvbC5jcHAKQEAgLTQwLDYgKzQwLDcgQEAKICNpbmNsdWRlICJUZXh0 LmgiCiAjaW5jbHVkZSAiVGV4dENvbnRyb2xJbm5lckVsZW1lbnRzLmgiCiAjaW5jbHVkZSAiVGV4 dEl0ZXJhdG9yLmgiCisjaW5jbHVkZSA8d3RmL3RleHQvU3RyaW5nQnVpbGRlci5oPgogI2luY2x1 ZGUgPHd0Zi91bmljb2RlL0NoYXJhY3Rlck5hbWVzLmg+CiAKIHVzaW5nIG5hbWVzcGFjZSBzdGQ7 CkBAIC0xNDMsMTEgKzE0NCwxMSBAQCB2b2lkIFJlbmRlclRleHRDb250cm9sOjp1cGRhdGVGcm9t RWxlbWVudCgpCiAgICAgICAgIHVwZGF0ZVVzZXJNb2RpZnlQcm9wZXJ0eShub2RlKCksIGlubmVy VGV4dC0+cmVuZGVyZXIoKS0+c3R5bGUoKSk7CiB9CiAKLXZvaWQgUmVuZGVyVGV4dENvbnRyb2w6 OnNldElubmVyVGV4dFZhbHVlKGNvbnN0IFN0cmluZyYgaW5uZXJUZXh0VmFsdWUpCit2b2lkIFJl bmRlclRleHRDb250cm9sOjpzZXRJbm5lclRleHRWYWx1ZShjb25zdCBTdHJpbmcmIHZhbHVlKQog ewotICAgIFN0cmluZyB2YWx1ZSA9IGlubmVyVGV4dFZhbHVlOwotICAgIGlmICh2YWx1ZSAhPSB0 ZXh0KCkgfHwgIWlubmVyVGV4dEVsZW1lbnQoKS0+aGFzQ2hpbGROb2RlcygpKSB7Ci0gICAgICAg IGlmICh2YWx1ZSAhPSB0ZXh0KCkgJiYgZG9jdW1lbnQoKSAmJiBBWE9iamVjdENhY2hlOjphY2Nl c3NpYmlsaXR5RW5hYmxlZCgpKQorICAgIGJvb2wgdGV4dElzQ2hhbmdlZCA9IHZhbHVlICE9IHRl eHQoKTsKKyAgICBpZiAodGV4dElzQ2hhbmdlZCB8fCAhaW5uZXJUZXh0RWxlbWVudCgpLT5oYXND aGlsZE5vZGVzKCkpIHsKKyAgICAgICAgaWYgKHRleHRJc0NoYW5nZWQgJiYgZG9jdW1lbnQoKSAm JiBBWE9iamVjdENhY2hlOjphY2Nlc3NpYmlsaXR5RW5hYmxlZCgpKQogICAgICAgICAgICAgZG9j dW1lbnQoKS0+YXhPYmplY3RDYWNoZSgpLT5wb3N0Tm90aWZpY2F0aW9uKHRoaXMsIEFYT2JqZWN0 Q2FjaGU6OkFYVmFsdWVDaGFuZ2VkLCBmYWxzZSk7CiAKICAgICAgICAgRXhjZXB0aW9uQ29kZSBl YyA9IDA7CkBAIC0zMzMsMzMgKzMzNCwyOCBAQCB2b2lkIFJlbmRlclRleHRDb250cm9sOjpzdWJ0 cmVlSGFzQ2hhbmdlZCgpCiAgICAgbV9sYXN0Q2hhbmdlV2FzVXNlckVkaXQgPSB0cnVlOwogfQog Ci1TdHJpbmcgUmVuZGVyVGV4dENvbnRyb2w6OmZpbmlzaFRleHQoVmVjdG9yPFVDaGFyPiYgcmVz dWx0KSBjb25zdAorc3RhdGljIFN0cmluZyBmaW5pc2hUZXh0KFN0cmluZ0J1aWxkZXImIHJlc3Vs dCkKIHsKICAgICAvLyBSZW1vdmUgb25lIHRyYWlsaW5nIG5ld2xpbmU7IHRoZXJlJ3MgYWx3YXlz IG9uZSB0aGF0J3MgY29sbGFwc2VkIG91dCBieSByZW5kZXJpbmcuCi0gICAgc2l6ZV90IHNpemUg PSByZXN1bHQuc2l6ZSgpOworICAgIHNpemVfdCBzaXplID0gcmVzdWx0Lmxlbmd0aCgpOwogICAg IGlmIChzaXplICYmIHJlc3VsdFtzaXplIC0gMV0gPT0gJ1xuJykKLSAgICAgICAgcmVzdWx0LnNo cmluaygtLXNpemUpOwotCi0gICAgcmV0dXJuIFN0cmluZzo6YWRvcHQocmVzdWx0KTsKKyAgICAg ICAgcmVzdWx0LnJlc2l6ZSgtLXNpemUpOworICAgIHJldHVybiByZXN1bHQudG9TdHJpbmcoKTsK IH0KIAogU3RyaW5nIFJlbmRlclRleHRDb250cm9sOjp0ZXh0KCkKIHsKICAgICBIVE1MRWxlbWVu dCogaW5uZXJUZXh0ID0gaW5uZXJUZXh0RWxlbWVudCgpOwogICAgIGlmICghaW5uZXJUZXh0KQot ICAgICAgICByZXR1cm4gIiI7CisgICAgICAgIHJldHVybiBlbXB0eVN0cmluZygpOwogIAotICAg IFZlY3RvcjxVQ2hhcj4gcmVzdWx0OwotCi0gICAgZm9yIChOb2RlKiBuID0gaW5uZXJUZXh0OyBu OyBuID0gbi0+dHJhdmVyc2VOZXh0Tm9kZShpbm5lclRleHQpKSB7Ci0gICAgICAgIGlmIChuLT5o YXNUYWdOYW1lKGJyVGFnKSkKLSAgICAgICAgICAgIHJlc3VsdC5hcHBlbmQoJm5ld2xpbmVDaGFy YWN0ZXIsIDEpOwotICAgICAgICBlbHNlIGlmIChuLT5pc1RleHROb2RlKCkpIHsKLSAgICAgICAg ICAgIFN0cmluZyBkYXRhID0gc3RhdGljX2Nhc3Q8VGV4dCo+KG4pLT5kYXRhKCk7Ci0gICAgICAg ICAgICByZXN1bHQuYXBwZW5kKGRhdGEuY2hhcmFjdGVycygpLCBkYXRhLmxlbmd0aCgpKTsKLSAg ICAgICAgfQorICAgIFN0cmluZ0J1aWxkZXIgcmVzdWx0OworICAgIGZvciAoTm9kZSogbm9kZSA9 IGlubmVyVGV4dDsgbm9kZTsgbm9kZSA9IG5vZGUtPnRyYXZlcnNlTmV4dE5vZGUoaW5uZXJUZXh0 KSkgeworICAgICAgICBpZiAobm9kZS0+aGFzVGFnTmFtZShiclRhZykpCisgICAgICAgICAgICBy ZXN1bHQuYXBwZW5kKG5ld2xpbmVDaGFyYWN0ZXIpOworICAgICAgICBlbHNlIGlmIChub2RlLT5p c1RleHROb2RlKCkpCisgICAgICAgICAgICByZXN1bHQuYXBwZW5kKHN0YXRpY19jYXN0PFRleHQq Pihub2RlKS0+ZGF0YSgpKTsKICAgICB9Ci0KICAgICByZXR1cm4gZmluaXNoVGV4dChyZXN1bHQp OwogfQogCkBAIC0zODQsNDQgKzM4MCw0MSBAQCBTdHJpbmcgUmVuZGVyVGV4dENvbnRyb2w6OnRl eHRXaXRoSGFyZExpbmVCcmVha3MoKQogewogICAgIEhUTUxFbGVtZW50KiBpbm5lclRleHQgPSBp bm5lclRleHRFbGVtZW50KCk7CiAgICAgaWYgKCFpbm5lclRleHQpCi0gICAgICAgIHJldHVybiAi IjsKKyAgICAgICAgcmV0dXJuIGVtcHR5U3RyaW5nKCk7CiAKICAgICBSZW5kZXJCbG9jayogcmVu ZGVyZXIgPSB0b1JlbmRlckJsb2NrKGlubmVyVGV4dC0+cmVuZGVyZXIoKSk7CiAgICAgaWYgKCFy ZW5kZXJlcikKLSAgICAgICAgcmV0dXJuICIiOworICAgICAgICByZXR1cm4gZW1wdHlTdHJpbmco KTsKIAogICAgIE5vZGUqIGJyZWFrTm9kZTsKICAgICB1bnNpZ25lZCBicmVha09mZnNldDsKICAg ICBSb290SW5saW5lQm94KiBsaW5lID0gcmVuZGVyZXItPmZpcnN0Um9vdEJveCgpOwogICAgIGlm ICghbGluZSkKLSAgICAgICAgcmV0dXJuICIiOworICAgICAgICByZXR1cm4gZW1wdHlTdHJpbmco KTsKIAogICAgIGdldE5leHRTb2Z0QnJlYWsobGluZSwgYnJlYWtOb2RlLCBicmVha09mZnNldCk7 CiAKLSAgICBWZWN0b3I8VUNoYXI+IHJlc3VsdDsKLQotICAgIGZvciAoTm9kZSogbiA9IGlubmVy VGV4dC0+Zmlyc3RDaGlsZCgpOyBuOyBuID0gbi0+dHJhdmVyc2VOZXh0Tm9kZShpbm5lclRleHQp KSB7Ci0gICAgICAgIGlmIChuLT5oYXNUYWdOYW1lKGJyVGFnKSkKLSAgICAgICAgICAgIHJlc3Vs dC5hcHBlbmQoJm5ld2xpbmVDaGFyYWN0ZXIsIDEpOwotICAgICAgICBlbHNlIGlmIChuLT5pc1Rl eHROb2RlKCkpIHsKLSAgICAgICAgICAgIFRleHQqIHRleHQgPSBzdGF0aWNfY2FzdDxUZXh0Kj4o bik7Ci0gICAgICAgICAgICBTdHJpbmcgZGF0YSA9IHRleHQtPmRhdGEoKTsKKyAgICBTdHJpbmdC dWlsZGVyIHJlc3VsdDsKKyAgICBmb3IgKE5vZGUqIG5vZGUgPSBpbm5lclRleHQtPmZpcnN0Q2hp bGQoKTsgbm9kZTsgbm9kZSA9IG5vZGUtPnRyYXZlcnNlTmV4dE5vZGUoaW5uZXJUZXh0KSkgewor ICAgICAgICBpZiAobm9kZS0+aGFzVGFnTmFtZShiclRhZykpCisgICAgICAgICAgICByZXN1bHQu YXBwZW5kKG5ld2xpbmVDaGFyYWN0ZXIpOworICAgICAgICBlbHNlIGlmIChub2RlLT5pc1RleHRO b2RlKCkpIHsKKyAgICAgICAgICAgIFN0cmluZyBkYXRhID0gc3RhdGljX2Nhc3Q8VGV4dCo+KG5v ZGUpLT5kYXRhKCk7CiAgICAgICAgICAgICB1bnNpZ25lZCBsZW5ndGggPSBkYXRhLmxlbmd0aCgp OwogICAgICAgICAgICAgdW5zaWduZWQgcG9zaXRpb24gPSAwOwotICAgICAgICAgICAgd2hpbGUg KGJyZWFrTm9kZSA9PSBuICYmIGJyZWFrT2Zmc2V0IDw9IGxlbmd0aCkgeworICAgICAgICAgICAg d2hpbGUgKGJyZWFrTm9kZSA9PSBub2RlICYmIGJyZWFrT2Zmc2V0IDw9IGxlbmd0aCkgewogICAg ICAgICAgICAgICAgIGlmIChicmVha09mZnNldCA+IHBvc2l0aW9uKSB7CiAgICAgICAgICAgICAg ICAgICAgIHJlc3VsdC5hcHBlbmQoZGF0YS5jaGFyYWN0ZXJzKCkgKyBwb3NpdGlvbiwgYnJlYWtP ZmZzZXQgLSBwb3NpdGlvbik7CiAgICAgICAgICAgICAgICAgICAgIHBvc2l0aW9uID0gYnJlYWtP ZmZzZXQ7Ci0gICAgICAgICAgICAgICAgICAgIHJlc3VsdC5hcHBlbmQoJm5ld2xpbmVDaGFyYWN0 ZXIsIDEpOworICAgICAgICAgICAgICAgICAgICByZXN1bHQuYXBwZW5kKG5ld2xpbmVDaGFyYWN0 ZXIpOwogICAgICAgICAgICAgICAgIH0KICAgICAgICAgICAgICAgICBnZXROZXh0U29mdEJyZWFr KGxpbmUsIGJyZWFrTm9kZSwgYnJlYWtPZmZzZXQpOwogICAgICAgICAgICAgfQogICAgICAgICAg ICAgcmVzdWx0LmFwcGVuZChkYXRhLmNoYXJhY3RlcnMoKSArIHBvc2l0aW9uLCBsZW5ndGggLSBw b3NpdGlvbik7CiAgICAgICAgIH0KLSAgICAgICAgd2hpbGUgKGJyZWFrTm9kZSA9PSBuKQorICAg ICAgICB3aGlsZSAoYnJlYWtOb2RlID09IG5vZGUpCiAgICAgICAgICAgICBnZXROZXh0U29mdEJy ZWFrKGxpbmUsIGJyZWFrTm9kZSwgYnJlYWtPZmZzZXQpOwogICAgIH0KLQogICAgIHJldHVybiBm aW5pc2hUZXh0KHJlc3VsdCk7CiB9CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRl cmluZy9SZW5kZXJUZXh0Q29udHJvbC5oIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRl clRleHRDb250cm9sLmgKaW5kZXggNzFkOGU5YTliMTI0ODJjYTk2NzgyYzEyODU1Yjg3NTk0ZjM0 NzRkNi4uMDQ0NTdhN2EzMjBjNzc2NWNmYzZhZTg4MzBhZWRmNWY4NTBlYmExOSAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRleHRDb250cm9sLmgKKysrIGIvU291 cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRleHRDb250cm9sLmgKQEAgLTk4LDggKzk4LDYg QEAgcHJpdmF0ZToKIAogICAgIHZpcnR1YWwgYm9vbCByZXF1aXJlc0ZvcmNlZFN0eWxlUmVjYWxj UHJvcGFnYXRpb24oKSBjb25zdCB7IHJldHVybiB0cnVlOyB9CiAKLSAgICBTdHJpbmcgZmluaXNo VGV4dChWZWN0b3I8VUNoYXI+JikgY29uc3Q7Ci0KICAgICBib29sIGhhc1Zpc2libGVUZXh0QXJl YSgpIGNvbnN0OwogICAgIGZyaWVuZCB2b2lkIHNldFNlbGVjdGlvblJhbmdlKE5vZGUqLCBpbnQg c3RhcnQsIGludCBlbmQpOwogICAgIHN0YXRpYyBib29sIGlzU2VsZWN0YWJsZUVsZW1lbnQoSFRN TEVsZW1lbnQqLCBOb2RlKik7Cg==