123860 2013-11-05 20:47:17 -0800 Widget's position change should not initiate layout, only when its size changes. 2013-11-05 23:29:57 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 zalan zalan commit-queue esprehn+autocc glenn kling kondapallykalyan oldest_to_newest 947497 0 zalan 2013-11-05 20:47:17 -0800 SSIA 947506 1 216121 zalan 2013-11-05 21:17:14 -0800 Created attachment 216121 Patch 947520 2 216121 kling 2013-11-05 21:42:08 -0800 Comment on attachment 216121 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=216121&action=review r=me > Source/WebCore/ChangeLog:12 > + Manual test added. Unfortunately we can't test against the number of layouts yet. It would be good to do something about this. :) > Source/WebCore/rendering/RenderWidget.cpp:147 > + if (boundsChanged && hasLayer() && layer()->isComposited()) > layer()->backing()->updateAfterWidgetResize(); The name updateAfterWidgetResize() makes it sound like we wouldn't need to call it after position-only changes. If so, we should avoid the extra work. If not, we should rename it to something more accurate, e.g updateAfterWidgetBoundsChange(). > Source/WebCore/rendering/RenderWidget.cpp:335 > - // if the frame bounds got changed, or if view needs layout (possibly indicating > - // content size is wrong) we have to do a layout to set the right widget size > - if (m_widget && m_widget->isFrameView()) { > + // if the frame size got changed, or if view needs layout (possibly indicating > + // content size is wrong) we have to do a layout to set the right widget size. > + if (m_widget->isFrameView()) { I'm a bit spooked by removing the null check here, but I don't see how we would be here with a null widget. If we were being destroyed, the weak pointer guard above would have taken care of it. It would be nice if we could get rid of this possible state by enforcing presence of a Widget at compile-time. Might be a bit non-trivial though. 947522 3 zalan 2013-11-05 21:45:10 -0800 > It would be good to do something about this. :) > > > Source/WebCore/rendering/RenderWidget.cpp:147 > > + if (boundsChanged && hasLayer() && layer()->isComposited()) > > layer()->backing()->updateAfterWidgetResize(); > > The name updateAfterWidgetResize() makes it sound like we wouldn't need to call it after position-only changes. > If so, we should avoid the extra work. If not, we should rename it to something more accurate, e.g updateAfterWidgetBoundsChange(). It is indeed wrong naming as it calls both position and size changes. Will fix it. 947571 4 216121 commit-queue 2013-11-05 23:29:55 -0800 Comment on attachment 216121 Patch Clearing flags on attachment: 216121 Committed r158727: <http://trac.webkit.org/changeset/158727> 947572 5 commit-queue 2013-11-05 23:29:57 -0800 All reviewed patches have been landed. Closing bug. 216121 2013-11-05 21:17:14 -0800 2013-11-05 23:29:55 -0800 Patch bug-123860-20131105211713.patch text/plain 6160 zalan U3VidmVyc2lvbiBSZXZpc2lvbjogMTU4NjkwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZWNiNjgxZGIzZTE2MTdl MTkzZDE0YmU1Y2FmZWYzMTE1ZjRjNGQzYi4uMzk0NzI1YjVlZDNjOWUyZjQzMGUyMmQ1OWIyZGJm MjkxM2EwNjc3YyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDEzLTExLTA1ICBaYWxh biBCdWp0YXMgIDx6YWxhbkBhcHBsZS5jb20+CisKKyAgICAgICAgV2lkZ2V0J3MgcG9zaXRpb24g Y2hhbmdlIHNob3VsZCBub3QgaW5pdGlhdGUgbGF5b3V0LCBvbmx5IHdoZW4gaXRzIHNpemUgY2hh bmdlcy4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEy Mzg2MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJl bmRlcldpZGdldHMgaW5pdGlhdGUgdW5uZWNlc3NhcnkgbGF5b3V0cyB3aGlsZSBzY3JvbGxpbmcg d2hlbiB0aGV5IGFyZSBlbWJlZGRlZCB0bworICAgICAgICBvdmVyZmxvdzpzY3JvbGwgY29udGFp bmVycy4gU2Nyb2xsIHBvc2l0aW9uIGNoYW5nZSBkb2Vzbid0IGRpcnR5IHRoZSByZW5kZXIgdHJl ZQorICAgICAgICBzbyBpdCBzaG91bGQgbm90IHRyaWdnZXIgbGF5b3V0IGVpdGhlci4KKworICAg ICAgICBNYW51YWwgdGVzdCBhZGRlZC4gVW5mb3J0dW5hdGVseSB3ZSBjYW4ndCB0ZXN0IGFnYWlu c3QgdGhlIG51bWJlciBvZiBsYXlvdXRzIHlldC4KKworICAgICAgICAqIHJlbmRlcmluZy9SZW5k ZXJXaWRnZXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyV2lkZ2V0OjpzZXRXaWRnZXRH ZW9tZXRyeSk6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJXaWRnZXQ6OnVwZGF0ZVdpZGdldEdl b21ldHJ5KToKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlcldpZGdldDo6dXBkYXRlV2lkZ2V0UG9z aXRpb24pOgorCiAyMDEzLTExLTA1ICBDb21taXQgUXVldWUgIDxjb21taXQtcXVldWVAd2Via2l0 Lm9yZz4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTU4Njc4LgpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlcldpZGdldC5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyV2lkZ2V0LmNwcAppbmRleCA5NzRlNmM0ODU1NjlmZWRi NjNmZGVhOGYwY2Y3NGY0ZTAxM2ExMDUxLi45ZTk3NzgxNTJhMjU4YWQ1ODU1ZTQ4NzIxMTA1NDZk YTE2ZGJjNjBiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyV2lk Z2V0LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyV2lkZ2V0LmNwcApA QCAtMTIyLDkgKzEyMiwxMCBAQCBzdGF0aWMgaW5saW5lIEludFJlY3Qgcm91bmRlZEludFJlY3Qo Y29uc3QgTGF5b3V0UmVjdCYgcmVjdCkKIGJvb2wgUmVuZGVyV2lkZ2V0OjpzZXRXaWRnZXRHZW9t ZXRyeShjb25zdCBMYXlvdXRSZWN0JiBmcmFtZSkKIHsKICAgICBJbnRSZWN0IGNsaXBSZWN0ID0g cm91bmRlZEludFJlY3QoZW5jbG9zaW5nTGF5ZXIoKS0+Y2hpbGRyZW5DbGlwUmVjdCgpKTsKLSAg ICBJbnRSZWN0IG5ld0ZyYW1lID0gcm91bmRlZEludFJlY3QoZnJhbWUpOworICAgIEludFJlY3Qg bmV3RnJhbWVSZWN0ID0gcm91bmRlZEludFJlY3QoZnJhbWUpOworICAgIEludFJlY3Qgb2xkRnJh bWVSZWN0ID0gbV93aWRnZXQtPmZyYW1lUmVjdCgpOwogICAgIGJvb2wgY2xpcENoYW5nZWQgPSBt X2NsaXBSZWN0ICE9IGNsaXBSZWN0OwotICAgIGJvb2wgYm91bmRzQ2hhbmdlZCA9IG1fd2lkZ2V0 LT5mcmFtZVJlY3QoKSAhPSBuZXdGcmFtZTsKKyAgICBib29sIGJvdW5kc0NoYW5nZWQgPSBvbGRG cmFtZVJlY3QgIT0gbmV3RnJhbWVSZWN0OwogCiAgICAgaWYgKCFib3VuZHNDaGFuZ2VkICYmICFj bGlwQ2hhbmdlZCkKICAgICAgICAgcmV0dXJuIGZhbHNlOwpAQCAtMTM0LDcgKzEzNSw3IEBAIGJv b2wgUmVuZGVyV2lkZ2V0OjpzZXRXaWRnZXRHZW9tZXRyeShjb25zdCBMYXlvdXRSZWN0JiBmcmFt ZSkKICAgICBXZWFrUHRyPFJlbmRlcldpZGdldD4gd2Vha1RoaXMgPSBjcmVhdGVXZWFrUHRyKCk7 CiAgICAgLy8gVGhlc2UgY2FsbHMgKm1heSogY2F1c2UgdGhpcyByZW5kZXJlciB0byBkaXNhcHBl YXIgZnJvbSB1bmRlcm5lYXRoLi4uCiAgICAgaWYgKGJvdW5kc0NoYW5nZWQpCi0gICAgICAgIG1f d2lkZ2V0LT5zZXRGcmFtZVJlY3QobmV3RnJhbWUpOworICAgICAgICBtX3dpZGdldC0+c2V0RnJh bWVSZWN0KG5ld0ZyYW1lUmVjdCk7CiAgICAgZWxzZSBpZiAoY2xpcENoYW5nZWQpCiAgICAgICAg IG1fd2lkZ2V0LT5jbGlwUmVjdENoYW5nZWQoKTsKICAgICAvLyAuLi5zbyB3ZSBmb2xsb3cgdXAg d2l0aCBhIHNhbml0eSBjaGVjay4KQEAgLTE0MiwxOSArMTQzLDE4IEBAIGJvb2wgUmVuZGVyV2lk Z2V0OjpzZXRXaWRnZXRHZW9tZXRyeShjb25zdCBMYXlvdXRSZWN0JiBmcmFtZSkKICAgICAgICAg cmV0dXJuIHRydWU7CiAKICNpZiBVU0UoQUNDRUxFUkFURURfQ09NUE9TSVRJTkcpCi0gICAgaWYg KGhhc0xheWVyKCkgJiYgbGF5ZXIoKS0+aXNDb21wb3NpdGVkKCkpCisgICAgaWYgKGJvdW5kc0No YW5nZWQgJiYgaGFzTGF5ZXIoKSAmJiBsYXllcigpLT5pc0NvbXBvc2l0ZWQoKSkKICAgICAgICAg bGF5ZXIoKS0+YmFja2luZygpLT51cGRhdGVBZnRlcldpZGdldFJlc2l6ZSgpOwogI2VuZGlmCi0g ICAgCi0gICAgcmV0dXJuIGJvdW5kc0NoYW5nZWQ7CisgICAgcmV0dXJuIG9sZEZyYW1lUmVjdC5z aXplKCkgIT0gbmV3RnJhbWVSZWN0LnNpemUoKTsKIH0KIAogYm9vbCBSZW5kZXJXaWRnZXQ6OnVw ZGF0ZVdpZGdldEdlb21ldHJ5KCkKIHsKLSAgICBMYXlvdXRSZWN0IGNvbnRlbnRCb3ggPSBjb250 ZW50Qm94UmVjdCgpOwogICAgIGlmICghbV93aWRnZXQtPnRyYW5zZm9ybXNBZmZlY3RGcmFtZVJl Y3QoKSkKICAgICAgICAgcmV0dXJuIHNldFdpZGdldEdlb21ldHJ5KGFic29sdXRlQ29udGVudEJv eCgpKTsKIAorICAgIExheW91dFJlY3QgY29udGVudEJveCA9IGNvbnRlbnRCb3hSZWN0KCk7CiAg ICAgTGF5b3V0UmVjdCBhYnNvbHV0ZUNvbnRlbnRCb3gobG9jYWxUb0Fic29sdXRlUXVhZChGbG9h dFF1YWQoY29udGVudEJveCkpLmJvdW5kaW5nQm94KCkpOwogICAgIGlmIChtX3dpZGdldC0+aXNG cmFtZVZpZXcoKSkgewogICAgICAgICBjb250ZW50Qm94LnNldExvY2F0aW9uKGFic29sdXRlQ29u dGVudEJveC5sb2NhdGlvbigpKTsKQEAgLTMyNiwxOCArMzI2LDE2IEBAIHZvaWQgUmVuZGVyV2lk Z2V0Ojp1cGRhdGVXaWRnZXRQb3NpdGlvbigpCiAgICAgICAgIHJldHVybjsKIAogICAgIFdlYWtQ dHI8UmVuZGVyV2lkZ2V0PiB3ZWFrVGhpcyA9IGNyZWF0ZVdlYWtQdHIoKTsKLQotICAgIGJvb2wg Ym91bmRzQ2hhbmdlZCA9IHVwZGF0ZVdpZGdldEdlb21ldHJ5KCk7Ci0KKyAgICBib29sIHdpZGdl dFNpemVDaGFuZ2VkID0gdXBkYXRlV2lkZ2V0R2VvbWV0cnkoKTsKICAgICBpZiAoIXdlYWtUaGlz KQogICAgICAgICByZXR1cm47CiAKLSAgICAvLyBpZiB0aGUgZnJhbWUgYm91bmRzIGdvdCBjaGFu Z2VkLCBvciBpZiB2aWV3IG5lZWRzIGxheW91dCAocG9zc2libHkgaW5kaWNhdGluZwotICAgIC8v IGNvbnRlbnQgc2l6ZSBpcyB3cm9uZykgd2UgaGF2ZSB0byBkbyBhIGxheW91dCB0byBzZXQgdGhl IHJpZ2h0IHdpZGdldCBzaXplCi0gICAgaWYgKG1fd2lkZ2V0ICYmIG1fd2lkZ2V0LT5pc0ZyYW1l VmlldygpKSB7CisgICAgLy8gaWYgdGhlIGZyYW1lIHNpemUgZ290IGNoYW5nZWQsIG9yIGlmIHZp ZXcgbmVlZHMgbGF5b3V0IChwb3NzaWJseSBpbmRpY2F0aW5nCisgICAgLy8gY29udGVudCBzaXpl IGlzIHdyb25nKSB3ZSBoYXZlIHRvIGRvIGEgbGF5b3V0IHRvIHNldCB0aGUgcmlnaHQgd2lkZ2V0 IHNpemUuCisgICAgaWYgKG1fd2lkZ2V0LT5pc0ZyYW1lVmlldygpKSB7CiAgICAgICAgIEZyYW1l VmlldyogZnJhbWVWaWV3ID0gdG9GcmFtZVZpZXcobV93aWRnZXQuZ2V0KCkpOwogICAgICAgICAv LyBDaGVjayB0aGUgZnJhbWUncyBwYWdlIHRvIG1ha2Ugc3VyZSB0aGF0IHRoZSBmcmFtZSBpc24n dCBpbiB0aGUgcHJvY2VzcyBvZiBiZWluZyBkZXN0cm95ZWQuCi0gICAgICAgIGlmICgoYm91bmRz Q2hhbmdlZCB8fCBmcmFtZVZpZXctPm5lZWRzTGF5b3V0KCkpICYmIGZyYW1lVmlldy0+ZnJhbWUo KS5wYWdlKCkpCisgICAgICAgIGlmICgod2lkZ2V0U2l6ZUNoYW5nZWQgfHwgZnJhbWVWaWV3LT5u ZWVkc0xheW91dCgpKSAmJiBmcmFtZVZpZXctPmZyYW1lKCkucGFnZSgpKQogICAgICAgICAgICAg ZnJhbWVWaWV3LT5sYXlvdXQoKTsKICAgICB9CiB9CmRpZmYgLS1naXQgYS9DaGFuZ2VMb2cgYi9D aGFuZ2VMb2cKaW5kZXggMmE5NDFjYWY2OTg4Y2Y1ZTg2ODkxZTA5NzJiN2UxYzU4NzdlOGRiYS4u ZWVmOTAzZGMzNzMyMGJiZmVjMTE1ODI1MTliYmE0YzQwZWZmZjAwMyAxMDA2NDQKLS0tIGEvQ2hh bmdlTG9nCisrKyBiL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTExLTA1ICBaYWxh biBCdWp0YXMgIDx6YWxhbkBhcHBsZS5jb20+CisKKyAgICAgICAgV2lkZ2V0J3MgcG9zaXRpb24g Y2hhbmdlIHNob3VsZCBub3QgaW5pdGlhdGUgbGF5b3V0LCBvbmx5IHdoZW4gaXRzIHNpemUgY2hh bmdlcy4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEy Mzg2MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJl bmRlcldpZGdldHMgaW5pdGlhdGUgdW5uZWNlc3NhcnkgbGF5b3V0cyB3aGlsZSBzY3JvbGxpbmcg d2hlbiB0aGV5IGFyZSBlbWJlZGRlZCB0bworICAgICAgICBvdmVyZmxvdzpzY3JvbGwgY29udGFp bmVycy4gU2Nyb2xsIHBvc2l0aW9uIGNoYW5nZSBkb2Vzbid0IGRpcnR5IHRoZSByZW5kZXIgdHJl ZQorICAgICAgICBzbyBpdCBzaG91bGQgbm90IHRyaWdnZXIgbGF5b3V0IGVpdGhlci4KKworICAg ICAgICAqIE1hbnVhbFRlc3RzL2xheW91dHMtb24tcmVuZGVyd2lkZ2V0cy13aGlsZS1zY3JvbGxp bmcuaHRtbDogQWRkZWQuCisKIDIwMTMtMTEtMDUgIMOJdmEgQmFsw6F6c2ZhbHZpICA8YmFsYXpz ZmFsdmkuZXZhQHN0dWQudS1zemVnZWQuaHU+CiAKICAgICAgICAgUmVtb3ZlIGxlZnRvdmVyIFF0 IHJlbGF0ZWQgdGhpbmdzIGZyb20gV2ViS2l0TWFjcm9zLmNtYWtlCmRpZmYgLS1naXQgYS9NYW51 YWxUZXN0cy9sYXlvdXRzLW9uLXJlbmRlcndpZGdldHMtd2hpbGUtc2Nyb2xsaW5nLmh0bWwgYi9N YW51YWxUZXN0cy9sYXlvdXRzLW9uLXJlbmRlcndpZGdldHMtd2hpbGUtc2Nyb2xsaW5nLmh0bWwK bmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMDAwMC4uYWNlYmNhOTY3ZjNiM2YwNzE1MWUwOTg0ZWQ5OWNjYTZjZTkzYWUxYgotLS0g L2Rldi9udWxsCisrKyBiL01hbnVhbFRlc3RzL2xheW91dHMtb24tcmVuZGVyd2lkZ2V0cy13aGls ZS1zY3JvbGxpbmcuaHRtbApAQCAtMCwwICsxLDI2IEBACis8IWRvY3R5cGUgaHRtbD4KKzxodG1s PgorPGhlYWQ+CisgIDxzdHlsZT4KKyAgICBodG1sLCBib2R5IHsKKyAgICAgIGhlaWdodDogMTAw JTsKKyAgICB9CisgICAgCisgICAgLmNvbnRhaW5lciB7CisgICAgICBoZWlnaHQ6IDEwMCU7Cisg ICAgICBvdmVyZmxvdzogc2Nyb2xsOworICAgIH0KKyAgICBpZnJhbWUgeworICAgICAgZGlzcGxh eTogYmxvY2s7CisgICAgICB3aWR0aDogMjAwcHg7CisgICAgICBoZWlnaHQ6IDYwMHB4OworICAg IH0KKyAgPC9zdHlsZT4KKzwvaGVhZD4KKzxib2R5PgorICA8ZGl2IGNsYXNzPSJjb250YWluZXIi PgorICAgIDxpZnJhbWUgc3JjPSJkYXRhOnRleHQvaHRtbCxTY3JvbGwgdGhpcyBvdXQgb2YgdGhl IHZpZXcuIj48L2lmcmFtZT4KKyAgICA8aWZyYW1lIHNyYz0iZGF0YTp0ZXh0L2h0bWwsU3RvcCBo ZXJlIGFuZCBpbnNwZWN0IGlmIGxheW91dHMgZ290IGluaXRpYXRlZCwgd2hpbGUgc2NvbGxpbmcg aW4gdGhlIFdlYiBJbnNwZWN0b3IuIj48L2lmcmFtZT4KKyAgPC9kaXY+Cis8L2JvZHk+Cis8L2h0 bWw+Cg==