85325 2012-05-01 16:47:29 -0700 Remove uneeded min/max pref width assignment from RenderView 2012-05-02 17:48:33 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 45950 1 eric eric hyatt jamesr jchaffraix ojan webkit.review.bot oldest_to_newest 613790 0 eric 2012-05-01 16:47:29 -0700 Remove uneeded min/max pref width assignment from RenderView 613793 1 139712 eric 2012-05-01 16:51:34 -0700 Created attachment 139712 Patch 613808 2 eric 2012-05-01 17:20:17 -0700 With cr-linux (thus the layout tests) passing, this should just be a rubber-stamp for anyone willing. 613812 3 139712 eae 2012-05-01 17:23:10 -0700 Comment on attachment 139712 Patch Seems reasonable, would be good to eventually get rid of it though! 613813 4 139712 leviw 2012-05-01 17:23:33 -0700 Comment on attachment 139712 Patch Great description and I agree with your conclusion. LGTM. 613816 5 eric 2012-05-01 17:27:20 -0700 Of course we need an actual reviewer to pull the trigger... 613819 6 139712 jchaffraix 2012-05-01 17:28:51 -0700 Comment on attachment 139712 Patch It sounds like we should annotate the function to say that we should remove it and have people not relying on it. 613826 7 139717 eric 2012-05-01 17:33:22 -0700 Created attachment 139717 Patch for landing 613876 8 139717 webkit.review.bot 2012-05-01 19:08:51 -0700 Comment on attachment 139717 Patch for landing Clearing flags on attachment: 139717 Committed r115779: <http://trac.webkit.org/changeset/115779> 613877 9 webkit.review.bot 2012-05-01 19:08:56 -0700 All reviewed patches have been landed. Closing bug. 614502 10 139717 darin 2012-05-02 12:02:09 -0700 Comment on attachment 139717 Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=139717&action=review > Source/WebCore/rendering/RenderView.h:60 > + // FIXME: This override is not needed and should be removed > + // it only exists to make computePreferredLogicalWidths public. > + virtual void computePreferredLogicalWidths() OVERRIDE; The way to do that is: using RenderBlock::computePreferredLogicalWidths; That makes computePreferredLogicalWidths public in RenderView without adding an override. 614871 11 eric 2012-05-02 17:48:33 -0700 (In reply to comment #10) > (From update of attachment 139717 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=139717&action=review > > > Source/WebCore/rendering/RenderView.h:60 > > + // FIXME: This override is not needed and should be removed > > + // it only exists to make computePreferredLogicalWidths public. > > + virtual void computePreferredLogicalWidths() OVERRIDE; > > The way to do that is: > > using RenderBlock::computePreferredLogicalWidths; > > That makes computePreferredLogicalWidths public in RenderView without adding an override. Cute. I wasn't aware of that aspect of c++. It's unclear that this function even needs to exist. I believe removing it has a small chance of breaking some (possibly untested) frame flattening code paths, so I've left it for now. I would like to go back and see about removing this function entirely (as I had on my branch), once I'm done landing seamless. 139712 2012-05-01 16:51:34 -0700 2012-05-01 17:33:15 -0700 Patch bug-85325-20120501165133.patch text/plain 2448 eric U3VidmVyc2lvbiBSZXZpc2lvbjogMTE1NzU1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMTExMjY4NjdjYzhlNjAz MzRkYTY0MDVhNTdkMmY1Mjk4OGJhODFmMy4uMzAzZmJiNzBkOWZmMWNhYjUwNjJkY2FiZjI0YmJm ZjJlZjEyYTU4YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDMwIEBACiAyMDEyLTA1LTAxICBFcmlj IFNlaWRlbCAgPGVyaWNAd2Via2l0Lm9yZz4KIAorICAgICAgICBSZW1vdmUgdW5lZWRlZCBtaW4v bWF4IHByZWYgd2lkdGggYXNzaWdubWVudCBmcm9tIFJlbmRlclZpZXcKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTg1MzI1CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhpcyBjb2RlIGhhcyBiZWVuIHdpdGgg dXMgc2luY2UgdGhlIG9yaWdpbmFsIGltcG9ydCBmcm9tIEtERToKKyAgICAgICAgaHR0cDovL3Ry YWMud2Via2l0Lm9yZy9icm93c2VyL3RydW5rL1dlYkNvcmUva2h0bWwvcmVuZGVyaW5nL3JlbmRl cl9yb290LmNwcD9hbm5vdGF0ZT1ibGFtZSZyZXY9NCNMOTMKKyAgICAgICAgSXQncyBuZXZlciBi ZWVuIGRvY3VtZW50ZWQsIG9yIGV4cGxhaW5lZC4gIFJlbW92aW5nIGl0IHNob3dlZCBubworICAg ICAgICBlZmZlY3Qgb24gbXkgbG9jYWwgbGF5b3V0IHRlc3RzIHJ1bi4KKyAgICAgICAgSG93ZXZl ciB0aGlzIGNvZGUgYmxvY2tzIHByb3BlciBpbXBsZW1lbnRhdGlvbiBvZiA8aWZyYW1lIHNlYW1s ZXNzPgorICAgICAgICBhcyB3ZSBoYXZlIHRvIGRvIHByb3BlciBtaW4vbWF4IHdpZHRoIG5lZ290 aWF0aW9uIGFjcm9zcyB0aGUgaWZyYW1lIGJvdW5kYXJ5LgorCisgICAgICAgIEkgd291bGQgcmVt b3ZlIHRoZSB3aG9sZSBmdW5jdGlvbiwgYnV0IGRvaW5nIHNvIG9wZW5zIGEgd2hvbGUgY2FuIG9m IHdvcm1zCisgICAgICAgIGFzIHRoaXMgb3ZlcnJpZGUgaXMgcHVibGljLCB5ZXQgbm9ybWFsbHkg dGhpcyBmdW5jdGlvbiBpcyAqcHJpdmF0ZSogKHdlbGwgcHJvdGVjdGVkIG9uIFJlbmRlckJveCku CisgICAgICAgIEl0IHNlZW1zIHBsYXVzaWJsZSB0aGF0IGZyYW1lIGZsYXR0ZW5pbmcgY29kZSBu ZWVkcyB0aGlzIG92ZXJyaWRlIHNpbmNlIGl0IGRvZXNuJ3QgYWx3YXlzCisgICAgICAgIGNhbGwg dGhlIG1pbi9tYXhQcmVmZXJyZWRXaWR0aCgpIGNhbGxzIHdoaWNoIG5vcm1hbGx5IGF1dG9tYXRp Y2FsbHkgY2FsbCB0aGlzIGNvbXB1dGUqCisgICAgICAgIGZ1bmN0aW9uIGlmIHRoZSBwcmVmLXdp ZHRocyBhcmUgZGlydHkuCisgICAgICAgIEluc3RlYWQgb2YgdHJ5aW5nIHRvIHRyYWNrIHRoYXQg YWxsIGRvd24sIEknbSBqdXN0IHJlbW92aW5nIHRoaXMgbGluZSwgYW5kIHdlJ2xsIGdvCisgICAg ICAgIGJhY2sgYW5kIHJlbW92ZSB0aGUgd2hvbGUgZnVuY3Rpb24gYXQgYSBsYXRlciBkYXRlIGlm IHBvc3NpYmxlLgorCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclZpZXcuY3BwOgorICAgICAg ICAoV2ViQ29yZTo6UmVuZGVyVmlldzo6Y29tcHV0ZVByZWZlcnJlZExvZ2ljYWxXaWR0aHMpOgor CisyMDEyLTA1LTAxICBFcmljIFNlaWRlbCAgPGVyaWNAd2Via2l0Lm9yZz4KKwogICAgICAgICBB ZGQgc3VwcG9ydCBmb3Igc2VhbWxlc3MgYXR0cmlidXRlIGFzIHdlbGwgYXMgc2VhbWxlc3Mgc2Fu ZGJveCBmbGFnIGFuZCBkZWZhdWx0IENTUyBzdHlsaW5nCiAgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04NTMwMgogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi Q29yZS9yZW5kZXJpbmcvUmVuZGVyVmlldy5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcv UmVuZGVyVmlldy5jcHAKaW5kZXggN2YyNzA5OGZlNTBhN2NlMjY0ZjZkMzUzZDFkYjUyN2U4ZjEw MGRjZi4uYWM1NmJmMTc2OWJjNGY3NjRhZmFiMmM5NjU0MDliMWNkMTI4NTcyNSAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclZpZXcuY3BwCisrKyBiL1NvdXJjZS9X ZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJWaWV3LmNwcApAQCAtOTksOCArOTksNiBAQCB2b2lkIFJl bmRlclZpZXc6OmNvbXB1dGVQcmVmZXJyZWRMb2dpY2FsV2lkdGhzKCkKICAgICBBU1NFUlQocHJl ZmVycmVkTG9naWNhbFdpZHRoc0RpcnR5KCkpOwogCiAgICAgUmVuZGVyQmxvY2s6OmNvbXB1dGVQ cmVmZXJyZWRMb2dpY2FsV2lkdGhzKCk7Ci0KLSAgICBtX21heFByZWZlcnJlZExvZ2ljYWxXaWR0 aCA9IG1fbWluUHJlZmVycmVkTG9naWNhbFdpZHRoOwogfQogCiBib29sIFJlbmRlclZpZXc6Omlz Q2hpbGRBbGxvd2VkKFJlbmRlck9iamVjdCogY2hpbGQsIFJlbmRlclN0eWxlKikgY29uc3QK 139717 2012-05-01 17:33:22 -0700 2012-05-02 12:02:09 -0700 Patch for landing bug-85325-20120501173320.patch text/plain 3975 eric U3VidmVyc2lvbiBSZXZpc2lvbjogMTE1NzU1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMTExMjY4NjdjYzhlNjAz MzRkYTY0MDVhNTdkMmY1Mjk4OGJhODFmMy4uOWIxMDJhNGNiMjZlOWI4ZjNhZDExYzBkYWJkMzAy ZGM0N2ViMDNkYiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDMwIEBACiAyMDEyLTA1LTAxICBFcmlj IFNlaWRlbCAgPGVyaWNAd2Via2l0Lm9yZz4KIAorICAgICAgICBSZW1vdmUgdW5lZWRlZCBtaW4v bWF4IHByZWYgd2lkdGggYXNzaWdubWVudCBmcm9tIFJlbmRlclZpZXcKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTg1MzI1CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgSnVsaWVuIENoYWZmcmFpeC4KKworICAgICAgICBUaGlzIGNvZGUgaGFzIGJlZW4gd2l0 aCB1cyBzaW5jZSB0aGUgb3JpZ2luYWwgaW1wb3J0IGZyb20gS0RFOgorICAgICAgICBodHRwOi8v dHJhYy53ZWJraXQub3JnL2Jyb3dzZXIvdHJ1bmsvV2ViQ29yZS9raHRtbC9yZW5kZXJpbmcvcmVu ZGVyX3Jvb3QuY3BwP2Fubm90YXRlPWJsYW1lJnJldj00I0w5MworICAgICAgICBJdCdzIG5ldmVy IGJlZW4gZG9jdW1lbnRlZCwgb3IgZXhwbGFpbmVkLiAgUmVtb3ZpbmcgaXQgc2hvd2VkIG5vCisg ICAgICAgIGVmZmVjdCBvbiBteSBsb2NhbCBsYXlvdXQgdGVzdHMgcnVuLgorICAgICAgICBIb3dl dmVyIHRoaXMgY29kZSBibG9ja3MgcHJvcGVyIGltcGxlbWVudGF0aW9uIG9mIDxpZnJhbWUgc2Vh bWxlc3M+CisgICAgICAgIGFzIHdlIGhhdmUgdG8gZG8gcHJvcGVyIG1pbi9tYXggd2lkdGggbmVn b3RpYXRpb24gYWNyb3NzIHRoZSBpZnJhbWUgYm91bmRhcnkuCisKKyAgICAgICAgSSB3b3VsZCBy ZW1vdmUgdGhlIHdob2xlIGZ1bmN0aW9uLCBidXQgZG9pbmcgc28gb3BlbnMgYSB3aG9sZSBjYW4g b2Ygd29ybXMKKyAgICAgICAgYXMgdGhpcyBvdmVycmlkZSBpcyBwdWJsaWMsIHlldCBub3JtYWxs eSB0aGlzIGZ1bmN0aW9uIGlzICpwcml2YXRlKiAod2VsbCBwcm90ZWN0ZWQgb24gUmVuZGVyQm94 KS4KKyAgICAgICAgSXQgc2VlbXMgcGxhdXNpYmxlIHRoYXQgZnJhbWUgZmxhdHRlbmluZyBjb2Rl IG5lZWRzIHRoaXMgb3ZlcnJpZGUgc2luY2UgaXQgZG9lc24ndCBhbHdheXMKKyAgICAgICAgY2Fs bCB0aGUgbWluL21heFByZWZlcnJlZFdpZHRoKCkgY2FsbHMgd2hpY2ggbm9ybWFsbHkgYXV0b21h dGljYWxseSBjYWxsIHRoaXMgY29tcHV0ZSoKKyAgICAgICAgZnVuY3Rpb24gaWYgdGhlIHByZWYt d2lkdGhzIGFyZSBkaXJ0eS4KKyAgICAgICAgSW5zdGVhZCBvZiB0cnlpbmcgdG8gdHJhY2sgdGhh dCBhbGwgZG93biwgSSdtIGp1c3QgcmVtb3ZpbmcgdGhpcyBsaW5lLCBhbmQgd2UnbGwgZ28KKyAg ICAgICAgYmFjayBhbmQgcmVtb3ZlIHRoZSB3aG9sZSBmdW5jdGlvbiBhdCBhIGxhdGVyIGRhdGUg aWYgcG9zc2libGUuCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVyVmlldy5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpSZW5kZXJWaWV3Ojpjb21wdXRlUHJlZmVycmVkTG9naWNhbFdpZHRocyk6 CisKKzIwMTItMDUtMDEgIEVyaWMgU2VpZGVsICA8ZXJpY0B3ZWJraXQub3JnPgorCiAgICAgICAg IEFkZCBzdXBwb3J0IGZvciBzZWFtbGVzcyBhdHRyaWJ1dGUgYXMgd2VsbCBhcyBzZWFtbGVzcyBz YW5kYm94IGZsYWcgYW5kIGRlZmF1bHQgQ1NTIHN0eWxpbmcKICAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTg1MzAyCiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJWaWV3LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmlu Zy9SZW5kZXJWaWV3LmNwcAppbmRleCA3ZjI3MDk4ZmU1MGE3Y2UyNjRmNmQzNTNkMWRiNTI3ZThm MTAwZGNmLi5hYzU2YmYxNzY5YmM0Zjc2NGFmYWIyYzk2NTQwOWIxY2QxMjg1NzI1IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVmlldy5jcHAKKysrIGIvU291cmNl L1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclZpZXcuY3BwCkBAIC05OSw4ICs5OSw2IEBAIHZvaWQg UmVuZGVyVmlldzo6Y29tcHV0ZVByZWZlcnJlZExvZ2ljYWxXaWR0aHMoKQogICAgIEFTU0VSVChw cmVmZXJyZWRMb2dpY2FsV2lkdGhzRGlydHkoKSk7CiAKICAgICBSZW5kZXJCbG9jazo6Y29tcHV0 ZVByZWZlcnJlZExvZ2ljYWxXaWR0aHMoKTsKLQotICAgIG1fbWF4UHJlZmVycmVkTG9naWNhbFdp ZHRoID0gbV9taW5QcmVmZXJyZWRMb2dpY2FsV2lkdGg7CiB9CiAKIGJvb2wgUmVuZGVyVmlldzo6 aXNDaGlsZEFsbG93ZWQoUmVuZGVyT2JqZWN0KiBjaGlsZCwgUmVuZGVyU3R5bGUqKSBjb25zdApk aWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclZpZXcuaCBiL1NvdXJj ZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJWaWV3LmgKaW5kZXggZTZiYWEwOWRmOTI0ZTFmOTBi NTVmNGJjYjcwNDYzNzY4YWE3MGQ0Ni4uYjZmZmE0NzYxOTI3ZmFkMTA1ZmZlZWI4ODhmYWI0ODMx MjM1NTE0NSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclZpZXcu aAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVmlldy5oCkBAIC00NCwxOCAr NDQsMjAgQEAgcHVibGljOgogCiAgICAgYm9vbCBoaXRUZXN0KGNvbnN0IEhpdFRlc3RSZXF1ZXN0 JiwgSGl0VGVzdFJlc3VsdCYpOwogCi0gICAgdmlydHVhbCBjb25zdCBjaGFyKiByZW5kZXJOYW1l KCkgY29uc3QgeyByZXR1cm4gIlJlbmRlclZpZXciOyB9CisgICAgdmlydHVhbCBjb25zdCBjaGFy KiByZW5kZXJOYW1lKCkgY29uc3QgT1ZFUlJJREUgeyByZXR1cm4gIlJlbmRlclZpZXciOyB9CiAK LSAgICB2aXJ0dWFsIGJvb2wgaXNSZW5kZXJWaWV3KCkgY29uc3QgeyByZXR1cm4gdHJ1ZTsgfQor ICAgIHZpcnR1YWwgYm9vbCBpc1JlbmRlclZpZXcoKSBjb25zdCBPVkVSUklERSB7IHJldHVybiB0 cnVlOyB9CiAKLSAgICB2aXJ0dWFsIGJvb2wgcmVxdWlyZXNMYXllcigpIGNvbnN0IHsgcmV0dXJu IHRydWU7IH0KKyAgICB2aXJ0dWFsIGJvb2wgcmVxdWlyZXNMYXllcigpIGNvbnN0IE9WRVJSSURF IHsgcmV0dXJuIHRydWU7IH0KIAotICAgIHZpcnR1YWwgYm9vbCBpc0NoaWxkQWxsb3dlZChSZW5k ZXJPYmplY3QqLCBSZW5kZXJTdHlsZSopIGNvbnN0OworICAgIHZpcnR1YWwgYm9vbCBpc0NoaWxk QWxsb3dlZChSZW5kZXJPYmplY3QqLCBSZW5kZXJTdHlsZSopIGNvbnN0IE9WRVJSSURFOwogCi0g ICAgdmlydHVhbCB2b2lkIGxheW91dCgpOwotICAgIHZpcnR1YWwgdm9pZCBjb21wdXRlTG9naWNh bFdpZHRoKCk7Ci0gICAgdmlydHVhbCB2b2lkIGNvbXB1dGVMb2dpY2FsSGVpZ2h0KCk7Ci0gICAg dmlydHVhbCB2b2lkIGNvbXB1dGVQcmVmZXJyZWRMb2dpY2FsV2lkdGhzKCk7CisgICAgdmlydHVh bCB2b2lkIGxheW91dCgpIE9WRVJSSURFOworICAgIHZpcnR1YWwgdm9pZCBjb21wdXRlTG9naWNh bFdpZHRoKCkgT1ZFUlJJREU7CisgICAgdmlydHVhbCB2b2lkIGNvbXB1dGVMb2dpY2FsSGVpZ2h0 KCkgT1ZFUlJJREU7CisgICAgLy8gRklYTUU6IFRoaXMgb3ZlcnJpZGUgaXMgbm90IG5lZWRlZCBh bmQgc2hvdWxkIGJlIHJlbW92ZWQKKyAgICAvLyBpdCBvbmx5IGV4aXN0cyB0byBtYWtlIGNvbXB1 dGVQcmVmZXJyZWRMb2dpY2FsV2lkdGhzIHB1YmxpYy4KKyAgICB2aXJ0dWFsIHZvaWQgY29tcHV0 ZVByZWZlcnJlZExvZ2ljYWxXaWR0aHMoKSBPVkVSUklERTsKIAogICAgIC8vIFRoZSBzYW1lIGFz IHRoZSBGcmFtZVZpZXcncyBsYXlvdXRIZWlnaHQvbGF5b3V0V2lkdGggYnV0IHdpdGggbnVsbCBj aGVjayBndWFyZHMuCiAgICAgaW50IHZpZXdIZWlnaHQoKSBjb25zdDsK