Bug 83816

Summary: Fix the ACCELERATED_COMPOSITING code to not expose RenderLayer outside rendering
Product: WebKit Reporter: Julien Chaffraix <jchaffraix>
Component: Layout and RenderingAssignee: Julien Chaffraix <jchaffraix>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, enne, eric.carlson, eric, feature-media-reviews, jamesr, kbr, senorblanco, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 83811    
Attachments:
Description Flags
Proposed refactoring 1: Added some methods on RenderBoxModelObject to abstract the need for a RenderLayer.
none
Archive of layout-test-results from ec2-cr-linux-02 none

Description Julien Chaffraix 2012-04-12 13:33:08 PDT
Currently the accelerated canvas, WebGL, accelerated transitions / animations and more generally the accelerated code path are needlessly exposing RenderLayer (and RenderLayerBacking) to different objects outside rendering.

Let's change that.
Comment 1 Julien Chaffraix 2012-04-12 14:05:17 PDT
Created attachment 136968 [details]
Proposed refactoring 1: Added some methods on RenderBoxModelObject to abstract the need for a RenderLayer.
Comment 2 James Robinson 2012-04-12 14:28:23 PDT
Comment on attachment 136968 [details]
Proposed refactoring 1: Added some methods on RenderBoxModelObject to abstract the need for a RenderLayer.

View in context: https://bugs.webkit.org/attachment.cgi?id=136968&action=review

> Source/WebCore/rendering/RenderBoxModelObject.h:169
> +    void contentChanged(ContentChangeType);

Hm, so now RenderBoxModelObject has to know about composited content?
Comment 3 Julien Chaffraix 2012-04-12 15:16:06 PDT
Comment on attachment 136968 [details]
Proposed refactoring 1: Added some methods on RenderBoxModelObject to abstract the need for a RenderLayer.

View in context: https://bugs.webkit.org/attachment.cgi?id=136968&action=review

>> Source/WebCore/rendering/RenderBoxModelObject.h:169
>> +    void contentChanged(ContentChangeType);
> 
> Hm, so now RenderBoxModelObject has to know about composited content?

Basically yes. So far, we have attached the concept of composition to RenderLayers but it's really a RenderObject decision (RenderLayer being the hook on which we attach our composition objects). Also as we are hiding RenderLayers as an implementation details to the rest of the code, I don't see (alternative suggestions welcome) another way than to push this knowledge down to the rendering hierarchy.

As we currently enable composition only for RenderBoxModelObjects, it made sense to add it here.
Comment 4 WebKit Review Bot 2012-04-12 16:26:16 PDT
Comment on attachment 136968 [details]
Proposed refactoring 1: Added some methods on RenderBoxModelObject to abstract the need for a RenderLayer.

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

New failing tests:
svg/text/font-size-below-point-five.svg
Comment 5 WebKit Review Bot 2012-04-12 16:26:22 PDT
Created attachment 136994 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 6 Julien Chaffraix 2012-04-12 17:25:20 PDT
The cr-linux failure is svg/text/font-size-below-point-five.svg. I can reproduce the failure locally and it's not related to nor impacted by this patch.
Comment 7 James Robinson 2012-04-17 13:13:17 PDT
Comment on attachment 136968 [details]
Proposed refactoring 1: Added some methods on RenderBoxModelObject to abstract the need for a RenderLayer.

OK
Comment 8 WebKit Review Bot 2012-04-17 14:51:43 PDT
Comment on attachment 136968 [details]
Proposed refactoring 1: Added some methods on RenderBoxModelObject to abstract the need for a RenderLayer.

Clearing flags on attachment: 136968

Committed r114437: <http://trac.webkit.org/changeset/114437>
Comment 9 WebKit Review Bot 2012-04-17 14:51:58 PDT
All reviewed patches have been landed.  Closing bug.