Bug 71354 - Resizing Cappuccino is very laggy on WebKit since Safari 5.1
Summary: Resizing Cappuccino is very laggy on WebKit since Safari 5.1
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://githubissues.heroku.com/
Keywords: InRadar
Depends on: 74674 111815
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-02 03:36 PDT by Antoine Mercadal
Modified: 2013-03-08 09:48 PST (History)
7 users (show)

See Also:


Attachments
Testcase (212 bytes, text/html)
2011-12-12 11:40 PST, Simon Fraser (smfr)
no flags Details
Proposed patch (14.62 KB, patch)
2011-12-12 17:23 PST, Andreas Kling
no flags Details | Formatted Diff | Diff
Patch II: Return from the Dead (16.38 KB, patch)
2013-03-07 18:01 PST, Andreas Kling
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Mercadal 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/
Comment 1 Sam Weinig 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.
Comment 2 Antoine Mercadal 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
Comment 3 Radar WebKit Bug Importer 2011-12-12 11:19:46 PST
<rdar://problem/10565998>
Comment 4 Simon Fraser (smfr) 2011-12-12 11:40:16 PST
Created attachment 118820 [details]
Testcase
Comment 5 Andreas Kling 2011-12-12 17:23:03 PST
Created attachment 118921 [details]
Proposed patch
Comment 6 Alexey Proskuryakov 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.
Comment 7 Andreas Kling 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.
Comment 8 Anders Carlsson 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.
Comment 9 Andreas Kling 2011-12-12 19:08:31 PST
Committed r102652: <http://trac.webkit.org/changeset/102652>
Comment 10 Alexey Proskuryakov 2011-12-13 10:07:26 PST
It seems that the caused bug 74418.
Comment 11 Alexey Proskuryakov 2011-12-15 17:04:43 PST
It seems that this also caused bug 74616.
Comment 12 Alexey Proskuryakov 2011-12-15 19:10:50 PST
This got reverted in <http://trac.webkit.org/changeset/103009>, reopening.
Comment 13 Andreas Kling 2013-03-07 18:01:21 PST
Created attachment 192119 [details]
Patch II: Return from the Dead

Let's fix it properly this time.
Comment 14 Anders Carlsson 2013-03-07 18:03:15 PST
Comment on attachment 192119 [details]
Patch II: Return from the Dead

Is gud poch.
Comment 15 Andreas Kling 2013-03-07 18:26:53 PST
Committed r145169: <http://trac.webkit.org/changeset/145169>
Comment 16 Ryosuke Niwa 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.