147134 2015-07-20 17:57:39 -0700 Facebook in tiled fullscreen is slow 2015-07-20 19:02:44 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 simon.fraser simon.fraser simon.fraser thorton oldest_to_newest 1110851 0 simon.fraser 2015-07-20 17:57:39 -0700 Facebook in tiled fullscreen is slow 1110858 1 257154 simon.fraser 2015-07-20 18:19:30 -0700 Created attachment 257154 Patch 1110859 2 simon.fraser 2015-07-20 18:20:00 -0700 rdar://problem/21823349 1110861 3 257154 sabouhallawa 2015-07-20 18:39:10 -0700 Comment on attachment 257154 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257154&action=review > Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:291 > + if (!documentWidthChanged && !viewWidthChanged) why the check for m_webPage.mainFrame()->view()->needsLayout() was removed? > Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:300 > + if (m_isScalingViewToFitDocument && !documentWidthChanged) { Does m_isScalingViewToFitDocument need to be a member of the class? > Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:336 > + IntSize fixedLayoutSize(documentWidth, std::ceil((m_webPage.size().height() - m_webPage.corePage()->topContentInset()) / viewScale)); This block is very similar to the above one, can't the code be refactored such that the same code is not repeated? 1110862 4 257154 thorton 2015-07-20 18:45:10 -0700 Comment on attachment 257154 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257154&action=review > Source/WebKit2/ChangeLog:13 > + This happened because the the fixed layout size was computed using ceil(m_webPage.size().width() / viewScale) Do the scrollbars flicker all weirdly now when resizing a simple page inside the scaling region? >> Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:291 >> + if (!documentWidthChanged && !viewWidthChanged) > > why the check for m_webPage.mainFrame()->view()->needsLayout() was removed? Because smfr added a layoutIfNeeded above, so it will always be true. >> Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:300 >> + if (m_isScalingViewToFitDocument && !documentWidthChanged) { > > Does m_isScalingViewToFitDocument need to be a member of the class? Yes? And it already is. 1110864 5 simon.fraser 2015-07-20 18:58:42 -0700 (In reply to comment #4) > Comment on attachment 257154 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257154&action=review > > > Source/WebKit2/ChangeLog:13 > > + This happened because the the fixed layout size was computed using ceil(m_webPage.size().width() / viewScale) > > Do the scrollbars flicker all weirdly now when resizing a simple page inside > the scaling region? Not any worse than without the patch. > > Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:336 > > + IntSize fixedLayoutSize(documentWidth, std::ceil((m_webPage.size().height() - m_webPage.corePage()->topContentInset()) / viewScale)); > > This block is very similar to the above one, can't the code be refactored > such that the same code is not repeated? I tried to use a lambda and it wasn't clean. 1110865 6 simon.fraser 2015-07-20 19:02:44 -0700 https://trac.webkit.org/r187054 257154 2015-07-20 18:19:30 -0700 2015-07-20 18:45:10 -0700 Patch bug-147134-20150720181817.patch text/plain 5016 simon.fraser U3VidmVyc2lvbiBSZXZpc2lvbjogMTg3MDIzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZmYwN2NhMjlhNjJjYTdm YzA5ODAyN2EyMWUxODFiYzg0NjE1MTcyNy4uMWU4ZGIxMjU4NzE4OTI4NjZlZDdmZTQ0OTExMGRi MWNlOWYzNjY3NSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDE1LTA3LTIwICBTaW1v biBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBwbGUuY29tPgorCisgICAgICAgIEZhY2Vib29rIGlu IHRpbGVkIGZ1bGxzY3JlZW4gaXMgc2xvdworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MTQ3MTM0CisgICAgICAgIHJkYXI6Ly9wcm9ibGVtLzIxODIzMzQ5 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisgICAgICAgIAorICAgICAg ICBUaWxlZENvcmVBbmltYXRpb25EcmF3aW5nQXJlYTo6c2NhbGVWaWV3VG9GaXREb2N1bWVudElm TmVlZGVkKCkgY291bGQgY2F1c2UgYSBwYWdlIHRvIHRvZ2dsZQorICAgICAgICBiZXR3ZWVuIHR3 byBmaXhlZCBsYXlvdXQgc2l6ZXMgZGlmZmVyaW5nIGJ5IGEgcGl4ZWwsIGJlY2F1c2Ugb2Ygcm91 bmRpbmcuIFRoaXMgd291bGQgY2F1c2UKKyAgICAgICAgbG90cyBvZiBleHRyYSBsYXlvdXRzIGFu ZCBwYWludGluZy4KKyAgICAgICAgCisgICAgICAgIFRoaXMgaGFwcGVuZWQgYmVjYXVzZSB0aGUg dGhlIGZpeGVkIGxheW91dCBzaXplIHdhcyBjb21wdXRlZCB1c2luZyBjZWlsKG1fd2ViUGFnZS5z aXplKCkud2lkdGgoKSAvIHZpZXdTY2FsZSkKKworICAgICAgICAqIFdlYlByb2Nlc3MvV2ViUGFn ZS9tYWMvVGlsZWRDb3JlQW5pbWF0aW9uRHJhd2luZ0FyZWEubW06CisgICAgICAgIChXZWJLaXQ6 OlRpbGVkQ29yZUFuaW1hdGlvbkRyYXdpbmdBcmVhOjpzY2FsZVZpZXdUb0ZpdERvY3VtZW50SWZO ZWVkZWQpOgorCiAyMDE1LTA3LTE2ICBBbmRlcnMgQ2FybHNzb24gIDxhbmRlcnNjYUBhcHBsZS5j b20+CiAKICAgICAgICAgV2Vic2l0ZURhdGFTdG9yZSBvcGVyYXRpb25zIG5lZWQgdG8gZ3JhYiBi YWNrZ3JvdW5kIGFzc2VydGlvbnMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nl c3MvV2ViUGFnZS9tYWMvVGlsZWRDb3JlQW5pbWF0aW9uRHJhd2luZ0FyZWEubW0gYi9Tb3VyY2Uv V2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvbWFjL1RpbGVkQ29yZUFuaW1hdGlvbkRyYXdpbmdB cmVhLm1tCmluZGV4IDRhYjczN2U2M2I3NmI2MzBiMzU4OWY0MWYzOWI0Nzc5ODUwOWMxZTAuLjE3 NTk4ZDgwY2FmYWQ0MmMwYTcyZTQxOTc0MTI4OGMxYzkwNjliMDIgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9tYWMvVGlsZWRDb3JlQW5pbWF0aW9uRHJhd2lu Z0FyZWEubW0KKysrIGIvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL21hYy9UaWxl ZENvcmVBbmltYXRpb25EcmF3aW5nQXJlYS5tbQpAQCAtMjgzLDEwICsyODMsMTIgQEAgdm9pZCBU aWxlZENvcmVBbmltYXRpb25EcmF3aW5nQXJlYTo6c2NhbGVWaWV3VG9GaXREb2N1bWVudElmTmVl ZGVkKCkKICAgICBtX3dlYlBhZ2UubGF5b3V0SWZOZWVkZWQoKTsKIAogICAgIGludCB2aWV3V2lk dGggPSBtX3dlYlBhZ2Uuc2l6ZSgpLndpZHRoKCk7Ci0gICAgYm9vbCBkb2N1bWVudFdpZHRoQ2hh bmdlZE9ySW52YWxpZGF0ZWQgPSBtX3dlYlBhZ2UubWFpbkZyYW1lKCktPnZpZXcoKS0+bmVlZHNM YXlvdXQoKSB8fCAobV9sYXN0RG9jdW1lbnRTaXplRm9yU2NhbGVUb0ZpdC53aWR0aCgpICE9IG1f d2ViUGFnZS5tYWluRnJhbWVWaWV3KCktPnJlbmRlclZpZXcoKS0+dW5zY2FsZWREb2N1bWVudFJl Y3QoKS53aWR0aCgpKTsKKyAgICBpbnQgZG9jdW1lbnRXaWR0aCA9IG1fd2ViUGFnZS5tYWluRnJh bWVWaWV3KCktPnJlbmRlclZpZXcoKS0+dW5zY2FsZWREb2N1bWVudFJlY3QoKS53aWR0aCgpOwor CisgICAgYm9vbCBkb2N1bWVudFdpZHRoQ2hhbmdlZCA9IG1fbGFzdERvY3VtZW50U2l6ZUZvclNj YWxlVG9GaXQud2lkdGgoKSAhPSBkb2N1bWVudFdpZHRoOwogICAgIGJvb2wgdmlld1dpZHRoQ2hh bmdlZCA9IG1fbGFzdFZpZXdTaXplRm9yU2NhbGVUb0ZpdC53aWR0aCgpICE9IHZpZXdXaWR0aDsK IAotICAgIGlmICghZG9jdW1lbnRXaWR0aENoYW5nZWRPckludmFsaWRhdGVkICYmICF2aWV3V2lk dGhDaGFuZ2VkKQorICAgIGlmICghZG9jdW1lbnRXaWR0aENoYW5nZWQgJiYgIXZpZXdXaWR0aENo YW5nZWQpCiAgICAgICAgIHJldHVybjsKIAogICAgIC8vIFRoZSB2aWV3IGlzIG5vdyBiaWdnZXIg dGhhbiB0aGUgZG9jdW1lbnQsIHNvIHdlJ2xsIHJlLWV2YWx1YXRlIHdoZXRoZXIgd2UgaGF2ZSB0 byBzY2FsZS4KQEAgLTI5NSwxNCArMjk3LDE2IEBAIHZvaWQgVGlsZWRDb3JlQW5pbWF0aW9uRHJh d2luZ0FyZWE6OnNjYWxlVmlld1RvRml0RG9jdW1lbnRJZk5lZWRlZCgpCiAKICAgICAvLyBPdXIg Y3VycmVudCB1bmRlcnN0YW5kaW5nIG9mIHRoZSBkb2N1bWVudCB3aWR0aCBpcyBzdGlsbCB1cCB0 byBkYXRlLCBhbmQgd2UncmUgaW4gc2NhbGluZyBtb2RlLgogICAgIC8vIFVwZGF0ZSB0aGUgdmll d1NjYWxlIHdpdGhvdXQgZG9pbmcgYW4gZXh0cmEgbGF5b3V0IHRvIHJlLWRldGVybWluZSB0aGUg ZG9jdW1lbnQgd2lkdGguCi0gICAgaWYgKG1faXNTY2FsaW5nVmlld1RvRml0RG9jdW1lbnQgJiYg IWRvY3VtZW50V2lkdGhDaGFuZ2VkT3JJbnZhbGlkYXRlZCkgewotICAgICAgICBmbG9hdCB2aWV3 U2NhbGUgPSAoZmxvYXQpdmlld1dpZHRoIC8gKGZsb2F0KW1fbGFzdERvY3VtZW50U2l6ZUZvclNj YWxlVG9GaXQud2lkdGgoKTsKKyAgICBpZiAobV9pc1NjYWxpbmdWaWV3VG9GaXREb2N1bWVudCAm JiAhZG9jdW1lbnRXaWR0aENoYW5nZWQpIHsKICAgICAgICAgbV9sYXN0Vmlld1NpemVGb3JTY2Fs ZVRvRml0ID0gbV93ZWJQYWdlLnNpemUoKTsKLSAgICAgICAgdmlld1NjYWxlID0gc3RkOjptYXgo dmlld1NjYWxlLCBtaW5pbXVtVmlld1NjYWxlKTsKLSAgICAgICAgbV93ZWJQYWdlLnNjYWxlVmll dyh2aWV3U2NhbGUpOwotCi0gICAgICAgIEludFNpemUgZml4ZWRMYXlvdXRTaXplKHN0ZDo6Y2Vp bChtX3dlYlBhZ2Uuc2l6ZSgpLndpZHRoKCkgLyB2aWV3U2NhbGUpLCBzdGQ6OmNlaWwoKG1fd2Vi UGFnZS5zaXplKCkuaGVpZ2h0KCkgLSBtX3dlYlBhZ2UuY29yZVBhZ2UoKS0+dG9wQ29udGVudElu c2V0KCkpIC8gdmlld1NjYWxlKSk7CisgICAgICAgIGZsb2F0IHZpZXdTY2FsZSA9IChmbG9hdCl2 aWV3V2lkdGggLyAoZmxvYXQpbV9sYXN0RG9jdW1lbnRTaXplRm9yU2NhbGVUb0ZpdC53aWR0aCgp OworICAgICAgICBpZiAodmlld1NjYWxlIDwgbWluaW11bVZpZXdTY2FsZSkgeworICAgICAgICAg ICAgdmlld1NjYWxlID0gbWluaW11bVZpZXdTY2FsZTsKKyAgICAgICAgICAgIGRvY3VtZW50V2lk dGggPSBzdGQ6OmNlaWwodmlld1dpZHRoIC8gdmlld1NjYWxlKTsKKyAgICAgICAgfQorICAgICAg ICBJbnRTaXplIGZpeGVkTGF5b3V0U2l6ZShkb2N1bWVudFdpZHRoLCBzdGQ6OmNlaWwoKG1fd2Vi UGFnZS5zaXplKCkuaGVpZ2h0KCkgLSBtX3dlYlBhZ2UuY29yZVBhZ2UoKS0+dG9wQ29udGVudElu c2V0KCkpIC8gdmlld1NjYWxlKSk7CiAgICAgICAgIG1fd2ViUGFnZS5zZXRGaXhlZExheW91dFNp emUoZml4ZWRMYXlvdXRTaXplKTsKKyAgICAgICAgbV93ZWJQYWdlLnNjYWxlVmlldyh2aWV3U2Nh bGUpOwogICAgICAgICByZXR1cm47CiAgICAgfQogCkBAIC0zMTQsNyArMzE4LDcgQEAgdm9pZCBU aWxlZENvcmVBbmltYXRpb25EcmF3aW5nQXJlYTo6c2NhbGVWaWV3VG9GaXREb2N1bWVudElmTmVl ZGVkKCkKICAgICBtX2xhc3RWaWV3U2l6ZUZvclNjYWxlVG9GaXQgPSBtX3dlYlBhZ2Uuc2l6ZSgp OwogICAgIG1fbGFzdERvY3VtZW50U2l6ZUZvclNjYWxlVG9GaXQgPSBkb2N1bWVudFNpemU7CiAK LSAgICBpbnQgZG9jdW1lbnRXaWR0aCA9IGRvY3VtZW50U2l6ZS53aWR0aCgpOworICAgIGRvY3Vt ZW50V2lkdGggPSBkb2N1bWVudFNpemUud2lkdGgoKTsKIAogICAgIGZsb2F0IHZpZXdTY2FsZSA9 IDE7CiAKQEAgLTMyNSw4ICszMjksMTEgQEAgdm9pZCBUaWxlZENvcmVBbmltYXRpb25EcmF3aW5n QXJlYTo6c2NhbGVWaWV3VG9GaXREb2N1bWVudElmTmVlZGVkKCkKICAgICAgICAgbV9pc1NjYWxp bmdWaWV3VG9GaXREb2N1bWVudCA9IHRydWU7CiAgICAgICAgIG1fd2ViUGFnZS5zZXRVc2VGaXhl ZExheW91dCh0cnVlKTsKICAgICAgICAgdmlld1NjYWxlID0gKGZsb2F0KXZpZXdXaWR0aCAvIChm bG9hdClkb2N1bWVudFdpZHRoOwotICAgICAgICB2aWV3U2NhbGUgPSBzdGQ6Om1heCh2aWV3U2Nh bGUsIG1pbmltdW1WaWV3U2NhbGUpOwotICAgICAgICBJbnRTaXplIGZpeGVkTGF5b3V0U2l6ZShz dGQ6OmNlaWwobV93ZWJQYWdlLnNpemUoKS53aWR0aCgpIC8gdmlld1NjYWxlKSwgc3RkOjpjZWls KChtX3dlYlBhZ2Uuc2l6ZSgpLmhlaWdodCgpIC0gbV93ZWJQYWdlLmNvcmVQYWdlKCktPnRvcENv bnRlbnRJbnNldCgpKSAvIHZpZXdTY2FsZSkpOworICAgICAgICBpZiAodmlld1NjYWxlIDwgbWlu aW11bVZpZXdTY2FsZSkgeworICAgICAgICAgICAgdmlld1NjYWxlID0gbWluaW11bVZpZXdTY2Fs ZTsKKyAgICAgICAgICAgIGRvY3VtZW50V2lkdGggPSBzdGQ6OmNlaWwodmlld1dpZHRoIC8gdmll d1NjYWxlKTsKKyAgICAgICAgfQorICAgICAgICBJbnRTaXplIGZpeGVkTGF5b3V0U2l6ZShkb2N1 bWVudFdpZHRoLCBzdGQ6OmNlaWwoKG1fd2ViUGFnZS5zaXplKCkuaGVpZ2h0KCkgLSBtX3dlYlBh Z2UuY29yZVBhZ2UoKS0+dG9wQ29udGVudEluc2V0KCkpIC8gdmlld1NjYWxlKSk7CiAgICAgICAg IG1fd2ViUGFnZS5zZXRGaXhlZExheW91dFNpemUoZml4ZWRMYXlvdXRTaXplKTsKICAgICB9CiAK