139062 2014-11-26 06:26:11 -0800 [GTK] Timers might never be fired during animations 2014-12-11 03:21:57 -0800 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Gtk P2 Normal --- 1 cgarcia webkit-unassigned gustavo pnormand svillar zan oldest_to_newest 1051084 0 cgarcia 2014-11-26 06:26:11 -0800 This can happen in old/slow machines where the time to render layers might take more than 0.016. Since the layer flush timer is using a higher priority than WebCore timers, when scheduling all (or several) layer flushes immediately, no other sources with lower priority are dispatched in the main loop. I could reproduce this in an old machine running the outlook web application that uses a spinner animation during the load. The spinner starts very early and the application adds an even listener for the load event that is never emitted, because fonts are loaded on demand by the CSS font selector using a timer that is never fired. 1051085 1 242226 cgarcia 2014-11-26 06:34:37 -0800 Created attachment 242226 Patch 1054448 2 242226 mrobinson 2014-12-11 02:35:24 -0800 Comment on attachment 242226 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242226&action=review Looks good to me. I have a couple readability suggestions, but I think the general approach is just fine. Though please double-check that this doesn't harm animations while resizing on faster machines. Thanks! > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:66 > +// Use a higher priority than WebCore timers. > +static const int layerFlushTimerPriority = GDK_PRIORITY_REDRAW - 1; > + Maybe just move this down to right before it is used? > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:264 > + static const double targetDelay = 1 / 60.0; I would call this targetFramerate or something like that. > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:270 > + static const double maxDurationImmediateFlush = 0.100; Maybe call this time maxDurationOfImmediateFlushes. > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:272 > + double nextFlush = std::max(targetDelay - (current - fireTime), 0.0); I would call this timeToNextFlush. > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:281 > + if (m_lastImmediateFlushTime && m_lastImmediateFlushTime + maxDurationImmediateFlush < current) { > + nextFlush = targetDelay; > + m_lastImmediateFlushTime = 0; > } Would it be possible to move this to a helper called shouldSkipAFrameBecauseOfContinousImmediateFlushes? I know the name is super silly, but I think it would help the readability of the code here. 1054452 3 cgarcia 2014-12-11 03:21:57 -0800 Committed r177144: <http://trac.webkit.org/changeset/177144> 242226 2014-11-26 06:34:37 -0800 2014-12-11 02:35:24 -0800 Patch wk2-layer-flush.diff text/plain 6963 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBmZGU2NmU4Li42MGY4NzczIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMzcg QEAKKzIwMTQtMTEtMjYgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgorCisgICAgICAgIFtHVEtdIFRpbWVycyBtaWdodCBuZXZlciBiZSBmaXJlZCBkdXJpbmcgYW5p bWF0aW9ucworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTM5MDYyCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg VGhpcyBjYW4gaGFwcGVuIGluIG9sZC9zbG93IG1hY2hpbmVzIHdoZXJlIHRoZSB0aW1lIHRvIHJl bmRlcgorICAgICAgICBsYXllcnMgbWlnaHQgdGFrZSBtb3JlIHRoYW4gMC4wMTYuIFNpbmNlIHRo ZSBsYXllciBmbHVzaCB0aW1lciBpcworICAgICAgICB1c2luZyBhIGhpZ2hlciBwcmlvcml0eSB0 aGFuIFdlYkNvcmUgdGltZXJzLCB3aGVuIHNjaGVkdWxpbmcgYWxsCisgICAgICAgIChvciBzZXZl cmFsKSBsYXllciBmbHVzaGVzIGltbWVkaWF0ZWx5LCBubyBvdGhlciBzb3VyY2VzIHdpdGgKKyAg ICAgICAgbG93ZXIgcHJpb3JpdHkgYXJlIGRpc3BhdGNoZWQgaW4gdGhlIG1haW4gbG9vcC4gV2Ug Y291bGQgZGV0ZWN0IGlmCisgICAgICAgIHdlIGFyZSBzY2hlZHVsaW5nIGxheWVyIGZsdXNoZXMg aW1tZWRpYXRlbHkgZm9yIHRvbyBsb25nICgxMDBtcykKKyAgICAgICAgYW5kIHNjaGVkdWxlIHRo ZSBuZXh0IGZsdXNoIGFmdGVyIGEgZGVsYXkgdG8gZW5zdXJlIG90aGVyIHNvdXJjZXMKKyAgICAg ICAgd2l0aCBsb3dlciBwcmlvcml0eSBoYXZlIGEgY2hhbmNlIHRvIGJlIGRpc3BhdGNoZWQuIEFs c28gdXNlIGEKKyAgICAgICAgbG93ZXIgcHJpb3JpdHksIEdES19QUklPUklUWV9FVkVOVFMgaXMg dG9vIGhpZ2gsIHNvIHdlIHVzZQorICAgICAgICBHREtfUFJJT1JJVFlfUkVEUkFXIC0gMSB0byBl bnN1cmUgaXQncyBoaWdoZXIgdGhhbiBXZWJDb3JlIHRpbWVycy4KKworICAgICAgICAqIFdlYlBy b2Nlc3MvV2ViUGFnZS9ndGsvTGF5ZXJUcmVlSG9zdEd0ay5jcHA6CisgICAgICAgIChXZWJLaXQ6 OkxheWVyVHJlZUhvc3RHdGs6OkxheWVyVHJlZUhvc3RHdGspOiBSZW5hbWUKKyAgICAgICAgbV9s YXN0Rmx1c2hUaW1lIGFzIG1fbGFzdEltbWVkaWF0ZUZsdXNoVGltZS4KKyAgICAgICAgKFdlYktp dDo6TGF5ZXJUcmVlSG9zdEd0azo6bGF5ZXJGbHVzaFRpbWVyRmlyZWQpOiBTYXZlIHRoZQorICAg ICAgICBmaXJlVGltZSBiZWZvcmUgY2FsbGluZyBmbHVzaEFuZFJlbmRlckxheWVycygpIGFuZCBj b21wdXRlIHRoZQorICAgICAgICBuZXh0IGZsdXNoIGRlbGF5IGJhc2VkIG9uIHRoZSBlbGFwc2Vk IHRpbWUgdXNpbmcgbW9ub3RvbmljYWxseQorICAgICAgICBpbmNyZWFzaW5nIHRpbWUgaW5zdGVh ZCBvZiBjdXJyZW50IHRpbWUuIFVzZSB0aGUgdGFyZ2V0IGRlbGF5CisgICAgICAgIGFzIG5leHQg Zmx1c2ggZGVsYXkgaWYgd2UgaGF2ZSBzY2hlZHVsZWQgZmx1c2hlcyBpbW1lZGlhdGVseSBmb3IK KyAgICAgICAgbW9yZSB0aGFuIDEwMG1zLgorICAgICAgICAoV2ViS2l0OjpMYXllclRyZWVIb3N0 R3RrOjpmbHVzaEFuZFJlbmRlckxheWVycyk6IERvIG5vdCBzYXZlIHRoZQorICAgICAgICBsYXll ciBmbHVzaCB0aW1lIGhlcmUsIGl0J3MgZG9uZSBpbiBsYXllckZsdXNoVGltZXJGaXJlZCgpIHNv IHRoYXQKKyAgICAgICAgd2UgZG9uJ3QgbmVlZCB0byBrZWVwIGl0IGFzIGEgbWVtYmVyLgorICAg ICAgICAoV2ViS2l0OjpMYXllclRyZWVIb3N0R3RrOjpzY2hlZHVsZUxheWVyRmx1c2gpOiBVc2Ug Z2xvYmFsCisgICAgICAgIGxheWVyRmx1c2hUaW1lclByaW9yaXR5LgorICAgICAgICAqIFdlYlBy b2Nlc3MvV2ViUGFnZS9ndGsvTGF5ZXJUcmVlSG9zdEd0ay5oOgorCiAyMDE0LTExLTI0ICBDYXJs b3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNvbT4KIAogICAgICAgICBVbnJldmll d2VkLiBVcGRhdGUgT3B0aW9uc0dUSy5jbWFrZSBhbmQgTkVXUyBmb3IgMi43LjIgcmVsZWFzZS4K ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9ndGsvTGF5ZXJU cmVlSG9zdEd0ay5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvZ3RrL0xh eWVyVHJlZUhvc3RHdGsuY3BwCmluZGV4IGVhYTYzZTEuLmM0ZTZhODggMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9ndGsvTGF5ZXJUcmVlSG9zdEd0ay5jcHAK KysrIGIvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL2d0ay9MYXllclRyZWVIb3N0 R3RrLmNwcApAQCAtNjEsNiArNjEsOSBAQCB1c2luZyBuYW1lc3BhY2UgV2ViQ29yZTsKIAogbmFt ZXNwYWNlIFdlYktpdCB7CiAKKy8vIFVzZSBhIGhpZ2hlciBwcmlvcml0eSB0aGFuIFdlYkNvcmUg dGltZXJzLgorc3RhdGljIGNvbnN0IGludCBsYXllckZsdXNoVGltZXJQcmlvcml0eSA9IEdES19Q UklPUklUWV9SRURSQVcgLSAxOworCiBQYXNzUmVmUHRyPExheWVyVHJlZUhvc3RHdGs+IExheWVy VHJlZUhvc3RHdGs6OmNyZWF0ZShXZWJQYWdlKiB3ZWJQYWdlKQogewogICAgIFJlZlB0cjxMYXll clRyZWVIb3N0R3RrPiBob3N0ID0gYWRvcHRSZWYobmV3IExheWVyVHJlZUhvc3RHdGsod2ViUGFn ZSkpOwpAQCAtNzIsNyArNzUsNyBAQCBMYXllclRyZWVIb3N0R3RrOjpMYXllclRyZWVIb3N0R3Rr KFdlYlBhZ2UqIHdlYlBhZ2UpCiAgICAgOiBMYXllclRyZWVIb3N0KHdlYlBhZ2UpCiAgICAgLCBt X2lzVmFsaWQodHJ1ZSkKICAgICAsIG1fbm90aWZ5QWZ0ZXJTY2hlZHVsZWRMYXllckZsdXNoKGZh bHNlKQotICAgICwgbV9sYXN0Rmx1c2hUaW1lKDApCisgICAgLCBtX2xhc3RJbW1lZGlhdGVGbHVz aFRpbWUoMCkKICAgICAsIG1fbGF5ZXJGbHVzaFNjaGVkdWxpbmdFbmFibGVkKHRydWUpCiB7CiB9 CkBAIC0yNTMsMTQgKzI1NiwzMiBAQCB2b2lkIExheWVyVHJlZUhvc3RHdGs6OnBhaW50Q29udGVu dHMoY29uc3QgR3JhcGhpY3NMYXllciogZ3JhcGhpY3NMYXllciwgR3JhcGhpYwogCiB2b2lkIExh eWVyVHJlZUhvc3RHdGs6OmxheWVyRmx1c2hUaW1lckZpcmVkKCkKIHsKKyAgICBkb3VibGUgZmly ZVRpbWUgPSBtb25vdG9uaWNhbGx5SW5jcmVhc2luZ1RpbWUoKTsKICAgICBmbHVzaEFuZFJlbmRl ckxheWVycygpOworICAgIGlmIChtX2xheWVyRmx1c2hUaW1lckNhbGxiYWNrLmlzU2NoZWR1bGVk KCkgfHwgIWRvd25jYXN0PEdyYXBoaWNzTGF5ZXJUZXh0dXJlTWFwcGVyPigqbV9yb290TGF5ZXIp LmxheWVyKCktPmRlc2NlbmRhbnRzT3JTZWxmSGF2ZVJ1bm5pbmdBbmltYXRpb25zKCkpCisgICAg ICAgIHJldHVybjsKIAotICAgIGlmICghbV9sYXllckZsdXNoVGltZXJDYWxsYmFjay5pc1NjaGVk dWxlZCgpICYmIGRvd25jYXN0PEdyYXBoaWNzTGF5ZXJUZXh0dXJlTWFwcGVyPigqbV9yb290TGF5 ZXIpLmxheWVyKCktPmRlc2NlbmRhbnRzT3JTZWxmSGF2ZVJ1bm5pbmdBbmltYXRpb25zKCkpIHsK LSAgICAgICAgY29uc3QgZG91YmxlIHRhcmdldEZQUyA9IDYwOwotICAgICAgICBkb3VibGUgbmV4 dEZsdXNoID0gc3RkOjptYXgoKDEgLyB0YXJnZXRGUFMpIC0gKGN1cnJlbnRUaW1lKCkgLSBtX2xh c3RGbHVzaFRpbWUpLCAwLjApOwotICAgICAgICBtX2xheWVyRmx1c2hUaW1lckNhbGxiYWNrLnNj aGVkdWxlQWZ0ZXJEZWxheSgiW1dlYktpdF0gbGF5ZXJGbHVzaFRpbWVyIiwgc3RkOjpiaW5kKCZM YXllclRyZWVIb3N0R3RrOjpsYXllckZsdXNoVGltZXJGaXJlZCwgdGhpcyksCi0gICAgICAgICAg ICBzdGQ6OmNocm9ubzo6ZHVyYXRpb25fY2FzdDxzdGQ6OmNocm9ubzo6bWljcm9zZWNvbmRzPihz dGQ6OmNocm9ubzo6ZHVyYXRpb248ZG91YmxlPihuZXh0Rmx1c2gpKSwgR0RLX1BSSU9SSVRZX0VW RU5UUyk7CisgICAgc3RhdGljIGNvbnN0IGRvdWJsZSB0YXJnZXREZWxheSA9IDEgLyA2MC4wOwor ICAgIC8vIFdoZW4gcmVuZGVyaW5nIGxheWVycyB0YWtlcyBtb3JlIHRpbWUgdGhhbiB0aGUgdGFy Z2V0IGRlbGF5ICgwLjAxNiksIHdlIGVuZCB1cCBzY2hlZHVsaW5nIGxheWVyIGZsdXNoZXMKKyAg ICAvLyBpbW1lZGlhdGVseS4gU2luY2UgdGhlIGxheWVyIGZsdXNoIHRpbWVyIGhhcyBhIGhpZ2hl ciBwcmlvcml0eSB0aGFuIFdlYkNvcmUgdGltZXJzLCB0aGVzZSBhcmUgbmV2ZXIKKyAgICAvLyBm aXJlZCB3aGlsZSB3ZSBrZWVwIHNjaGVkdWxpbmcgbGF5ZXIgZmx1c2hlcyBpbW1lZGlhdGVseS4g MTAwbXMgaXMgYWJvdXQgYSBwZXJjZXB0YWJsZSBkZWxheSBpbiBVSSwgc28KKyAgICAvLyB3aGVu IHNjaGVkdWxpbmcgbGF5ZXIgZmx1c2hlcyBpbW1lZGlhdGVseSBmb3IgbW9yZSB0aGFuIDEwMG1z LCB3ZSB1c2UgdGhlIHRhcmdldCBkZWxheSBmb3IgdGhlIG5leHQgc2NoZWR1bGUKKyAgICAvLyB0 byBlbnN1cmUgcGVuZGluZyB0aW1lcnMgaGF2ZSBhIGNoYW5nZSB0byBiZSBmaXJlZC4KKyAgICBz dGF0aWMgY29uc3QgZG91YmxlIG1heER1cmF0aW9uSW1tZWRpYXRlRmx1c2ggPSAwLjEwMDsKKyAg ICBkb3VibGUgY3VycmVudCA9IG1vbm90b25pY2FsbHlJbmNyZWFzaW5nVGltZSgpOworICAgIGRv dWJsZSBuZXh0Rmx1c2ggPSBzdGQ6Om1heCh0YXJnZXREZWxheSAtIChjdXJyZW50IC0gZmlyZVRp bWUpLCAwLjApOworICAgIGlmIChuZXh0Rmx1c2gpCisgICAgICAgIG1fbGFzdEltbWVkaWF0ZUZs dXNoVGltZSA9IDA7CisgICAgZWxzZSBpZiAoIW1fbGFzdEltbWVkaWF0ZUZsdXNoVGltZSkKKyAg ICAgICAgbV9sYXN0SW1tZWRpYXRlRmx1c2hUaW1lID0gY3VycmVudDsKKworICAgIGlmIChtX2xh c3RJbW1lZGlhdGVGbHVzaFRpbWUgJiYgbV9sYXN0SW1tZWRpYXRlRmx1c2hUaW1lICsgbWF4RHVy YXRpb25JbW1lZGlhdGVGbHVzaCA8IGN1cnJlbnQpIHsKKyAgICAgICAgbmV4dEZsdXNoID0gdGFy Z2V0RGVsYXk7CisgICAgICAgIG1fbGFzdEltbWVkaWF0ZUZsdXNoVGltZSA9IDA7CiAgICAgfQor CisgICAgbV9sYXllckZsdXNoVGltZXJDYWxsYmFjay5zY2hlZHVsZUFmdGVyRGVsYXkoIltXZWJL aXRdIGxheWVyRmx1c2hUaW1lciIsIHN0ZDo6YmluZCgmTGF5ZXJUcmVlSG9zdEd0azo6bGF5ZXJG bHVzaFRpbWVyRmlyZWQsIHRoaXMpLAorICAgICAgICBzdGQ6OmNocm9ubzo6ZHVyYXRpb25fY2Fz dDxzdGQ6OmNocm9ubzo6bWljcm9zZWNvbmRzPihzdGQ6OmNocm9ubzo6ZHVyYXRpb248ZG91Ymxl PihuZXh0Rmx1c2gpKSwgbGF5ZXJGbHVzaFRpbWVyUHJpb3JpdHkpOwogfQogCiBib29sIExheWVy VHJlZUhvc3RHdGs6OmZsdXNoUGVuZGluZ0xheWVyQ2hhbmdlcygpCkBAIC0zMTMsNyArMzM0LDYg QEAgdm9pZCBMYXllclRyZWVIb3N0R3RrOjpmbHVzaEFuZFJlbmRlckxheWVycygpCiAgICAgaWYg KCFjb250ZXh0IHx8ICFjb250ZXh0LT5tYWtlQ29udGV4dEN1cnJlbnQoKSkKICAgICAgICAgcmV0 dXJuOwogCi0gICAgbV9sYXN0Rmx1c2hUaW1lID0gY3VycmVudFRpbWUoKTsKICAgICBpZiAoIWZs dXNoUGVuZGluZ0xheWVyQ2hhbmdlcygpKQogICAgICAgICByZXR1cm47CiAKQEAgLTM1OSw3ICsz NzksNyBAQCB2b2lkIExheWVyVHJlZUhvc3RHdGs6OnNjaGVkdWxlTGF5ZXJGbHVzaCgpCiAKICAg ICAvLyBXZSB1c2UgYSBHTGliIHRpbWVyIGJlY2F1c2Ugb3RoZXJ3aXNlIEdUSysgZXZlbnQgaGFu ZGxpbmcgZHVyaW5nIGRyYWdnaW5nIGNhbiBzdGFydmUgV2ViQ29yZSB0aW1lcnMsIHdoaWNoIGhh dmUgYSBsb3dlciBwcmlvcml0eS4KICAgICBpZiAoIW1fbGF5ZXJGbHVzaFRpbWVyQ2FsbGJhY2su aXNTY2hlZHVsZWQoKSkKLSAgICAgICAgbV9sYXllckZsdXNoVGltZXJDYWxsYmFjay5zY2hlZHVs ZSgiW1dlYktpdF0gbGF5ZXJGbHVzaFRpbWVyIiwgc3RkOjpiaW5kKCZMYXllclRyZWVIb3N0R3Rr OjpsYXllckZsdXNoVGltZXJGaXJlZCwgdGhpcyksIEdES19QUklPUklUWV9FVkVOVFMpOworICAg ICAgICBtX2xheWVyRmx1c2hUaW1lckNhbGxiYWNrLnNjaGVkdWxlKCJbV2ViS2l0XSBsYXllckZs dXNoVGltZXIiLCBzdGQ6OmJpbmQoJkxheWVyVHJlZUhvc3RHdGs6OmxheWVyRmx1c2hUaW1lckZp cmVkLCB0aGlzKSwgbGF5ZXJGbHVzaFRpbWVyUHJpb3JpdHkpOwogfQogCiB2b2lkIExheWVyVHJl ZUhvc3RHdGs6OnNldExheWVyRmx1c2hTY2hlZHVsaW5nRW5hYmxlZChib29sIGxheWVyRmx1c2hp bmdFbmFibGVkKQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdl L2d0ay9MYXllclRyZWVIb3N0R3RrLmggYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBh Z2UvZ3RrL0xheWVyVHJlZUhvc3RHdGsuaAppbmRleCBkOTM3MGMyLi5hNzZlZWYwIDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvZ3RrL0xheWVyVHJlZUhvc3RH dGsuaAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvZ3RrL0xheWVyVHJl ZUhvc3RHdGsuaApAQCAtMTAwLDcgKzEwMCw3IEBAIHByaXZhdGU6CiAgICAgUGFnZU92ZXJsYXlM YXllck1hcCBtX3BhZ2VPdmVybGF5TGF5ZXJzOwogICAgIHN0ZDo6dW5pcXVlX3B0cjxXZWJDb3Jl OjpUZXh0dXJlTWFwcGVyPiBtX3RleHR1cmVNYXBwZXI7CiAgICAgT3duUHRyPFdlYkNvcmU6OkdM Q29udGV4dD4gbV9jb250ZXh0OwotICAgIGRvdWJsZSBtX2xhc3RGbHVzaFRpbWU7CisgICAgZG91 YmxlIG1fbGFzdEltbWVkaWF0ZUZsdXNoVGltZTsKICAgICBib29sIG1fbGF5ZXJGbHVzaFNjaGVk dWxpbmdFbmFibGVkOwogICAgIEdNYWluTG9vcFNvdXJjZSBtX2xheWVyRmx1c2hUaW1lckNhbGxi YWNrOwogfTsK