58557 2011-04-14 11:19:20 -0700 Extract layoutRepacedElements from layoutInlineChildren 2011-05-04 20:16:39 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) All All NEW P2 Enhancement --- 57779 1 rniwa webkit-unassigned cmarcelo dglazkov eric hyatt oldest_to_newest 385898 0 rniwa 2011-04-14 11:19:20 -0700 This is a cleanup. 385903 1 89612 rniwa 2011-04-14 11:21:01 -0700 Created attachment 89612 cleanup 385913 2 89612 eric 2011-04-14 11:42:54 -0700 Comment on attachment 89612 cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=89612&action=review > Source/WebCore/rendering/RenderBlockLineLayout.cpp:716 > +inline void RenderBlock::layoutRepacedElements(bool relayoutChildren, bool fullLayout, Vector<RenderBlock::FloatWithRect>& floats, bool& hasInlineChild) I thought about doing just this. HOweer, this does more than just layout replaced elments. It's sorta a first-pass layout. I wasn't really sure what to call it when I wrote a similar patch. 385922 3 89612 dbates 2011-04-14 11:48:21 -0700 Comment on attachment 89612 cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=89612&action=review > Source/WebCore/ChangeLog:8 > + Extracted a block of code in layoutInlineChildren as layoutRepacedElements. layoutRepacedElements => layoutReplacedElements (you used layoutRepacedElements throughout this patch) And, as Eric Seidel remarked, this function seems to more than just handle replace elements. > Source/WebCore/rendering/RenderBlockLineLayout.cpp:740 > + } else if (object->isText() || (object->isRenderInline() && !endOfInline)) { Nit: The parentheses around the second disjunct are unnecessary since && (logical and) has higher precedence than || (logical or). 385940 4 89612 rniwa 2011-04-14 12:03:58 -0700 Comment on attachment 89612 cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=89612&action=review >> Source/WebCore/ChangeLog:8 >> + Extracted a block of code in layoutInlineChildren as layoutRepacedElements. > > layoutRepacedElements => layoutReplacedElements > > (you used layoutRepacedElements throughout this patch) > > And, as Eric Seidel remarked, this function seems to more than just handle replace elements. Oops. I'll rename the function. Any suggestion? >> Source/WebCore/rendering/RenderBlockLineLayout.cpp:740 >> + } else if (object->isText() || (object->isRenderInline() && !endOfInline)) { > > Nit: The parentheses around the second disjunct are unnecessary since && (logical and) has higher precedence than || (logical or). There's at least one port where not having this parenthesis causes a build failure because new versions of gcc gives you a warning on this. 392971 5 89612 eric 2011-04-26 16:18:36 -0700 Comment on attachment 89612 cleanup We need a better name. We'll work on this mor in our hackathon on thurs. 89612 2011-04-14 11:21:01 -0700 2011-04-26 16:18:36 -0700 cleanup bug-58557-20110414112100.patch text/plain 6858 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDgzODc0KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDQtMTQgIFJ5b3N1a2Ug Tml3YSAgPHJuaXdhQHdlYmtpdC5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgRXh0cmFjdCBsYXlvdXRSZXBhY2VkRWxlbWVudHMgZnJvbSBsYXlv dXRJbmxpbmVDaGlsZHJlbgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9NTg1NTcKKworICAgICAgICBFeHRyYWN0ZWQgYSBibG9jayBvZiBjb2RlIGluIGxh eW91dElubGluZUNoaWxkcmVuIGFzIGxheW91dFJlcGFjZWRFbGVtZW50cy4KKworICAgICAgICAq IHJlbmRlcmluZy9SZW5kZXJCbG9jay5oOgorICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJCbG9j a0xpbmVMYXlvdXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyQmxvY2s6OmxheW91dFJl cGFjZWRFbGVtZW50cyk6IEV4dHJhY3RlZC4KKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlckJsb2Nr OjpsYXlvdXRJbmxpbmVDaGlsZHJlbik6IENhbGxzIGxheW91dFJlcGFjZWRFbGVtZW50cy4KKwog MjAxMS0wNC0xNCAgWXVyeSBTZW1pa2hhdHNreSAgPHl1cnlzQGNocm9taXVtLm9yZz4KIAogICAg ICAgICBSZXZpZXdlZCBieSBQYXZlbCBGZWxkbWFuLgpJbmRleDogU291cmNlL1dlYkNvcmUvcmVu ZGVyaW5nL1JlbmRlckJsb2NrLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcmVuZGVy aW5nL1JlbmRlckJsb2NrLmgJKHJldmlzaW9uIDgzODA3KQorKysgU291cmNlL1dlYkNvcmUvcmVu ZGVyaW5nL1JlbmRlckJsb2NrLmgJKHdvcmtpbmcgY29weSkKQEAgLTM1MSw4ICszNTEsNiBAQCBw cml2YXRlOgogICAgIHZpcnR1YWwgdm9pZCByZXBhaW50T3ZlcmhhbmdpbmdGbG9hdHMoYm9vbCBw YWludEFsbERlc2NlbmRhbnRzKTsKIAogICAgIHZvaWQgbGF5b3V0QmxvY2tDaGlsZHJlbihib29s IHJlbGF5b3V0Q2hpbGRyZW4sIGludCYgbWF4RmxvYXRMb2dpY2FsQm90dG9tKTsKLSAgICB2b2lk IGxheW91dElubGluZUNoaWxkcmVuKGJvb2wgcmVsYXlvdXRDaGlsZHJlbiwgaW50JiByZXBhaW50 TG9naWNhbFRvcCwgaW50JiByZXBhaW50TG9naWNhbEJvdHRvbSk7Ci0gICAgQmlkaVJ1biogaGFu ZGxlVHJhaWxpbmdTcGFjZXMoQmlkaVJ1bkxpc3Q8QmlkaVJ1bj4mLCBCaWRpQ29udGV4dCopOwog CiAgICAgdmlydHVhbCB2b2lkIGJvcmRlckZpdEFkanVzdChpbnQmIHgsIGludCYgdykgY29uc3Q7 IC8vIFNocmluayB0aGUgYm94IGluIHdoaWNoIHRoZSBib3JkZXIgcGFpbnRzIGlmIGJvcmRlci1m aXQgaXMgc2V0LgogCkBAIC01MDUsNiArNTAzLDkgQEAgcHJpdmF0ZToKIAogICAgIHZvaWQgY29t cHV0ZUlubGluZURpcmVjdGlvblBvc2l0aW9uc0ZvckxpbmUoUm9vdElubGluZUJveCosIGJvb2wg Zmlyc3RMaW5lLCBCaWRpUnVuKiBmaXJzdFJ1biwgQmlkaVJ1biogdHJhaWxpbmdTcGFjZVJ1biwg Ym9vbCByZWFjaGVkRW5kLCBHbHlwaE92ZXJmbG93QW5kRmFsbGJhY2tGb250c01hcCYsIFZlcnRp Y2FsUG9zaXRpb25DYWNoZSYpOwogICAgIHZvaWQgY29tcHV0ZUJsb2NrRGlyZWN0aW9uUG9zaXRp b25zRm9yTGluZShSb290SW5saW5lQm94KiwgQmlkaVJ1biosIEdseXBoT3ZlcmZsb3dBbmRGYWxs YmFja0ZvbnRzTWFwJiwgVmVydGljYWxQb3NpdGlvbkNhY2hlJik7CisgICAgQmlkaVJ1biogaGFu ZGxlVHJhaWxpbmdTcGFjZXMoQmlkaVJ1bkxpc3Q8QmlkaVJ1bj4mLCBCaWRpQ29udGV4dCopOwor ICAgIHZvaWQgbGF5b3V0UmVwYWNlZEVsZW1lbnRzKGJvb2wgcmVsYXlvdXRDaGlsZHJlbiwgYm9v bCBmdWxsTGF5b3V0LCBWZWN0b3I8UmVuZGVyQmxvY2s6OkZsb2F0V2l0aFJlY3Q+JiwgYm9vbCYg aGFzSW5saW5lQ2hpbGQpOworICAgIHZvaWQgbGF5b3V0SW5saW5lQ2hpbGRyZW4oYm9vbCByZWxh eW91dENoaWxkcmVuLCBpbnQmIHJlcGFpbnRMb2dpY2FsVG9wLCBpbnQmIHJlcGFpbnRMb2dpY2Fs Qm90dG9tKTsKICAgICB2b2lkIGRlbGV0ZUVsbGlwc2lzTGluZUJveGVzKCk7CiAgICAgdm9pZCBj aGVja0xpbmVzRm9yVGV4dE92ZXJmbG93KCk7CiAKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3JlbmRl cmluZy9SZW5kZXJCbG9ja0xpbmVMYXlvdXQuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJD b3JlL3JlbmRlcmluZy9SZW5kZXJCbG9ja0xpbmVMYXlvdXQuY3BwCShyZXZpc2lvbiA4MzgwNykK KysrIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJCbG9ja0xpbmVMYXlvdXQuY3BwCSh3 b3JraW5nIGNvcHkpCkBAIC03MTMsNiArNzEzLDQwIEBAIGlubGluZSBCaWRpUnVuKiBSZW5kZXJC bG9jazo6aGFuZGxlVHJhaWwKICAgICByZXR1cm4gdHJhaWxpbmdTcGFjZVJ1bjsKIH0KIAoraW5s aW5lIHZvaWQgUmVuZGVyQmxvY2s6OmxheW91dFJlcGFjZWRFbGVtZW50cyhib29sIHJlbGF5b3V0 Q2hpbGRyZW4sIGJvb2wgZnVsbExheW91dCwgVmVjdG9yPFJlbmRlckJsb2NrOjpGbG9hdFdpdGhS ZWN0PiYgZmxvYXRzLCBib29sJiBoYXNJbmxpbmVDaGlsZCkKK3sKKyAgICBib29sIGVuZE9mSW5s aW5lID0gZmFsc2U7CisgICAgZm9yIChSZW5kZXJPYmplY3QqIG9iamVjdCA9IGJpZGlGaXJzdCh0 aGlzLCAwLCBmYWxzZSk7IG9iamVjdDsgb2JqZWN0ID0gYmlkaU5leHQodGhpcywgb2JqZWN0LCAw LCBmYWxzZSwgJmVuZE9mSW5saW5lKSkgeworICAgICAgICBpZiAoIWhhc0lubGluZUNoaWxkICYm IG9iamVjdC0+aXNJbmxpbmUoKSkKKyAgICAgICAgICAgIGhhc0lubGluZUNoaWxkID0gdHJ1ZTsK KworICAgICAgICBpZiAob2JqZWN0LT5pc1JlcGxhY2VkKCkgfHwgb2JqZWN0LT5pc0Zsb2F0aW5n KCkgfHwgb2JqZWN0LT5pc1Bvc2l0aW9uZWQoKSkgeworICAgICAgICAgICAgaWYgKHJlbGF5b3V0 Q2hpbGRyZW4gfHwgb2JqZWN0LT5zdHlsZSgpLT53aWR0aCgpLmlzUGVyY2VudCgpIHx8IG9iamVj dC0+c3R5bGUoKS0+aGVpZ2h0KCkuaXNQZXJjZW50KCkpCisgICAgICAgICAgICAgICAgb2JqZWN0 LT5zZXRDaGlsZE5lZWRzTGF5b3V0KHRydWUsIGZhbHNlKTsKKworICAgICAgICAgICAgLy8gSWYg cmVsYXlvdXRDaGlsZHJlbiBpcyBzZXQgYW5kIHdlIGhhdmUgcGVyY2VudGFnZSBwYWRkaW5nLCB3 ZSBhbHNvIG5lZWQgdG8gaW52YWxpZGF0ZSB0aGUgY2hpbGQncyBwcmVmIHdpZHRocy4KKyAgICAg ICAgICAgIGlmIChyZWxheW91dENoaWxkcmVuICYmIChvYmplY3QtPnN0eWxlKCktPnBhZGRpbmdT dGFydCgpLmlzUGVyY2VudCgpIHx8IG9iamVjdC0+c3R5bGUoKS0+cGFkZGluZ0VuZCgpLmlzUGVy Y2VudCgpKSkKKyAgICAgICAgICAgICAgICBvYmplY3QtPnNldFByZWZlcnJlZExvZ2ljYWxXaWR0 aHNEaXJ0eSh0cnVlLCBmYWxzZSk7CisKKyAgICAgICAgICAgIGlmIChvYmplY3QtPmlzUG9zaXRp b25lZCgpKQorICAgICAgICAgICAgICAgIG9iamVjdC0+Y29udGFpbmluZ0Jsb2NrKCktPmluc2Vy dFBvc2l0aW9uZWRPYmplY3QodG9SZW5kZXJCb3gob2JqZWN0KSk7CisgICAgICAgICAgICBlbHNl IGlmIChvYmplY3QtPmlzRmxvYXRpbmcoKSkKKyAgICAgICAgICAgICAgICBmbG9hdHMuYXBwZW5k KFJlbmRlckJsb2NrOjpGbG9hdFdpdGhSZWN0KHRvUmVuZGVyQm94KG9iamVjdCkpKTsKKyAgICAg ICAgICAgIGVsc2UgaWYgKGZ1bGxMYXlvdXQgfHwgb2JqZWN0LT5uZWVkc0xheW91dCgpKSB7Cisg ICAgICAgICAgICAgICAgLy8gUmVwbGFjZWQgZWxlbWVudHMKKyAgICAgICAgICAgICAgICB0b1Jl bmRlckJveChvYmplY3QpLT5kaXJ0eUxpbmVCb3hlcyhmdWxsTGF5b3V0KTsKKyAgICAgICAgICAg ICAgICBvYmplY3QtPmxheW91dElmTmVlZGVkKCk7CisgICAgICAgICAgICB9CisgICAgICAgIH0g ZWxzZSBpZiAob2JqZWN0LT5pc1RleHQoKSB8fCAob2JqZWN0LT5pc1JlbmRlcklubGluZSgpICYm ICFlbmRPZklubGluZSkpIHsKKyAgICAgICAgICAgIGlmICghb2JqZWN0LT5pc1RleHQoKSkKKyAg ICAgICAgICAgICAgICB0b1JlbmRlcklubGluZShvYmplY3QpLT51cGRhdGVBbHdheXNDcmVhdGVM aW5lQm94ZXMoKTsKKyAgICAgICAgICAgIGlmIChmdWxsTGF5b3V0IHx8IG9iamVjdC0+c2VsZk5l ZWRzTGF5b3V0KCkpCisgICAgICAgICAgICAgICAgZGlydHlMaW5lQm94ZXNGb3JSZW5kZXJlcihv YmplY3QsIGZ1bGxMYXlvdXQpOworICAgICAgICAgICAgb2JqZWN0LT5zZXROZWVkc0xheW91dChm YWxzZSk7CisgICAgICAgIH0KKyAgICB9Cit9CisKIHZvaWQgUmVuZGVyQmxvY2s6OmxheW91dElu bGluZUNoaWxkcmVuKGJvb2wgcmVsYXlvdXRDaGlsZHJlbiwgaW50JiByZXBhaW50TG9naWNhbFRv cCwgaW50JiByZXBhaW50TG9naWNhbEJvdHRvbSkKIHsKICAgICBib29sIHVzZVJlcGFpbnRCb3Vu ZHMgPSBmYWxzZTsKQEAgLTczOSw0MyArNzczLDkgQEAgdm9pZCBSZW5kZXJCbG9jazo6bGF5b3V0 SW5saW5lQ2hpbGRyZW4oYgogICAgICAgICAgZGVsZXRlRWxsaXBzaXNMaW5lQm94ZXMoKTsKIAog ICAgIGlmIChmaXJzdENoaWxkKCkpIHsKLSAgICAgICAgLy8gbGF5b3V0IHJlcGxhY2VkIGVsZW1l bnRzCi0gICAgICAgIGJvb2wgZW5kT2ZJbmxpbmUgPSBmYWxzZTsKLSAgICAgICAgUmVuZGVyT2Jq ZWN0KiBvID0gYmlkaUZpcnN0KHRoaXMsIDAsIGZhbHNlKTsKICAgICAgICAgVmVjdG9yPEZsb2F0 V2l0aFJlY3Q+IGZsb2F0czsKICAgICAgICAgYm9vbCBoYXNJbmxpbmVDaGlsZCA9IGZhbHNlOwot ICAgICAgICB3aGlsZSAobykgewotICAgICAgICAgICAgaWYgKCFoYXNJbmxpbmVDaGlsZCAmJiBv LT5pc0lubGluZSgpKQotICAgICAgICAgICAgICAgIGhhc0lubGluZUNoaWxkID0gdHJ1ZTsKLQot ICAgICAgICAgICAgaWYgKG8tPmlzUmVwbGFjZWQoKSB8fCBvLT5pc0Zsb2F0aW5nKCkgfHwgby0+ aXNQb3NpdGlvbmVkKCkpIHsKLSAgICAgICAgICAgICAgICBSZW5kZXJCb3gqIGJveCA9IHRvUmVu ZGVyQm94KG8pOwotICAgICAgICAgICAgICAgIAotICAgICAgICAgICAgICAgIGlmIChyZWxheW91 dENoaWxkcmVuIHx8IG8tPnN0eWxlKCktPndpZHRoKCkuaXNQZXJjZW50KCkgfHwgby0+c3R5bGUo KS0+aGVpZ2h0KCkuaXNQZXJjZW50KCkpCi0gICAgICAgICAgICAgICAgICAgIG8tPnNldENoaWxk TmVlZHNMYXlvdXQodHJ1ZSwgZmFsc2UpOwotICAgICAgICAgICAgICAgICAgICAKLSAgICAgICAg ICAgICAgICAvLyBJZiByZWxheW91dENoaWxkcmVuIGlzIHNldCBhbmQgd2UgaGF2ZSBwZXJjZW50 YWdlIHBhZGRpbmcsIHdlIGFsc28gbmVlZCB0byBpbnZhbGlkYXRlIHRoZSBjaGlsZCdzIHByZWYg d2lkdGhzLgotICAgICAgICAgICAgICAgIGlmIChyZWxheW91dENoaWxkcmVuICYmIChvLT5zdHls ZSgpLT5wYWRkaW5nU3RhcnQoKS5pc1BlcmNlbnQoKSB8fCBvLT5zdHlsZSgpLT5wYWRkaW5nRW5k KCkuaXNQZXJjZW50KCkpKQotICAgICAgICAgICAgICAgICAgICBvLT5zZXRQcmVmZXJyZWRMb2dp Y2FsV2lkdGhzRGlydHkodHJ1ZSwgZmFsc2UpOwotICAgICAgICAgICAgCi0gICAgICAgICAgICAg ICAgaWYgKG8tPmlzUG9zaXRpb25lZCgpKQotICAgICAgICAgICAgICAgICAgICBvLT5jb250YWlu aW5nQmxvY2soKS0+aW5zZXJ0UG9zaXRpb25lZE9iamVjdChib3gpOwotICAgICAgICAgICAgICAg IGVsc2UgaWYgKG8tPmlzRmxvYXRpbmcoKSkKLSAgICAgICAgICAgICAgICAgICAgZmxvYXRzLmFw cGVuZChGbG9hdFdpdGhSZWN0KGJveCkpOwotICAgICAgICAgICAgICAgIGVsc2UgaWYgKGZ1bGxM YXlvdXQgfHwgby0+bmVlZHNMYXlvdXQoKSkgewotICAgICAgICAgICAgICAgICAgICAvLyBSZXBs YWNlZCBlbGVtZW50cwotICAgICAgICAgICAgICAgICAgICB0b1JlbmRlckJveChvKS0+ZGlydHlM aW5lQm94ZXMoZnVsbExheW91dCk7Ci0gICAgICAgICAgICAgICAgICAgIG8tPmxheW91dElmTmVl ZGVkKCk7Ci0gICAgICAgICAgICAgICAgfQotICAgICAgICAgICAgfSBlbHNlIGlmIChvLT5pc1Rl eHQoKSB8fCAoby0+aXNSZW5kZXJJbmxpbmUoKSAmJiAhZW5kT2ZJbmxpbmUpKSB7Ci0gICAgICAg ICAgICAgICAgaWYgKCFvLT5pc1RleHQoKSkKLSAgICAgICAgICAgICAgICAgICAgdG9SZW5kZXJJ bmxpbmUobyktPnVwZGF0ZUFsd2F5c0NyZWF0ZUxpbmVCb3hlcygpOwotICAgICAgICAgICAgICAg IGlmIChmdWxsTGF5b3V0IHx8IG8tPnNlbGZOZWVkc0xheW91dCgpKQotICAgICAgICAgICAgICAg ICAgICBkaXJ0eUxpbmVCb3hlc0ZvclJlbmRlcmVyKG8sIGZ1bGxMYXlvdXQpOwotICAgICAgICAg ICAgICAgIG8tPnNldE5lZWRzTGF5b3V0KGZhbHNlKTsKLSAgICAgICAgICAgIH0KLSAgICAgICAg ICAgIG8gPSBiaWRpTmV4dCh0aGlzLCBvLCAwLCBmYWxzZSwgJmVuZE9mSW5saW5lKTsKLSAgICAg ICAgfQorICAgICAgICBsYXlvdXRSZXBhY2VkRWxlbWVudHMocmVsYXlvdXRDaGlsZHJlbiwgZnVs bExheW91dCwgZmxvYXRzLCBoYXNJbmxpbmVDaGlsZCk7CiAKICAgICAgICAgLy8gV2Ugd2FudCB0 byBza2lwIGFoZWFkIHRvIHRoZSBmaXJzdCBkaXJ0eSBsaW5lCiAgICAgICAgIElubGluZUJpZGlS ZXNvbHZlciByZXNvbHZlcjsK