WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
117365
[CSS Regions] RenderRegions should have a RenderLayer+Backing when they contain a Composited RenderLayer
https://bugs.webkit.org/show_bug.cgi?id=117365
Summary
[CSS Regions] RenderRegions should have a RenderLayer+Backing when they conta...
Alexandru Chiculita
Reported
2013-06-07 16:09:28 PDT
In order to enable compositing in RenderRegions we need to make the regions become RenderLayers when composited layers are displayed in these regions.
Attachments
Patch V1
(54.30 KB, patch)
2013-06-12 16:41 PDT
,
Alexandru Chiculita
hyatt
: review-
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
(714.46 KB, application/zip)
2013-06-13 08:17 PDT
,
Build Bot
no flags
Details
Patch V2
(55.53 KB, patch)
2013-06-28 01:51 PDT
,
Alexandru Chiculita
no flags
Details
Formatted Diff
Diff
Patch V3
(55.93 KB, patch)
2013-07-11 05:48 PDT
,
Alexandru Chiculita
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
(485.22 KB, application/zip)
2013-07-11 17:56 PDT
,
Build Bot
no flags
Details
Patch V4
(56.23 KB, patch)
2013-07-24 01:15 PDT
,
Alexandru Chiculita
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
(493.19 KB, application/zip)
2013-07-24 06:09 PDT
,
Build Bot
no flags
Details
Patch V5
(70.90 KB, patch)
2013-07-31 11:49 PDT
,
Alexandru Chiculita
no flags
Details
Formatted Diff
Diff
Patch V6
(74.00 KB, patch)
2013-08-01 08:53 PDT
,
Alexandru Chiculita
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Patch V7
(74.44 KB, patch)
2013-08-14 13:45 PDT
,
Alexandru Chiculita
no flags
Details
Formatted Diff
Diff
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
Alexandru Chiculita
Comment 1
2013-06-12 16:41:12 PDT
Created
attachment 204545
[details]
Patch V1
Build Bot
Comment 2
2013-06-13 08:17:14 PDT
Comment on
attachment 204545
[details]
Patch V1
Attachment 204545
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/848116
New failing tests: editing/pasteboard/4641033.html
Build Bot
Comment 3
2013-06-13 08:17:17 PDT
Created
attachment 204588
[details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
Alexandru Chiculita
Comment 4
2013-06-14 14:16:14 PDT
(In reply to
comment #2
)
> (From update of
attachment 204545
[details]
) >
Attachment 204545
[details]
did not pass mac-wk2-ews (mac-wk2): > Output:
http://webkit-queues.appspot.com/results/848116
> > New failing tests: > editing/pasteboard/4641033.html
This is unrelated and I've seen it on other bugs/patches.
Dave Hyatt
Comment 5
2013-06-27 12:17:29 PDT
Comment on
attachment 204545
[details]
Patch V1 View in context:
https://bugs.webkit.org/attachment.cgi?id=204545&action=review
r-
> Source/WebCore/rendering/RenderBox.cpp:169 > +RenderBlock* RenderBox::containingBlockForFloatingObject()
I think this needs to be renamed. containingBlock is a precise CSS term, and that is not what you're returning here. This is the outermost block that has the float in its floating object list. outermostBlockContainingFloatingObject is probably ok.
Alexandru Chiculita
Comment 6
2013-06-28 01:51:33 PDT
Created
attachment 205679
[details]
Patch V2 Updated to outermostBlockContainingFloatingObject. Also, the RenderNamedFlowThread is hitting the max object size limit enforced by the RenderArena, so I converted the new hashmaps to OwnPtrs instead.
Dave Hyatt
Comment 7
2013-07-10 11:13:25 PDT
Comment on
attachment 205679
[details]
Patch V2 Looks fine, but shouldn't you have the ifdef around the rest of the code and not just the mapping update? At the very least it seems like you should not have the extra member variable cost of OwnPtr<LayerToRegionMap> m_layerToRegionMap; when accelerated compositing is turned off.
Alexandru Chiculita
Comment 8
2013-07-11 04:31:10 PDT
(In reply to
comment #7
)
> (From update of
attachment 205679
[details]
) > Looks fine, but shouldn't you have the ifdef around the rest of the code and not just the mapping update? At the very least it seems like you should not have the extra member variable cost of OwnPtr<LayerToRegionMap> m_layerToRegionMap; when accelerated compositing is turned off.
You're right, good catch.
Alexandru Chiculita
Comment 9
2013-07-11 05:48:18 PDT
Created
attachment 206451
[details]
Patch V3
Build Bot
Comment 10
2013-07-11 17:56:19 PDT
Comment on
attachment 206451
[details]
Patch V3
Attachment 206451
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1024897
New failing tests: fullscreen/full-screen-iframe-with-max-width-height.html
Build Bot
Comment 11
2013-07-11 17:56:22 PDT
Created
attachment 206499
[details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
Alexandru Chiculita
Comment 12
2013-07-15 02:07:32 PDT
Looks l(In reply to
comment #10
)
> (From update of
attachment 206451
[details]
) >
Attachment 206451
[details]
did not pass mac-wk2-ews (mac-wk2): > Output:
http://webkit-queues.appspot.com/results/1024897
> > New failing tests: > fullscreen/full-screen-iframe-with-max-width-height.html
Looks like this test is unrelated to my change. I couldn't make it fail locally.
Alexandru Chiculita
Comment 13
2013-07-24 01:15:40 PDT
Created
attachment 207382
[details]
Patch V4
Build Bot
Comment 14
2013-07-24 06:09:50 PDT
Comment on
attachment 207382
[details]
Patch V4
Attachment 207382
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1094473
New failing tests: http/tests/security/cross-origin-plugin-private-browsing-toggled.html
Build Bot
Comment 15
2013-07-24 06:09:52 PDT
Created
attachment 207389
[details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
Build Bot
Comment 16
2013-07-29 11:11:15 PDT
Comment on
attachment 207382
[details]
Patch V4
Attachment 207382
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/1299023
Alexandru Chiculita
Comment 17
2013-07-31 11:49:26 PDT
Created
attachment 207867
[details]
Patch V5 Added code to invalidate the layer map when there's a layer added without a layout trigger.
Alexandru Chiculita
Comment 18
2013-08-01 08:53:46 PDT
Created
attachment 207929
[details]
Patch V6 This time including binary files :)
Build Bot
Comment 19
2013-08-13 02:55:37 PDT
Comment on
attachment 207929
[details]
Patch V6
Attachment 207929
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/1452371
Simon Fraser (smfr)
Comment 20
2013-08-14 11:41:27 PDT
Comment on
attachment 207929
[details]
Patch V6 View in context:
https://bugs.webkit.org/attachment.cgi?id=207929&action=review
> LayoutTests/fast/regions/layers/dynamic-layer-added-with-no-layout.html:28 > + // Force a layout > + document.body.offsetTop;
Why is this necessary?
Dave Hyatt
Comment 21
2013-08-14 11:43:04 PDT
Comment on
attachment 207929
[details]
Patch V6 Looks good. Maybe you could add asserts that you don't need layout inside the update floating objects painting container stuff? That will protect against people calling it at a bad time. I'd also bail if you need layout in addition to asserting.
Alexandru Chiculita
Comment 22
2013-08-14 13:45:05 PDT
Created
attachment 208756
[details]
Patch V7 Added needsLayout checks.
Dave Hyatt
Comment 23
2013-08-14 13:47:28 PDT
Comment on
attachment 208756
[details]
Patch V7 r=me
WebKit Commit Bot
Comment 24
2013-08-14 14:34:59 PDT
Comment on
attachment 208756
[details]
Patch V7 Clearing flags on attachment: 208756 Committed
r154072
: <
http://trac.webkit.org/changeset/154072
>
WebKit Commit Bot
Comment 25
2013-08-14 14:35:04 PDT
All reviewed patches have been landed. Closing bug.
Simon Fraser (smfr)
Comment 26
2013-08-14 18:32:58 PDT
Looks like some other tests need new baselines:
http://build.webkit.org/results/Apple%20Lion%20Release%20WK2%20(Tests)/r154084%20(12081)/fast/repaint/japanese-rl-selection-repaint-in-regions-pretty-diff.html
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug