Re: [webkit-dev] On web-exposing features disabled at runtime
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
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
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
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
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
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
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
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
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
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
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