[webkit-dev] Adding ENABLE_FLEXBOX to WebCore

Darin Fisher darin at chromium.org
Wed Jun 8 17:13:50 PDT 2011 

On Wed, Jun 8, 2011 at 4:59 PM, James Robinson <jamesr at google.com> wrote:

> On Wed, Jun 8, 2011 at 4:55 PM, Darin Fisher <darin at chromium.org> wrote:
>
>> Oh, okay.  Why do we have override_features.gypi then?
>>
>
> We don't, Adam tried to remove it earlier this week and was foiled by some
> weird complex failure.  We should get rid of it ASAP.
>
>
OK ... I guess things have changed.



>
>> Regardless, it seems like we could create a mechanism so that the result
>> of build-webkit
>> uses different ENABLE_ options than a stock Chromium build.  There's a
>> trivial way to switch
>> b/w the two in the GYP files.
>>
>
> There's danger in testing a different set of ENABLE_s than we ship unless
> we are really confident in understanding how the ENABLE_'d code interacts
> with the rest of the codebase.
>
>

I'm not sure that is a big deal.  The Chromium build bots at
build.chromium.org run DRT built from a Chromium checkout.  The
build.webkit.org bots are intended to provide compile and DRT feedback for
the Chromium port that is visible to the rest of the WebKit community.  If
the configs between build.webkit.org and build.chromium.org differ, maybe
that is not so bad?

Anyways, all of this is just to avoid what would ultimately be a much better
solution:  it should be possible to develop runtime enabled CSS features.
 Then, we wouldn't worry about different build configs or having to stand-up
different build bots.  It would be easier for the next person wanting to
develop a new CSS feature.

-Darin




> - James
>
>>
>> -Darin
>>
>>
>> On Wed, Jun 8, 2011 at 4:29 PM, James Robinson <jamesr at google.com> wrote:
>>
>>> On Wed, Jun 8, 2011 at 4:20 PM, Darin Fisher <darin at chromium.org> wrote:
>>>
>>>> It seems like it doesn't scale very well to have to stand-up new
>>>> buildbots for each
>>>> new feature.  At least in the Chromium port, it is possible for the
>>>> Chromium repo
>>>> to override ENABLE_ flags so that only DRT gets built with a prototype
>>>> feature,
>>>> making it easy to test a prototype feature using existing buildbots.
>>>>
>>>>
>>> If you mean by using feature_overrides.gypi to overwrite features.gypi,
>>> that mechanism doesn't actually work and people have attempted to remove it.
>>>  The bots that we actually pay attention to all use feature_overrides.gypi.
>>>  I would not encourage that pattern.
>>>
>>> - James
>>>
>>> -Darin
>>>>
>>>>
>>>> On Wed, Jun 8, 2011 at 3:43 PM, Adam Barth <abarth at webkit.org> wrote:
>>>>
>>>>> If you're super worried about folks shipping the feature before it's
>>>>> ready, then that approach can make sense.  I'm not sure how well it
>>>>> scales, but we can worry about that problem when we have N such
>>>>> configurations.
>>>>>
>>>>> Adam
>>>>>
>>>>>
>>>>> On Wed, Jun 8, 2011 at 3:19 PM, Tony Chang <tony at chromium.org> wrote:
>>>>> > I don't understand how changing the name prevents the feature from
>>>>> being
>>>>> > shipped half-done.  If we're going to ship a half-done feature, we
>>>>> may as
>>>>> > well use the vendor prefixed name so sites don't depend on the goofy
>>>>> name.
>>>>> >
>>>>> > However, I'm willing to run a buildbot for this and that seems better
>>>>> than
>>>>> > shipping a half-done feature.  I don't expect it to be a core builder
>>>>> and
>>>>> > Ojan and I will be the ones keeping it green.  Isn't this what we're
>>>>> doing
>>>>> > for other features like CSS Regions and Exclusions?
>>>>> >
>>>>> > On Wed, Jun 8, 2011 at 12:02 PM, Adam Barth <abarth at webkit.org>
>>>>> wrote:
>>>>> >>
>>>>> >> It seems like the simplest thing is to have an ENABLE macro that's
>>>>> >> turned on and to use the normal bots.  If you're really worried
>>>>> about
>>>>> >> folks shipping the feature half-done by accident, you can use a
>>>>> goofy
>>>>> >> name like -webkit-goofybox (or whatever) and rename it to the final
>>>>> >> name when you're ready.
>>>>> >>
>>>>> >> Adam
>>>>> >>
>>>>> >>
>>>>> >> On Wed, Jun 8, 2011 at 11:50 AM, Ojan Vafai <ojan at chromium.org>
>>>>> wrote:
>>>>> >> > Kind of. We could make the functionality only work at runtime, but
>>>>> >> > adding
>>>>> >> > the properties to the CSS parser would be difficult to make
>>>>> runtime
>>>>> >> > configurable. So, the CSS properties would parse correctly but do
>>>>> >> > nothing.
>>>>> >> > That's especially problematic for properties like "display" that
>>>>> would
>>>>> >> > then
>>>>> >> > get an invalid value.
>>>>> >> > My current plan was still to test this incrementally. We'd include
>>>>> tests
>>>>> >> > as
>>>>> >> > we went, but skip the flexbox subdirectory. We would just run the
>>>>> tests
>>>>> >> > locally during development. This has the downside that other
>>>>> changes
>>>>> >> > might
>>>>> >> > break the flexbox tests, but thats a pain I'm willing to live
>>>>> with.
>>>>> >> > I'm fine doing this differently if people have strong opinions.
>>>>> >> > Ojan
>>>>> >> >
>>>>> >> > On Wed, Jun 8, 2011 at 11:41 AM, Darin Fisher <darin at chromium.org
>>>>> >
>>>>> >> > wrote:
>>>>> >> >>
>>>>> >> >> Is it possible for this feature to be enabled at runtime?
>>>>> >> >>
>>>>> >> >> On Jun 8, 2011 11:38 AM, "Adam Barth" <abarth at webkit.org> wrote:
>>>>> >> >> > New features should be tested incrementally as they are
>>>>> developed.
>>>>> >> >> > That means running them on build.webkit.org. The decision to
>>>>> ship a
>>>>> >> >> > feature is separate.
>>>>> >> >> >
>>>>> >> >> > Adam
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> > On Wed, Jun 8, 2011 at 11:33 AM, Ojan Vafai <ojan at chromium.org
>>>>> >
>>>>> >> >> > wrote:
>>>>> >> >> >> I don't think we want to ship this until we have a reasonably
>>>>> >> >> >> feature
>>>>> >> >> >> complete implementation of the spec and that we're convinced
>>>>> the
>>>>> >> >> >> spec
>>>>> >> >> >> is
>>>>> >> >> >> stable. I expect that in implementing this we'll find areas of
>>>>> the
>>>>> >> >> >> spec
>>>>> >> >> >> that
>>>>> >> >> >> need reworking, but at this point it's mainly blocked on
>>>>> >> >> >> implementation
>>>>> >> >> >> experience.
>>>>> >> >> >> I'm not sure it's worth setting a bot up just for this,
>>>>> although I'm
>>>>> >> >> >> not
>>>>> >> >> >> opposed to it. I expect we should have this shippable within a
>>>>> >> >> >> couple
>>>>> >> >> >> months.
>>>>> >> >> >>
>>>>> >> >> >> Ojan
>>>>> >> >> >> On Wed, Jun 8, 2011 at 11:21 AM, Adam Barth <
>>>>> abarth at webkit.org>
>>>>> >> >> >> wrote:
>>>>> >> >> >>>
>>>>> >> >> >>> Can't we just define ENABLE_FLEXBOX on one or more of the
>>>>> commonly
>>>>> >> >> >>> used ports and use the regular bots?
>>>>> >> >> >>>
>>>>> >> >> >>> Adam
>>>>> >> >> >>>
>>>>> >> >> >>>
>>>>> >> >> >>> On Wed, Jun 8, 2011 at 10:57 AM, Tony Chang <
>>>>> tony at chromium.org>
>>>>> >> >> >>> wrote:
>>>>> >> >> >>> > Hi webkit-dev,
>>>>> >> >> >>> > I wanted to let you know that Ojan and I plan to add
>>>>> flexbox
>>>>> >> >> >>> > layout
>>>>> >> >> >>> > support
>>>>> >> >> >>> > to WebCore.  WebCore already supports an older flexbox
>>>>> >> >> >>> > implementation
>>>>> >> >> >>> > (display: box), but the new spec is designed to be easier
>>>>> for
>>>>> >> >> >>> > developers
>>>>> >> >> >>> > to
>>>>> >> >> >>> > understand and more powerful.  The old flexbox will still
>>>>> remain
>>>>> >> >> >>> > in
>>>>> >> >> >>> > WebCore
>>>>> >> >> >>> > since none of the CSS properties overlap with the new
>>>>> flexbox
>>>>> >> >> >>> > spec.
>>>>> >> >> >>> >  The
>>>>> >> >> >>> > spec can be found
>>>>> >> >> >>> >
>>>>> >> >> >>> >
>>>>> >> >> >>> >
>>>>> >> >> >>> > at: http://www.w3.org/TR/css3-flexbox/ (
>>>>> http://dev.w3.org/csswg/css3-flexbox/)
>>>>> >> >> >>> > This support will be behind the ENABLE_FLEXBOX feature
>>>>> define
>>>>> >> >> >>> > (https://bugs.webkit.org/show_bug.cgi?id=62049) and there
>>>>> is a
>>>>> >> >> >>> > meta
>>>>> >> >> >>> > bug
>>>>> >> >> >>> > tracking the feature's development
>>>>> >> >> >>> > (https://bugs.webkit.org/show_bug.cgi?id=62048).  I expect
>>>>> this
>>>>> >> >> >>> > feature
>>>>> >> >> >>> > to
>>>>> >> >> >>> > eventually be enabled by all ports.
>>>>> >> >> >>> > I am ready to setup a buildbot for tracking the compile and
>>>>> >> >> >>> > flexbox
>>>>> >> >> >>> > related
>>>>> >> >> >>> > layout tests.  Should I go ahead and get this added to
>>>>> >> >> >>> > build.webkit.org's
>>>>> >> >> >>> > waterfall?
>>>>> >> >> >>> > Thanks,
>>>>> >> >> >>> > Tony
>>>>> >> >> >>> >
>>>>> >> >> >>> > _______________________________________________
>>>>> >> >> >>> > webkit-dev mailing list
>>>>> >> >> >>> > webkit-dev at lists.webkit.org
>>>>> >> >> >>> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>>>> >> >> >>> >
>>>>> >> >> >>> >
>>>>> >> >> >>> _______________________________________________
>>>>> >> >> >>> webkit-dev mailing list
>>>>> >> >> >>> webkit-dev at lists.webkit.org
>>>>> >> >> >>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>>>> >> >> >>
>>>>> >> >> >>
>>>>> >> >> > _______________________________________________
>>>>> >> >> > webkit-dev mailing list
>>>>> >> >> > webkit-dev at lists.webkit.org
>>>>> >> >> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>>>> >> >
>>>>> >> >
>>>>> >> _______________________________________________
>>>>> >> webkit-dev mailing list
>>>>> >> webkit-dev at lists.webkit.org
>>>>> >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>>>> >
>>>>> >
>>>>> _______________________________________________
>>>>> webkit-dev mailing list
>>>>> webkit-dev at lists.webkit.org
>>>>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> webkit-dev mailing list
>>>> webkit-dev at lists.webkit.org
>>>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20110608/1278a00d/attachment-0001.html>

More information about the webkit-dev mailing list