Bug 167693

Summary: [css-grid] Remove compilation flag ENABLE_CSS_GRID_LAYOUT
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: CSSAssignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, benjamin, commit-queue, darin, jfernandez, simon.fraser, svillar
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 60731    
Attachments:
Description Flags
Patch
none
Apply suggested changes
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Rebased patch for landing none

Description Manuel Rego Casasnovas 2017-02-01 09:25:30 PST
Now that CSS Grid Layout is enabled by default (r211415) and Safari 10.1 is shipping it [1],
it seems we might not need the compilation flag anymore.

[1] https://developer.apple.com/library/prerelease/content/releasenotes/General/WhatsNewInSafari/Articles/Safari_10_1.html
Comment 1 Manuel Rego Casasnovas 2017-02-01 09:30:04 PST
Created attachment 300330 [details]
Patch
Comment 2 Sergio Villar Senin 2017-02-06 01:45:24 PST
Comment on attachment 300330 [details]
Patch

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

Nice work. I guess that having a runtime preference won't make much sense either, but we could remove it in a follow up patch.

I'm giving r+ but wait some time for an Apple review (although I guess they'll be fine with the change as Safari is shipping it).

> Source/WebCore/dom/Position.cpp:1016
>          || is<RenderFlexibleBox>(*renderer)) {

Perhaps you could put these three in a single line now
Comment 3 Manuel Rego Casasnovas 2017-02-08 05:27:20 PST
Created attachment 300891 [details]
Apply suggested changes
Comment 4 Manuel Rego Casasnovas 2017-02-08 05:29:42 PST
Comment on attachment 300330 [details]
Patch

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

Thanks for the review.

Now let's wait for some feedback from Apple reviewers.

>> Source/WebCore/dom/Position.cpp:1016
>>          || is<RenderFlexibleBox>(*renderer)) {
> 
> Perhaps you could put these three in a single line now

Done.
Comment 5 Build Bot 2017-02-08 07:13:30 PST
Comment on attachment 300891 [details]
Apply suggested changes

Attachment 300891 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3025048

New failing tests:
compositing/masks/solid-color-masked.html
Comment 6 Build Bot 2017-02-08 07:13:33 PST
Created attachment 300894 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 7 Manuel Rego Casasnovas 2017-02-13 01:12:50 PST
Patch is ready for landing and waiting for Apple's feedback.

If nobody complains during this week, I think it's safe we land it.
Comment 8 Manuel Rego Casasnovas 2017-02-20 01:24:27 PST
Created attachment 302124 [details]
Rebased patch for landing
Comment 9 WebKit Commit Bot 2017-02-20 02:03:08 PST
Comment on attachment 302124 [details]
Rebased patch for landing

Clearing flags on attachment: 302124

Committed r212629: <http://trac.webkit.org/changeset/212629>
Comment 10 WebKit Commit Bot 2017-02-20 02:03:13 PST
All reviewed patches have been landed.  Closing bug.