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-14 Thread Dean Jackson

On 14 Feb 2014, at 12:17 am, Sergio Villar Senin svil...@igalia.com 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-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-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 Maciej Stachowiak

On Feb 10, 2014, at 1:38 PM, Benjamin Poulain benja...@webkit.org 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 Benjamin Poulain
On 2/11/14, 9:26 AM, Maciej Stachowiak wrote:
 On Feb 10, 2014, at 1:38 PM, Benjamin Poulain benja...@webkit.org
 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 11, 2014, at 10:25 AM, Benjamin Poulain benja...@webkit.org wrote:

 On 2/11/14, 9:26 AM, Maciej Stachowiak wrote:
 On Feb 10, 2014, at 1:38 PM, Benjamin Poulain benja...@webkit.org
 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


[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


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 svil...@igalia.com 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


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 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