Re: [webkit-dev] Exciting JS features (class fields) in need of review :)

[Maciej Stachowiak]

I left the boring review feedback that this work should be behind a feature 
flag. Mentioning it here because this may apply to other feature patches you 
have in progress. (I am not qualified to review the substance of what the patch 
is doing.)

> On Feb 13, 2019, at 1:51 PM, ca...@igalia.com wrote:
> 
> Hi WebKitters,
> 
> My colleagues at Igalia have been working on a number of JS language 
> features! We want WebKit to have implementations in order to provide feedback 
> for TC39, and to help meet the requirements to have them merged into the 
> specification proper.
> 
> Unfortunately, JSC suggested reviewers have been occupied for the past 6 
> months, unable to provide feedback and help us upstream the patches. I'm 
> reaching out to see if there are other folks reading webkit-dev who might be 
> able to check on the work, see how it looks, and help us get this stuff 
> upstream. We've been doing internal reviews, but unfortunately don't yet have 
> any JSC reviewers on our team.
> 
> You can find the patch for instance class fields at 
> https://bugs.webkit.org/show_bug.cgi?id=174212. Any kind of constructive 
> feedback (from anyone) would be much appreciated :)
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

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

Re: [webkit-dev] Experimental features review

[Simon Fraser]

> On Feb 13, 2019, at 11:32 AM, Michael Catanzaro  wrote:
> 
> Hi,
> 
> Last year, we cleaned up experimental features in WebPreferences.yaml to 
> ensure no experimental features were enabled by default. Since then we have 
> regressed a bit when enabling cool new web features. :) Currently we have 12 
> offenders, listed below. Most likely, the category: experimental line should 
> be removed from these features. Alternatively, the defaultValue should be 
> changed to either false or DEFAULT_EXPERIMENTAL_FEATURES_ENABLED (or 
> something depending on that).
> 
> If you know about any of these settings, please keep reading and help decide 
> what to do with them:
> 
> IntersectionObserverEnabled
> VisualViewportAPIEnabled

For these two, we now have them on by default because we think they are ready 
to ship. They still exist as experimental features so that people can turn them 
off for regression testing, but is the policy now to move them back to Debug 
features at this stage?

Simon

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

[webkit-dev] Exciting JS features (class fields) in need of review :)

[caitp]

Hi WebKitters,

My colleagues at Igalia have been working on a number of JS language features! 
We want WebKit to have implementations in order to provide feedback for TC39, 
and to help meet the requirements to have them merged into the specification 
proper.

Unfortunately, JSC suggested reviewers have been occupied for the past 6 
months, unable to provide feedback and help us upstream the patches. I'm 
reaching out to see if there are other folks reading webkit-dev who might be 
able to check on the work, see how it looks, and help us get this stuff 
upstream. We've been doing internal reviews, but unfortunately don't yet have 
any JSC reviewers on our team.

You can find the patch for instance class fields at 
https://bugs.webkit.org/show_bug.cgi?id=174212. Any kind of constructive 
feedback (from anyone) would be much appreciated :)

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

[webkit-dev] Experimental features review

[Michael Catanzaro]

Hi,

Last year, we cleaned up experimental features in WebPreferences.yaml 
to ensure no experimental features were enabled by default. Since then 
we have regressed a bit when enabling cool new web features. :) 
Currently we have 12 offenders, listed below. Most likely, the 
category: experimental line should be removed from these features. 
Alternatively, the defaultValue should be changed to either false or 
DEFAULT_EXPERIMENTAL_FEATURES_ENABLED (or something depending on that).


If you know about any of these settings, please keep reading and help 
decide what to do with them:


BlankAnchorTargetImpliesNoOpenerEnabled
ThirdPartyIframeRedirectBlockingEnabled
ScreenCaptureEnabled
WebRTCUnifiedPlanEnabled
WebRTCVP8CodecEnabled
WebRTCH264SimulcastEnabled
WebRTCMDNSICECandidatesEnabled
IntersectionObserverEnabled
VisualViewportAPIEnabled
DarkModeCSSEnabled
WebSQLDisabled
ProcessSwapOnCrossSiteNavigationEnabled

Details:

BlankAnchorTargetImpliesNoOpenerEnabled:
  type: bool
  defaultValue: true
  webcoreBinding: RuntimeEnabledFeatures
  humanReadableName: "Blank anchor target implies rel=noopener"
  humanReadableDescription: "target=_blank on anchor elements implies 
rel=noopener"

  category: experimental

ThirdPartyIframeRedirectBlockingEnabled:
  type: bool
  defaultValue: true
  humanReadableName: "Block top-level redirects by third-party iframes"
  humanReadableDescription: "Block top-level redirects by third-party 
iframes"

  category: experimental

ScreenCaptureEnabled:
 type: bool
 defaultValue: true
 webcoreBinding: RuntimeEnabledFeatures
 condition: ENABLE(MEDIA_STREAM) && PLATFORM(MAC)
 humanReadableName: "ScreenCapture"
 humanReadableDescription: "Enable ScreenCapture"
 category: experimental

WebRTCUnifiedPlanEnabled:
 type: bool
 defaultValue: true
 webcoreBinding: RuntimeEnabledFeatures
 condition: ENABLE(WEB_RTC)
 humanReadableName: "WebRTC Unified Plan"
 humanReadableDescription: "Use WebRTC Unified Plan"
 category: experimental

WebRTCVP8CodecEnabled:
 type: bool
 defaultValue: true
 webcoreBinding: RuntimeEnabledFeatures
 condition: ENABLE(WEB_RTC)
 humanReadableName: "WebRTC VP8 codec"
 humanReadableDescription: "Enable WebRTC VP8 codec"
 category: experimental

WebRTCH264SimulcastEnabled:
 type: bool
 defaultValue: true
 webcoreBinding: RuntimeEnabledFeatures
 condition: ENABLE(WEB_RTC)
 humanReadableName: "WebRTC H264 Simulcast"
 humanReadableDescription: "Enable WebRTC H264 Simulcast"
 category: experimental

WebRTCMDNSICECandidatesEnabled:
 type: bool
 defaultValue: true
 humanReadableName: "WebRTC mDNS ICE candidates"
 humanReadableDescription: "Enable WebRTC mDNS ICE candidates"
 webcoreBinding: RuntimeEnabledFeatures
 category: experimental
 condition: ENABLE(WEB_RTC)

If the above features are to remain experimental, they should be sorted 
lower in the file, below this comment explaining experimental feature 
policy:


# For experimental features:
# The type should be boolean.
# You must provide a humanReadableName and humanReadableDescription for 
all experimental features. They

# are the text exposed to the user from the WebKit client.
# The default value may be either false (for unstable features) or
# DEFAULT_EXPERIMENTAL_FEATURES_ENABLED (for features that are ready for
# wider testing).

More offenders:

IntersectionObserverEnabled:
 type: bool
 defaultValue: true
 humanReadableName: "Intersection Observer"
 humanReadableDescription: "Enable Intersection Observer support"
 webcoreBinding: RuntimeEnabledFeatures
 category: experimental
 condition: ENABLE(INTERSECTION_OBSERVER)

VisualViewportAPIEnabled:
 type: bool
 defaultValue: true
 humanReadableName: "Visual Viewport API"
 humanReadableDescription: "Enable Visual Viewport API"
 category: experimental

DarkModeCSSEnabled:
 type: bool
 defaultValue: true
 humanReadableName: "Dark Mode CSS Support"
 humanReadableDescription: "Enable Dark Mode CSS Support"
 webcoreBinding: RuntimeEnabledFeatures
 category: experimental
 condition: ENABLE(DARK_MODE_CSS)

WebSQLDisabled:
 type: bool
 defaultValue: true
 humanReadableName: "Disable Web SQL"
 humanReadableDescription: "Disable Web SQL"
 webcoreBinding: RuntimeEnabledFeatures
 category: experimental

This one's weird because it's Disabled rather than Enabled... the 
semantics should probably be reversed. We probably want a WebSQLEnabled 
feature defaulting to false?


ProcessSwapOnCrossSiteNavigationEnabled:
 type: bool
 defaultValue: DEFAULT_PROCESS_SWAP_ON_CROSS_SITE_NAVIGATION_ENABLED
 humanReadableName: "Swap Processes on Cross-Site Navigation"
 humanReadableDescription: "Swap WebContent processes on cross-site 
navigations"

 category: experimental
 webcoreBinding: none

DEFAULT_PROCESS_SWAP_ON_CROSS_SITE_NAVIGATION_ENABLED is TRUE on macOS 
and iOS, bypassing DEFAULT_EXPERIMENTAL_FEATURES_ENABLED.


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

Re: [webkit-dev] Code Style: Opinion on returning void

[Keith Miller]

I’ve definitely done this in JSC before. As with everyone else, I don’t feel 
particularly strongly about it.

> On Feb 7, 2019, at 8:45 PM, Chris Dumez  wrote:
> 
> Same here, I used it in PSON code with completion handlers. I liked the more 
> concise code but I also do not feel strongly about it.
> 
> The extra return line often would have meant adding curly brackets for if 
> statements leading to early returns.

IMO, this is why I think all if/elses should have curlies that way I don’t have 
to think about whether or not I need them, but ¯\_(ツ)_/¯. It would also make 
refactoring code much easier.

> 
> Chris Dumez
> 
> On Feb 7, 2019, at 8:23 PM, Zalan Bujtas  > wrote:
> 
>> I use this idiom too in the layout code. I guess I just prefer a more 
>> compact code.
>> (I don't feel too strongly about it though)
>> 
>> Alan.
>> 
>> 
>> On Thu, Feb 7, 2019 at 7:31 PM Alex Christensen > > wrote:
>> If you search for “return completionHandler” in WebKit you will find over a 
>> hundred instances.  Most if not all of them return void.  It means call the 
>> completion handler and return.  I probably wrote most of them and obviously 
>> think it’s a fabulous idiom.
>> 
>> > On Feb 7, 2019, at 2:32 PM, Geoffrey Garen > > > wrote:
>> > 
>> > FWIW, I’ve always felt conflicted about this case.
>> > 
>> > I very much prefer early return to if/else chains.
>> > 
>> > However, “return f()” when f returns void is a bit mind bending.
>> > 
>> > So, I can’t use my preferred style in functions that return void. Boo. 
>> > 
>> > Geoff
>> > 
>> >> On Feb 7, 2019, at 12:47 PM, Daniel Bates > >> > wrote:
>> >> 
>> >> Hi all,
>> >> 
>> >> Something bothers me about code like:
>> >> 
>> >> void f();
>> >> void g()
>> >> {
>> >>if (...)
>> >>return f();
>> >>return f();
>> >> }
>> >> 
>> >> I prefer:
>> >> 
>> >> void g()
>> >> {
>> >>if (...) {
>> >>f();
>> >>return
>> >>}
>> >>f();
>> >> }
>> >> 
>> >> Does it bother you? For the former? For the latter? Update our style 
>> >> guide?
>> >> 
>> >> Opinions, please.
>> >> 
>> >> Dan
>> >> ___
>> >> webkit-dev mailing list
>> >> webkit-dev@lists.webkit.org 
>> >> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> >> 
>> > 
>> > ___
>> > webkit-dev mailing list
>> > webkit-dev@lists.webkit.org 
>> > https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> > 
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org 
>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org 
>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

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