Bug 83954 - REGRESSION(110072): Clipping is not applied on layers that are animated using platform code
Summary: REGRESSION(110072): Clipping is not applied on layers that are animated using...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Julien Chaffraix
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-13 16:00 PDT by Alexandru Chiculita
Modified: 2012-05-02 14:46 PDT (History)
9 users (show)

See Also:


Attachments
Simple test case (417 bytes, text/html)
2012-04-13 16:01 PDT, Alexandru Chiculita
no flags Details
WIP patch, the logic is not bullet-proof enough as-is. (15.54 KB, patch)
2012-04-17 18:39 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Better fix 2: Keep track of whether we have skipped creating a RenderLayer and create them when needed. (15.50 KB, patch)
2012-04-23 17:26 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
rollout 110072 take 1. (83.70 KB, patch)
2012-04-27 15:08 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
rollout 110072 take 2. (83.59 KB, patch)
2012-04-30 09:11 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-03 (6.12 MB, application/zip)
2012-04-30 10:38 PDT, WebKit Review Bot
no flags Details
rollout 110072 take 3. Should be fine now. (85.02 KB, patch)
2012-04-30 17:52 PDT, Julien Chaffraix
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandru Chiculita 2012-04-13 16:00:37 PDT
The overflow:hidden is only creating a layer if there's any child that overflows the clipping box, but when the animations run in hardware there's no layout code that checks that anymore. See the attached test case.
Comment 1 Alexandru Chiculita 2012-04-13 16:01:38 PDT
Created attachment 137170 [details]
Simple test case

There should be no green outside the red box.
Comment 2 Julien Chaffraix 2012-04-17 18:39:14 PDT
Created attachment 137647 [details]
WIP patch, the logic is not bullet-proof enough as-is.
Comment 3 Julien Chaffraix 2012-04-23 17:26:29 PDT
Created attachment 138469 [details]
Better fix 2: Keep track of whether we have skipped creating a RenderLayer and create them when needed.
Comment 4 Simon Fraser (smfr) 2012-04-23 17:28:31 PDT
Seems related to bug 81989.
Comment 5 Julien Chaffraix 2012-04-27 10:49:43 PDT
Comment on attachment 138469 [details]
Better fix 2: Keep track of whether we have skipped creating a RenderLayer and create them when needed.

Removing the review flag on this patch. I will post a patch rolling out r110072 and some of the follow-ups instead as it came under my attention that it caused other regressions.
Comment 6 Julien Chaffraix 2012-04-27 15:08:18 PDT
Created attachment 139286 [details]
rollout 110072 take 1.
Comment 7 Julien Chaffraix 2012-04-30 09:11:02 PDT
Created attachment 139460 [details]
rollout 110072 take 2.
Comment 8 WebKit Review Bot 2012-04-30 09:14:54 PDT
Attachment 139460 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
LayoutTests/platform/gtk/test_expectations.txt:512:  More specific entry on line 325 overrides line 512 fast/workers/storage/use-same-database-in-page-and-workers.html  [test/expectations] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 WebKit Review Bot 2012-04-30 10:38:53 PDT
Comment on attachment 139460 [details]
rollout 110072 take 2.

Attachment 139460 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12582344

New failing tests:
http/tests/navigation/javascriptlink-frames.html
tables/mozilla_expected_failures/bugs/bug2479-5.html
Comment 10 WebKit Review Bot 2012-04-30 10:38:59 PDT
Created attachment 139473 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 11 Julien Chaffraix 2012-04-30 17:51:31 PDT
CC'ing some bot maintainer so that they see the tide coming as the next roll-out should be good to go.
Comment 12 Julien Chaffraix 2012-04-30 17:52:28 PDT
Created attachment 139554 [details]
rollout 110072 take 3. Should be fine now.
Comment 13 Julien Chaffraix 2012-05-02 09:47:21 PDT
Comment on attachment 139554 [details]
rollout 110072 take 3. Should be fine now.

For the record, I have forgotten to remove hasOverflowClipWithLayer() thus leaving tons of unneeded NULL-checks. I will change that prior to landing.
Comment 14 Julien Chaffraix 2012-05-02 10:16:30 PDT
Committed r115846: <http://trac.webkit.org/changeset/115846>
Comment 15 Andy Estes 2012-05-02 12:22:20 PDT
(In reply to comment #14)
> Committed r115846: <http://trac.webkit.org/changeset/115846>

Some of the tests this patch unskipped now fail on Mac bots (see <http://build.webkit.org/results/Lion%20Release%20(Tests)/r115846%20(8073)/results.html>). Do these just need new baselines, or are these legitimate failures?
Comment 16 Julien Chaffraix 2012-05-02 12:35:19 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > Committed r115846: <http://trac.webkit.org/changeset/115846>
> 
> Some of the tests this patch unskipped now fail on Mac bots (see <http://build.webkit.org/results/Lion%20Release%20(Tests)/r115846%20(8073)/results.html>). Do these just need new baselines, or are these legitimate failures?

Most of them just need a rebaseline (which I wouldn't have expected as they were not rebaselined since r110072). Only tables/mozilla/bugs/bug4527.html seems to be failing because of some other cause. The change in media/audio-repaint.html seems strange but not unseen (the renderers are removed from the dump instead of being reparented under a RenderLayer).

I was going to disable some of the tests to make the bots happy again but I missed the differences as the console didn't show the new failures :(
Comment 17 Andy Estes 2012-05-02 12:42:47 PDT
(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > Committed r115846: <http://trac.webkit.org/changeset/115846>
> > 
> > Some of the tests this patch unskipped now fail on Mac bots (see <http://build.webkit.org/results/Lion%20Release%20(Tests)/r115846%20(8073)/results.html>). Do these just need new baselines, or are these legitimate failures?
> 
> Most of them just need a rebaseline (which I wouldn't have expected as they were not rebaselined since r110072). Only tables/mozilla/bugs/bug4527.html seems to be failing because of some other cause. The change in media/audio-repaint.html seems strange but not unseen (the renderers are removed from the dump instead of being reparented under a RenderLayer).
> 
> I was going to disable some of the tests to make the bots happy again but I missed the differences as the console didn't show the new failures :(

Ok, I'll update the results (if you're already in the process of doing this, let me know).
Comment 18 Csaba Osztrogonác 2012-05-02 13:18:01 PDT
I updated the Qt results - http://trac.webkit.org/changeset/115875
Have you got any idea why have we more Qt specific expected results after this change?
Comment 19 Julien Chaffraix 2012-05-02 13:31:01 PDT
(In reply to comment #18)
> I updated the Qt results - http://trac.webkit.org/changeset/115875
> Have you got any idea why have we more Qt specific expected results after this change?

We haven't rebaselined Chromium yet which may explain for the extra baselines. I don't expect to do the Chromium rebaselining today as I have other stuff on my plate.