Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-14 Thread Dean Jackson

On 14 Feb 2014, at 12:17 am, Sergio Villar Senin  wrote:

> En 11/02/14 21:04, Maciej Stachowiak escribiu:
 
 Why not enabling the feature entirely on trunk? (and disable it when
 shipping product by disabling the compile time flag?).
 I think feature doing that tends to become stable a lot quicker.
>> Occasionally a feature is so experimental you don't even want it in nightly 
>> builds - it would cause too much instability.
>> 
>> But it's true, a lot of the time a feature is ready for testing in nightlies 
>> or developer builds, but should not be shipped just yet. I think a runtime 
>> flag is actually a good way to do that. The code that will actually compile 
>> and ship can more easily be tested that way (by testing with both modes of 
>> the flag).
> 
> So if I understood correctly the idea would be to bring the feature flag
> back and at the same time switch the runtime flag on by default for all
> the ports right?

FWIW this is what we agreed to do at the contributor’s meeting last year.

Enable by default in ToT (and thus nightly builds).
Compile-time disable on a shipping branch if you don’t want to expose it.

It’s not a hard rule though.

Dean
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-14 Thread Sergio Villar Senin
En 11/02/14 21:04, Maciej Stachowiak escribiu:
>> > 
>> > Why not enabling the feature entirely on trunk? (and disable it when
>> > shipping product by disabling the compile time flag?).
>> > I think feature doing that tends to become stable a lot quicker.
> Occasionally a feature is so experimental you don't even want it in nightly 
> builds - it would cause too much instability.
> 
> But it's true, a lot of the time a feature is ready for testing in nightlies 
> or developer builds, but should not be shipped just yet. I think a runtime 
> flag is actually a good way to do that. The code that will actually compile 
> and ship can more easily be tested that way (by testing with both modes of 
> the flag).

So if I understood correctly the idea would be to bring the feature flag
back and at the same time switch the runtime flag on by default for all
the ports right?

BR


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-11 Thread Maciej Stachowiak

On Feb 11, 2014, at 10:25 AM, Benjamin Poulain  wrote:

> On 2/11/14, 9:26 AM, Maciej Stachowiak wrote:
>> On Feb 10, 2014, at 1:38 PM, Benjamin Poulain 
>> wrote:
>>> 
>>> Can't we add a compile time flag instead? The chromium patch you
>>> linked is inelegant and it touches some hot paths.
>>> 
>>> Another advantage of compile time flag is Andreas eventually notice
>>> when they become useless and remove all the unnecessary cruft ;)
>> 
>> I think on the whole, runtime flags are a better way to do things.
>> They allow features to get much better testing before they are turned
>> on by default, among other things by enabling regression tests to
>> run. They also make it more likely that both configurations keep
>> compiling, regardless of which is default.
> 
> Why not enabling the feature entirely on trunk? (and disable it when
> shipping product by disabling the compile time flag?).
> I think feature doing that tends to become stable a lot quicker.

Occasionally a feature is so experimental you don't even want it in nightly 
builds - it would cause too much instability.

But it's true, a lot of the time a feature is ready for testing in nightlies or 
developer builds, but should not be shipped just yet. I think a runtime flag is 
actually a good way to do that. The code that will actually compile and ship 
can more easily be tested that way (by testing with both modes of the flag).

> We remove experimental features frequently. The compile time flags make
> it a lot easier to clean up.

That does seem like a benefit, though only if no one makes a mistake about what 
to put inside ifdefs. Not sure how to accomplish this while also maintaining 
the benefits of a runtime flag.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-11 Thread Benjamin Poulain
On 2/11/14, 9:26 AM, Maciej Stachowiak wrote:
> On Feb 10, 2014, at 1:38 PM, Benjamin Poulain 
> wrote:
>> 
>> Can't we add a compile time flag instead? The chromium patch you
>> linked is inelegant and it touches some hot paths.
>> 
>> Another advantage of compile time flag is Andreas eventually notice
>> when they become useless and remove all the unnecessary cruft ;)
> 
> I think on the whole, runtime flags are a better way to do things.
> They allow features to get much better testing before they are turned
> on by default, among other things by enabling regression tests to
> run. They also make it more likely that both configurations keep
> compiling, regardless of which is default.

Why not enabling the feature entirely on trunk? (and disable it when
shipping product by disabling the compile time flag?).
I think feature doing that tends to become stable a lot quicker.

We remove experimental features frequently. The compile time flags make
it a lot easier to clean up.

Benjamin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-11 Thread Maciej Stachowiak

On Feb 10, 2014, at 1:38 PM, Benjamin Poulain  wrote:

> 
> Can't we add a compile time flag instead?
> The chromium patch you linked is inelegant and it touches some hot paths.
> 
> Another advantage of compile time flag is Andreas eventually notice when they 
> become useless and remove all the unnecessary cruft ;)

I think on the whole, runtime flags are a better way to do things. They allow 
features to get much better testing before they are turned on by default, among 
other things by enabling regression tests to run. They also make it more likely 
that both configurations keep compiling, regardless of which is default.

It may be that the way Blink handled this is inelegant or inefficient (the 
latter we could test by measuring) but I'm betting there is some way to do this 
that actually works. 

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-11 Thread Sergio Villar Senin
En 10/02/14 21:55, Maciej Stachowiak escribiu:

>> The question is whether this is an acceptable behavior or not, and how
>> to fix it in case we don't like it. I guess it's safe to conclude that
>> it isn't something that we want in general, as it might confuse web
>> authors (they see the how properties are available but do nothing).
>> Regarding how to fix it, I guess the idea is to do something similar to
>> what Eric did in Blink last year[2], i.e., filtering out the list of
>> properties that are runtime disabled.
> 
> It's not a good behavior, in my opinion. I am not an expert on the CSS 
> parser, but I think we should hide the CSS interface for runtime-disabled 
> features. That would allow us to consider runtime disabling instead of 
> prefixing as a tool for early testing in more cases.

Not sure why you mention the parser here, but as I mentioned in my
email, the parser currently rejects those properties associated to
runtime disabled features. The problem is that, even in those cases
we're exposing them to the JS world.

In any case I totally agree with you that runtime disabling can be a
nice replacement of prefixed properties.

BR
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-11 Thread Manuel Rego Casasnovas
On Mon, 2014-02-10 at 13:38 -0800, Benjamin Poulain wrote:
> Can't we add a compile time flag instead?
> The chromium patch you linked is inelegant and it touches some hot paths.

BTW, only with the compile flag the issue is not completely fixed.

For example you can enable the compile flag but disable the feature at
runtime and the properties will be exposed anyway.

IMHO, if the feature is disabled at runtime the properties shouldn't be
exposed.

My 2 cents,
  Rego

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-10 Thread Sergio Villar Senin
En 10/02/14 22:38, Benjamin Poulain escribiu:
> On 2/10/14, 10:40 AM, Sergio Villar Senin wrote:
>> The question is whether this is an acceptable behavior or not, and how
>> to fix it in case we don't like it. I guess it's safe to conclude that
>> it isn't something that we want in general, as it might confuse web
>> authors (they see the how properties are available but do nothing).
>> Regarding how to fix it, I guess the idea is to do something similar to
>> what Eric did in Blink last year[2], i.e., filtering out the list of
>> properties that are runtime disabled.
>>
>> Opinions?
> 
> Can't we add a compile time flag instead?
> The chromium patch you linked is inelegant and it touches some hot paths.
> 
> Another advantage of compile time flag is Andreas eventually notice when
> they become useless and remove all the unnecessary cruft ;)

Heh, then I won't make his life easier ;)

Seriously speaking, there was a feature flag that was removed in
https://bugs.webkit.org/show_bug.cgi?id=86767. Let's bring it to life.

BR
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-10 Thread Benjamin Poulain

On 2/10/14, 10:40 AM, Sergio Villar Senin wrote:

I'm sending this to the list because it might affect different
components inside the project.

So as smfr correctly reported here[1] for all those features that don't
have a feature flag (like grid layout) we're web-exposing the CSS
properties of the feature even if it's disabled at runtime (we're
exposing them as well for features enabled at build time but disabled at
runtime).

If I am not wrong, we just disable the parsing of those properties that
are not runtime enabled, but the properties are exposed anyway,
something easy to check doing for example:
Object.getOwnPropertyDescriptor(document.body.style, property-name).


Yep that is bad.


The question is whether this is an acceptable behavior or not, and how
to fix it in case we don't like it. I guess it's safe to conclude that
it isn't something that we want in general, as it might confuse web
authors (they see the how properties are available but do nothing).
Regarding how to fix it, I guess the idea is to do something similar to
what Eric did in Blink last year[2], i.e., filtering out the list of
properties that are runtime disabled.

Opinions?


Can't we add a compile time flag instead?
The chromium patch you linked is inelegant and it touches some hot paths.

Another advantage of compile time flag is Andreas eventually notice when 
they become useless and remove all the unnecessary cruft ;)


Benjamin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] On web-exposing features disabled at runtime

2014-02-10 Thread Maciej Stachowiak

On Feb 10, 2014, at 10:40 AM, Sergio Villar Senin  wrote:

> Hi folks,
> 
> I'm sending this to the list because it might affect different
> components inside the project.
> 
> So as smfr correctly reported here[1] for all those features that don't
> have a feature flag (like grid layout) we're web-exposing the CSS
> properties of the feature even if it's disabled at runtime (we're
> exposing them as well for features enabled at build time but disabled at
> runtime).
> 
> If I am not wrong, we just disable the parsing of those properties that
> are not runtime enabled, but the properties are exposed anyway,
> something easy to check doing for example:
> Object.getOwnPropertyDescriptor(document.body.style, property-name).
> 
> The question is whether this is an acceptable behavior or not, and how
> to fix it in case we don't like it. I guess it's safe to conclude that
> it isn't something that we want in general, as it might confuse web
> authors (they see the how properties are available but do nothing).
> Regarding how to fix it, I guess the idea is to do something similar to
> what Eric did in Blink last year[2], i.e., filtering out the list of
> properties that are runtime disabled.

It's not a good behavior, in my opinion. I am not an expert on the CSS parser, 
but I think we should hide the CSS interface for runtime-disabled features. 
That would allow us to consider runtime disabling instead of prefixing as a 
tool for early testing in more cases.

 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] On web-exposing features disabled at runtime

2014-02-10 Thread Sergio Villar Senin
Hi folks,

I'm sending this to the list because it might affect different
components inside the project.

So as smfr correctly reported here[1] for all those features that don't
have a feature flag (like grid layout) we're web-exposing the CSS
properties of the feature even if it's disabled at runtime (we're
exposing them as well for features enabled at build time but disabled at
runtime).

If I am not wrong, we just disable the parsing of those properties that
are not runtime enabled, but the properties are exposed anyway,
something easy to check doing for example:
Object.getOwnPropertyDescriptor(document.body.style, property-name).

The question is whether this is an acceptable behavior or not, and how
to fix it in case we don't like it. I guess it's safe to conclude that
it isn't something that we want in general, as it might confuse web
authors (they see the how properties are available but do nothing).
Regarding how to fix it, I guess the idea is to do something similar to
what Eric did in Blink last year[2], i.e., filtering out the list of
properties that are runtime disabled.

Opinions?

BR

[1] https://bugs.webkit.org/show_bug.cgi?id=128271
[2] https://codereview.chromium.org/14324009
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev