RESOLVED FIXED 71354
Resizing Cappuccino is very laggy on WebKit since Safari 5.1
https://bugs.webkit.org/show_bug.cgi?id=71354
Summary Resizing Cappuccino is very laggy on WebKit since Safari 5.1
Antoine Mercadal
Reported 2011-11-02 03:36:44 PDT
All Cappuccino applications are suffering from a very laggy behavior when resizing the browser. This happens in all Capp apps, even the smaller ones, with barely nothing. This doesn't happen in Chrome/Chromium (latest) or in Firefox It's OK that the actual Cappuccino content doesn't resize very smoothly as there is sometimes a lot of computing to do, but at least, it shouldn't make the browser's window resizing action to be so laggy. You can have an example here http://githubissues.heroku.com/
Attachments
Testcase (212 bytes, text/html)
2011-12-12 11:40 PST, Simon Fraser (smfr)
no flags
Proposed patch (14.62 KB, patch)
2011-12-12 17:23 PST, Andreas Kling
no flags
Patch II: Return from the Dead (16.38 KB, patch)
2013-03-07 18:01 PST, Andreas Kling
andersca: review+
Sam Weinig
Comment 1 2011-11-03 09:53:16 PDT
Resizing is pretty bad in both WebKit1 and WebKit2 windows, but does seem a bit worse in WebKit2. I believe this is due to weird interaction between onresize handlers and our resize model.
Antoine Mercadal
Comment 2 2011-12-12 04:01:16 PST
I've worked on this issue a little bit more, it's not just Cappuccino specific. The problem seems to be related when asking from window.screenX and screenY in the same resize event hander. For instance this dirty fragment of code will hang when resizing: <html> <script> (function(){ function onresize() { console.log(window.screenX); console.log(window.screenY); } window.addEventListener("resize", onresize); })() </script> </html> If you comment one of the two log statement, it will work smoother. I tried to add other log for innerWidth, innerHeight, screenTop, screenLeft, they don't cause any trouble. Hope this helps
Radar WebKit Bug Importer
Comment 3 2011-12-12 11:19:46 PST
Simon Fraser (smfr)
Comment 4 2011-12-12 11:40:16 PST
Created attachment 118820 [details] Testcase
Andreas Kling
Comment 5 2011-12-12 17:23:03 PST
Created attachment 118921 [details] Proposed patch
Alexey Proskuryakov
Comment 6 2011-12-12 17:38:11 PST
Comment on attachment 118921 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=118921&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:110 > +#if PLATFORM(MAC) > + return m_page->windowFrameInScreenCoordinates(); > +#else Will a potentially stale value be used here as a result of this change? I'm not even sure if any WindowAndViewFramesChanged will be delivered until after the end of resizing.
Andreas Kling
Comment 7 2011-12-12 17:49:27 PST
(In reply to comment #6) > (From update of attachment 118921 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=118921&action=review > > > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:110 > > +#if PLATFORM(MAC) > > + return m_page->windowFrameInScreenCoordinates(); > > +#else > > Will a potentially stale value be used here as a result of this change? I'm not even sure if any WindowAndViewFramesChanged will be delivered until after the end of resizing. What do you mean by the end of resizing? When you let go of the window edge? On my (Lion) system, there's a steady stream of WindowAndViewFramesChanged messages during resize. And this change does indeed make <http://githubissues.heroku.com/> significantly less laggy.
Anders Carlsson
Comment 8 2011-12-12 17:52:35 PST
Comment on attachment 118921 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=118921&action=review >>> Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:110 >>> +#else >> >> Will a potentially stale value be used here as a result of this change? I'm not even sure if any WindowAndViewFramesChanged will be delivered until after the end of resizing. > > What do you mean by the end of resizing? When you let go of the window edge? On my (Lion) system, there's a steady stream of WindowAndViewFramesChanged messages during resize. And this change does indeed make <http://githubissues.heroku.com/> significantly less laggy. I think it's absolutely fine to return a stale value here - the window between resizing and the size being updated is small enough that it shouldn't matter in practice. I think Chrome does something similar.
Andreas Kling
Comment 9 2011-12-12 19:08:31 PST
Alexey Proskuryakov
Comment 10 2011-12-13 10:07:26 PST
It seems that the caused bug 74418.
Alexey Proskuryakov
Comment 11 2011-12-15 17:04:43 PST
It seems that this also caused bug 74616.
Alexey Proskuryakov
Comment 12 2011-12-15 19:10:50 PST
This got reverted in <http://trac.webkit.org/changeset/103009>, reopening.
Andreas Kling
Comment 13 2013-03-07 18:01:21 PST
Created attachment 192119 [details] Patch II: Return from the Dead Let's fix it properly this time.
Anders Carlsson
Comment 14 2013-03-07 18:03:15 PST
Comment on attachment 192119 [details] Patch II: Return from the Dead Is gud poch.
Andreas Kling
Comment 15 2013-03-07 18:26:53 PST
Ryosuke Niwa
Comment 16 2013-03-07 20:28:30 PST
This patch appears to have regressed http/tests/security/cross-frame-access-put.html: --- /Volumes/Data/slave/mountainlion-release-tests-wk2/build/layout-test-results/http/tests/security/cross-frame-access-put-expected.txt +++ /Volumes/Data/slave/mountainlion-release-tests-wk2/build/layout-test-results/http/tests/security/cross-frame-access-put-actual.txt @@ -520,9 +520,9 @@ ALERT: PASS: window.personalbar should be '[object BarInfo]' and is. ALERT: PASS: window.plugins should be 'undefined' and is. ALERT: PASS: window.screen should be '[object Screen]' and is. -ALERT: PASS: window.screenLeft should be '0' and is. +ALERT: PASS: window.screenLeft should be '-10000' and is. ALERT: PASS: window.screenTop matched the expected value. -ALERT: PASS: window.screenX should be '0' and is. +ALERT: PASS: window.screenX should be '-10000' and is. ALERT: PASS: window.screenY matched the expected value. ALERT: PASS: window.scrollbars should be '[object BarInfo]' and is. ALERT: PASS: window.scrollX should be '0' and is.
Note You need to log in before you can comment on or make changes to this bug.