Re: [webkit-dev] Initializing member variables

[Chris Dumez via webkit-dev]

No strong feelings but I have a slight preference for `{ }` initialization 
since it disallows narrowing conversions and works with types that do not have 
a declared constructor.
So if we have to align in one direction, it would be my preference.

Chris.

> On Sep 19, 2024, at 2:55 PM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
> 
> Should we do:
> 
> struct Foo {
> int bar = 0;
> }
> 
> Or
> 
> struct Foo {
> int bar { 0 };
> }
> 
> We do both at the moment.
> 
> - R. Niwa
> 
> ___
> 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] Naming singletons

[Chris Dumez via webkit-dev]

Happens to match our coding style already: 
https://webkit.org/code-style-guidelines/#singleton-static-member

> On Sep 13, 2024, at 2:41 PM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
> Hi all,
> 
> TL; DR: Add “Singleton” suffix to a function which returns a singleton to 
> help aid WebKit static analyzers.
> 
> It’s fairly common for some classes to provide a static member function which 
> returns a singleton. Such a function typically holds NeverDestroyed instance 
> of an object and it’s safe to call any mutating member functions on it 
> without locally storing Ref/RefPtr/CheckedRef/CheckedPtr.
> 
> Unfortunately, this poses a challenge for WebKit static analyzers which 
> enforces safe smart pointer usage because the static analyzers can’t 
> differentiate a static function which returns a singleton and a static 
> function which returns a non-singleton instance of an object. For this 
> reason, I suggest we start suffixing the name of each function which returns 
> a singleton with “Singleton” e.g. IOSurfacePool::sharedPoolSingleton instead 
> of IOSurfacePool::sharedPool. This will help aid static analyzers to identify 
> a function which returns a singleton and not generate superfluous warnings 
> for it.
> 
> - R. Niwa
> 
> ___
> 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] maybe_unused vs UNUSED_PARAM

[Chris Dumez via webkit-dev]

Right, as long as it is part of the language and consistent across compilers / 
platforms, I don’t think we need to use macros.

> On Jan 24, 2024, at 11:59 PM, Anne van Kesteren  wrote:
> 
> I had a [[fallthrough]] patch, but internal C code got in the way:
> 
> - https://en.cppreference.com/w/c/language/attributes/fallthrough
> - https://bugs.webkit.org/show_bug.cgi?id=265789
> 
> Using them directly where we can seems nice for (new) readers of the code at 
> least. Not sure what a macro for [[fallthrough]] would buy us for instance.
> 
>> On Jan 25, 2024, at 12:28 AM, Ryosuke Niwa via webkit-dev 
>>  wrote:
>> 
>> If we’re adopting [[maybe_unused]], do we just write that directly in each 
>> function declaration / definition? Or do we define some a macro to do that 
>> anyway?
>> 
>> What bout other kinds of attributes like [[noreturn]], [[fallthrough]], and 
>> [[likely]]? Are we gonna start writing them directly in code, or are we 
>> gonna continue to use macros?
>> 
>> - R. NIwa
>> 
>>> On Jan 24, 2024, at 9:49 AM, Chris Dumez via webkit-dev 
>>>  wrote:
>>> 
>>> Hi,
>>> 
>>> Thanks for starting this discussion.
>>> 
>>> I personally think it would be nice for us to switch to [[maybe_unused]] 
>>> since it is now part of the language and it seems to fit our needs. 
>>> However, I do think we should be consistent and stop using UNUSED_PARAM() / 
>>> ASSERT_UNUSED() in new code entirely then.
>>> 
>>> So if we decide to switch, I think should add style checks to prevent using 
>>> UNUSED_PARAM() / ASSERT_UNUSED() and recommend using [[maybe_unused]] 
>>> instead. Eventually, we should try to phase out existing usage of these 
>>> macros so that we can remove them entirely.
>>> 
>>> Cheers,
>>> Chris.
>>> 
>>>> On Jan 24, 2024, at 9:34 AM, Alex Christensen via webkit-dev 
>>>>  wrote:
>>>> 
>>>> For many years we have used the UNUSED_PARAM macros, and we have almost 
>>>> 3000 of them.  C++17 introduced [[maybe_unused]] for this purpose, and a 
>>>> few uses of it are starting to pop up in WebKit.  Should we switch, should 
>>>> we transition, should we allow both, or should we just stick with 
>>>> UNUSED_PARAM?
>>>> ___
>>>> 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

Re: [webkit-dev] maybe_unused vs UNUSED_PARAM

[Chris Dumez via webkit-dev]

Hi,

Thanks for starting this discussion.

I personally think it would be nice for us to switch to [[maybe_unused]] since 
it is now part of the language and it seems to fit our needs. However, I do 
think we should be consistent and stop using UNUSED_PARAM() / ASSERT_UNUSED() 
in new code entirely then.

So if we decide to switch, I think should add style checks to prevent using 
UNUSED_PARAM() / ASSERT_UNUSED() and recommend using [[maybe_unused]] instead. 
Eventually, we should try to phase out existing usage of these macros so that 
we can remove them entirely.

Cheers,
Chris.

> On Jan 24, 2024, at 9:34 AM, Alex Christensen via webkit-dev 
>  wrote:
> 
> For many years we have used the UNUSED_PARAM macros, and we have almost 3000 
> of them.  C++17 introduced [[maybe_unused]] for this purpose, and a few uses 
> of it are starting to pop up in WebKit.  Should we switch, should we 
> transition, should we allow both, or should we just stick with UNUSED_PARAM?
> ___
> 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] Stricter type checking in downcast<>()

[Chris Dumez via webkit-dev]

Hi,

I have recently landed stricter type checking in downcast<>() [1]. It is 
stricter because the check is now happening in release / production builds on 
some platforms (ARM-based).
My objective is to enable to check in release on all platforms in the near 
future (still a small performance hit on remaining platforms at the moment).

Because of this, it is now recommended to use dynamicDowncast<>() instead of 
is<>() + downcast<>(), to avoid duplicating the type check.
dynamicDowncast<>() is also less error-prone and often results in more concise 
code.

If you have a case where performance matters and you’re confident a type check 
is not required, you may rely on uncheckedDowncast<>().
uncheckedDowncast<>() behaves the same way downcast<>() used to before my 
change (type check on debug builds only).

One such example I’ve seen in our codebase is when using a switch statement 
based on the type:
```
switch (node.nodeType()) {
case Node::DOCUMENT_TYPE_NODE:
  uncheckedDowncast(node)->foo();
```

Please let me know if you have any concerns / questions.

Cheers,
Chris Dumez.

[1] https://commits.webkit.org/272296@main

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

Re: [webkit-dev] Approving / Rejects PRs on GitHub when not a reviewer?

[Chris Dumez via webkit-dev]

> On Nov 28, 2023, at 4:06 PM, Jean-Yves Avenard  
> wrote:
> 
> Hi
> 
>> On 29 Nov 2023, at 9:44 am, Chris Dumez via webkit-dev 
>>  wrote:
>> 
>> FYI, our official documentation on WebKit.org <http://webkit.org/> says:
>> ```
>> Making unofficial reviews before you become a reviewer is encouraged. This 
>> is an excellent way to show your skills. Note that you should not put r+ nor 
>> r- on patches in such unofficial reviews.
>> ```
>> I guess this wan’t updated after the move to GitHub. For me, no r+ or r- on 
>> bugzilla translates to no approve / deny PRs on GitHub. So I simply wish 
>> we’d start enforcing this policy again.
>> 
>> Having the tools help us would be great but I don’t think it stops us from 
>> enforcing our own policies like we used to.
> 
> Personally, I’ve been requesting non-official reviewers to review my patches 
> because I know that their skill set is perfectly matched (and it will help 
> make them official reviewer)
> 
> Having them giving r+ explicitly is, I find, easier to spot than looking 
> through the often busy GitHub page to find the comments.

Even if someone approves, you’d still need to find the comments. r+ with 
comments is super common.

> 
> Could we relax the ability to give informal r+ review to people with commit 
> rights? 

They can just leave a comment saying the patch looks good. They don’t have to 
approve the PR. Approving the PR means nothing since it won’t let you merge 
your PR.
People are expected to wait for an official review to trigger the merge queue, 
we’re currently making it harder than it needs to be if we have an official 
review or not.

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

Re: [webkit-dev] Approving / Rejects PRs on GitHub when not a reviewer?

[Chris Dumez via webkit-dev]

FYI, our official documentation on WebKit.org <http://webkit.org/> says:
```
Making unofficial reviews before you become a reviewer is encouraged. This is 
an excellent way to show your skills. Note that you should not put r+ nor r- on 
patches in such unofficial reviews.
```
I guess this wan’t updated after the move to GitHub. For me, no r+ or r- on 
bugzilla translates to no approve / deny PRs on GitHub. So I simply wish we’d 
start enforcing this policy again.

Having the tools help us would be great but I don’t think it stops us from 
enforcing our own policies like we used to.

> On Nov 28, 2023, at 1:58 PM, Michael Catanzaro  wrote:
> 
> On Tue, Nov 28 2023 at 01:23:12 PM -0800, Chris Dumez via webkit-dev 
>  wrote:
>> I´m on board if it also cancels PR rejections from non-reviewers, not just 
>> approvals. I don´t see how approvals differ from rejections.
> 
> Sure. It doesn't really matter whether rejections are canceled or not, 
> because the important part of the rejection is the comments that were added, 
> not the rejection status itself. A rejection from a non-reviewer is not 
> effective anyway, so it's fine to have a bot clarify that.
> 
> Michael
> 
> 

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

Re: [webkit-dev] Approving / Rejects PRs on GitHub when not a reviewer?

[Chris Dumez via webkit-dev]

> On Nov 28, 2023, at 11:51 AM, Michael Catanzaro  wrote:
> 
> On Tue, Nov 28 2023 at 10:27:41 AM -0800, Chris Dumez via webkit-dev 
>  wrote:
>> The reason I would like us to make enforce this rule is that I find it 
>> confusing. We have a lot of new comers in the project and I do not always 
>> know if a person is a reviewer or not yet. I imagine it may be even more 
>> confusing for non-Apple people.
>> I have in some cases not reviewed patches because I had seen the "green 
>> check” and thought the PR already had been approved.
> 
> +1, if there is a green checkmark, then I assume that person is a reviewer. 
> It's really confusing when non-reviewers add the green checkmark.

FYI, my understanding is that the person gets a *green* checkmark when the 
person is present in contributors.json (common case), even if not marked as a 
reviewer in that file.
They get a *grey* checkmark when they’re not present in contributors.json (not 
common). Also, the difference between green and grey for this tiny checkmark is 
super subtle.

That said, most of the instances where I saw it happened was with a green 
checkmark.

Either way, I think non-reviewers should not approve/reject PR, just add 
comments to avoid confusion.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Approving / Rejects PRs on GitHub when not a reviewer?

[Chris Dumez via webkit-dev]

Hi,

Back in the Bugzilla days, only reviewers were allowed to r+ or r- patches. 
Non-reviewers were - of course - encouraged to do informal reviews but they 
would do so by leaving comments. They would never r+ / r-.

Since we’ve moved to Github, it seems we have become a lot more lax about this 
and I have seen non-reviewers approve and reject PRs, not just leaving comments.
My understanding is that there is no way to prevent this with Github but could 
we at least make it a policy that non-reviewers should not approve / reject PRs 
and only leave comments instead?

The reason I would like us to make enforce this rule is that I find it 
confusing. We have a lot of new comers in the project and I do not always know 
if a person is a reviewer or not yet. I imagine it may be even more confusing 
for non-Apple people.

I have in some cases not reviewed patches because I had seen the "green check” 
and thought the PR already had been approved.
I have also seen cases of PRs rejected, asking the author to do more work, that 
I didn’t feel was necessary.
There is no easy way from the GitHub UI to tell if the person who 
approved/rejected your PR is actually a reviewer, as far as I know.

What do you think?

Thanks,
Chris Dumez.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Chris Dumez via webkit-dev]

> On Apr 12, 2023, at 10:23 AM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
> Yeah, enforcing that new or otherwise modified lines don’t have trailing 
> whitespaces would be good.

Yes, I wouldn’t mind that either.

However, https://commits.webkit.org/262879@main has just landed and if you look 
at the changes to Document.cpp, it is mostly spacing changes :(
It makes it harder to review or to identify meaningful changes in a patch after 
landing. It also pollutes git blame for no great reason.

> 
> - R. Niwa
> 
>> On Apr 12, 2023, at 10:20 AM, Yusuke Suzuki  wrote:
>> 
>> I agree that we should not do it because it pollutes change history of 
>> files, git-blame results, and review-diff in PR.
>> But at the same time, I think there is no reason to add a new trailing 
>> whitespace via a new commit.
>> It is nice if we can enforce this rule only for newly added code (via 
>> style-checker) not to add new trailing spaces.
>> 
>> -Yusuke
>> 
>>> On Apr 12, 2023, at 10:08 AM, Ryosuke Niwa via webkit-dev 
>>>  wrote:
>>> 
>>> WebKi proejctt’s long term policy has been to not do this:
>>> https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html
>>> 
>>> I don’t think we should change that.
>>> 
>>> - R. Niwa
>>> 
>>>> On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev 
>>>>  wrote:
>>>> 
>>>> I am against this because it adds a lot of noise to patches I am trying to 
>>>> review.
>>>> I have seen PRs where white space changes account for more than half the 
>>>> patch I am trying to review.
>>>> 
>>>> Dropping trailing spaces on the lines you’re modifying is OK but in the 
>>>> whole file is too noisy IMO.
>>>> 
>>>> Chris.
>>>> 
>>>>> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev 
>>>>>  wrote:
>>>>> 
>>>>> To reduce the overhead of switching between projects with different
>>>>> whitespace requirements, I would like to suggest we start being
>>>>> lenient when trailing whitespace is removed. In particular when a file
>>>>> is being changed to fix a bug.
>>>>> 
>>>>> I could see going even further and enforcing this via the style
>>>>> checker, if there is appetite for that.
>>>>> 
>>>>> Thanks for considering!
>>>>> ___
>>>>> 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

Re: [webkit-dev] Removal of trailing whitespace

[Chris Dumez via webkit-dev]

I am against this because it adds a lot of noise to patches I am trying to 
review.
I have seen PRs where white space changes account for more than half the patch 
I am trying to review.

Dropping trailing spaces on the lines you’re modifying is OK but in the whole 
file is too noisy IMO.

Chris.

> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev 
>  wrote:
> 
> To reduce the overhead of switching between projects with different
> whitespace requirements, I would like to suggest we start being
> lenient when trailing whitespace is removed. In particular when a file
> is being changed to fix a bug.
> 
> I could see going even further and enforcing this via the style
> checker, if there is appetite for that.
> 
> Thanks for considering!
> ___
> 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] Unsigned to avoid negative values

[Chris Dumez via webkit-dev]

Hi,

What’s the benefit? I don’t think we should be changing our long-time coding 
practices unless there are clear benefits from doing so.
From your email, it is not yet clear to me what those benefits would be.

Chris.

> On Jan 24, 2023, at 6:58 AM, Michael Catanzaro via webkit-dev 
>  wrote:
> 
> On Tue, Jan 24 2023 at 02:00:11 AM -0800, Myles Maxfield via webkit-dev 
>  wrote:
>> What do you think?
> 
> This has been a best practice for a long time now. It's a good rule to reduce 
> bugs if adopted consistently, but I also fear that if we were to try to adapt 
> existing WebKit code to follow these guidelines, we might accidentally 
> introduce a lot of new bugs, especially regarding container types like 
> Vector. So I don't know what to think!
> 
> 
> ___
> 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] Style guide: enforce `while (true)` over `for (; ; )`

[Chris Dumez via webkit-dev]

Sounds good to me.Chris DumezOn Oct 5, 2022, at 9:04 PM, Kirsling, Ross via webkit-dev  wrote:




I've always kind of liked that `for (;;)` doesn't involve an explicit constant, but I too like consistency even more. :)

From: Ryosuke Niwa via webkit-dev 
Sent: Thursday, October 6, 2022 1:01:01 PM
To: Yusuke Suzuki ; Tim Nguyen ; WebKit Development 
Subject: Re: [webkit-dev] Style guide: enforce `while (true)` over `for (; ; )`
 

I do prefer
for (;;) because of less typing but if the existing code mostly uses
while (true) then we should go with it.


On Oct 5, 2022, at 8:58 PM, Yusuke Suzuki via webkit-dev  wrote:


+1

-Yusuke

On Oct 5, 2022, at 5:07 PM, Tim Nguyen via webkit-dev  wrote:

Hi everyone,

The WebKit codebase has an inconsistent mix of `while (true)` and `for (;;)`. Given 2/3 of the usages are `while (true)` and only 1/3 is `for (;;)` from code search, I would suggest enforcing `while (true)`. I also think it is generally more explicit and readable
 than `for (;;)`. If everyone agrees, I’ll enforce this via webkit-style, so we can end up in a consistent place.

What does everyone think?

Cheers,
Tim
___
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 listwebkit-dev@lists.webkit.orghttps://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] Proposal: Mandatory Commit and Merge Queue

[Chris Dumez via webkit-dev]

> On Jun 2, 2022, at 6:29 PM, Michael Catanzaro  wrote:
> 
> 
> 
> On Thu, Jun 2 2022 at 04:06:42 PM -0700, Chris Dumez via webkit-dev 
>  wrote:
>> I am concerned by this proposal given how slow EWS and the merge queue are 
>> these days.
> 
> Hopefully Jonathan's three proposed blockers will address this:
> 
>> - run-webkit-tests consulting results.webkit.org to avoid retrying known 
>> flakey or failing tests
>> - Another MergeQueue bot
>> - Xcode workspace builds to speed up incremental builds

It’s all speculative until we get actual merge queue times with those changes 
implemented. Implementing.a new restrictive policy based on speculative data 
sounds risky to me. Let’s not “hope”, let’s gather data.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Proposal: Mandatory Commit and Merge Queue

[Chris Dumez via webkit-dev]

I am concerned by this proposal given how slow EWS and the merge queue are 
these days. 

I think the issue is likely related to failures on the bots and the retries 
needed to determine if those failures are new.

We’ve never had this restriction before. Seems we are becoming overly strict 
with the move to GitHub and development speed will suffer as a result.

Chris Dumez

> On Jun 2, 2022, at 3:41 PM, Geoffrey Garen via webkit-dev 
>  wrote:
> 
> 
>> As we move to GitHub, I would like to propose we strengthen our protections 
>> on `main` by making MergeQueue and CommitQueue mandatory. 
> 
> 
> What is the goal of this proposal?
> 
> What problem are you trying to solve, and with what level of urgency?
> 
> Thanks,
> Geoff
> 
> 
>> On Jun 2, 2022, at 2:35 PM, Jonathan Bedard via webkit-dev 
>>  wrote:
>> 
>> As we move to GitHub, I would like to propose we strengthen our protections 
>> on `main` by making MergeQueue and CommitQueue mandatory. This would mean 
>> that with a few exceptions, all changes would need to be built and run 
>> layout tests before they are landed. To spell out what the exceptions I had 
>> in mind are:
>> 
>> - Revert commits, identified by a commit message that starts with 
>> “Unreviewed, revering…” would be exempt
>> - Changes which only modify files that do not effect building or testing 
>> WebKit would be exempt. These files specifically are:
>>  .github/
>>  JSTests/
>>  ManualTests/
>>  metadata/
>>  PerformanceTests/
>>  Tools/
>>   CISuport/
>>   EWSTools/
>>   WebKitBot
>> Websites/
>> - Emergency build and infrastructure fixes, identified by a commit message 
>> that starts with “Emergency build fix” or “Emergency infrastructure fix” 
>> would be exempt
>> - A reviewer who is not the commit author can overwrite this protection by 
>> adding `unsafe-merge-queue` instead of the commit author
>> - Changes which passed an EWS layout test queue within the last 7 days would 
>> skip the layout test check
>> 
>> These exceptions are designed to provide contributors for a way to by-pass 
>> potentially slow checks if extraordinary situations, or in ones where CI has 
>> already validated the change. I think we should keep the ability for any 
>> committer to deploy an emergency fix, because our project has many 
>> contributors in different timezones and with different holiday schedules.
>> 
>> We know that this policy change would potentially slow down development, so 
>> I think these 3 improvements block making MergeQueue and CommitQueue 
>> mandatory:
>> 
>> - run-webkit-tests consulting results.webkit.org to avoid retrying known 
>> flakey or failing tests
>> - Another MergeQueue bot
>> - Xcode workspace builds to speed up incremental builds
>> 
>> Jonathan Bedard
>> WebKit Continuous Integration
>> 
>> ___
>> 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

Re: [webkit-dev] Add CODEOWNERS to WebKit

[Chris Dumez via webkit-dev]

I’m kind of ambivalent/neutral about this. One question though:

When adopting CODEOWNERS, will our existing watchlists get ported, or will each 
contributor have to modify CODEOWNERS themselves to match whatever was in the 
watchlists for them?

Thanks,
Chris.

> On Jun 2, 2022, at 1:12 PM, Jonathan Bedard via webkit-dev 
>  wrote:
> 
> Hey folks,
> 
> Yusuke is interested in adding the CODEOWNERS file to the WebKit project (see 
> https://github.com/WebKit/WebKit/pull/1137). There have been some objections 
> to this, the two ones I’m aware of:
> - WebKit doesn’t assign “ownership” to pieces of code, so the name is 
> unfortunate
> - The last match “wins”, so if you subscribed to a generic directory early in 
> the file, more specific matches would override that subscription
> 
> Despite these downsides, I think adding the CODEOWNERS file is good for the 
> project for a few reasons:
> - It’s a standard natively supported by GitHub, BitBucket and GitLab and will 
> be familiar to developers
> - File format is more simple than existing watchlist
> - Support for groups and individuals
> - No need for WebKit to host a service (other implementations of auto-CCing 
> would require this)
> 
> I intend to work with Yusuke to land 
> https://github.com/WebKit/WebKit/pull/1137 and start adopting CODEOWNERS on 
> Monday absent objections that folks think overshadow the benefits of the 
> CODEOWNERS file.
> 
> Jonathan
> ___
> 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] Proposal: Immediate Deprecation of ChangeLogs

[Chris Dumez via webkit-dev]

> On May 11, 2022, at 11:56 AM, Jonathan Bedard via webkit-dev 
>  wrote:
> 
> Trying to embed previous replies is going to get messy, will be referencing 
> those replies but not embedding them.
> 
> Unsafe-Merge-Queue should be very fast, I haven’t seen anything take longer 
> than 10 minutes from label application to landing or rejection. The average 
> case is 3-4 minutes. We’re aware of what the architectural problem with 
> Commit-Queue is that slows down the fast path, Unsafe-Merge-Queue has fixed 
> that. The solution we used isn’t transferable to Commit-Queue.

I personally don’t think that delaying a critical build fix / gardening by 10 
minutes is OK. It’s called “unsafe-merge-queue”, why does it take this long to 
commit? Why isn't it as fast as "generate a unique identifier and git push”?
Sure, I have no idea how the unsafe-merge-queue does but it is hard for me to 
imagine it should take more than 1 minute given what it needs to do and given 
that it should need to do extremely little testing given that it is “unsafe”.

4 to 10 minutes is nowhere near as fast as `git svn dcommit` and is therefore a 
pretty large regression.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Proposal: Immediate Deprecation of ChangeLogs

[Chris Dumez via webkit-dev]

> On May 11, 2022, at 12:13 AM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
> On Tue, May 10, 2022 at 9:27 PM Ryosuke Niwa  wrote:
>> 
>> 
>> On Tue, May 10, 2022 at 20:36 Chris Dumez  wrote:
>>> 
>>> [Not sure why Apple Mail sent Ryosuke’s replies to the Junk folder but I 
>>> finally noticed.]
>> 
>> 
>> It's something to do with @webkit.org not being able to send a proper sender 
>> ID due to it being a forwarding address.
>> 
>>> On May 10, 2022, at 3:04 PM, Ryosuke Niwa via webkit-dev 
>>>  wrote:
>>> 
>>> On Tue, May 10, 2022 at 3:01 PM Jonathan Bedard via webkit-dev
>>>  wrote:
>>> 
>>> 
>>> On May 10, 2022, at 2:46 PM, Geoffrey Garen  wrote:
>>> 
>>> Do I undertand correctly that the proposal here is
>>> 
>>> (a) Immediately Deprecate ChangeLogs
>>> 
>>> 
>>> Yes
>>> 
>>> (b) Immediately end support for posting patches from Subversion 
>>> checkouts?
>>> 
>>> 
>>> We would be immediately ending support for _landing_ patches posted from a 
>>> Subversion checkout. EWS would continue to accept and test patches posted 
>>> from Subversion checkouts.
>>> 
>>> 
>>> Just this week, I landed 2~3 patches using a pure Subversion checkout.
>>> It's actually my primary method of landing patches in WebKit right
>>> now.
>>> 
>>> 
>>> Do you feel like 1 week is not enough time for you to do a git checkout and 
>>> familiarize yourself enough with GIT to upload patches? Is that the issue? 
>>> If so, how long do you feel would be reasonable?
>> 
>> 
>> I already have GitHub clones. The issue I have is with committing directly. 
>> I need to be able to commit directly as is since commit queue even fast one 
>> is simply way too slow.

I agree that the fast-cq is a lie and is way slower than committing manually 
(and usually not by a little). I haven’t tried the unsafe-merge-queue in GitHub 
yet but I hope it is much faster than fast-cq was. If unsafe-merge-queue is not 
nearly as fast as a manual commit then I think it needs to be fixed.
Given that the plan of record is that nobody has direct commit access to the 
GIT repo and the only way to land a build fix or test gardening will be via the 
merge queue, I think it is critical.

I’ll note though that IMO, there are some very specific cases for when 
extremely fast commit is required. Namely, build fixes and tests gardening, to 
get the tree / bots in a sane state as soon as possible and keep things running 
smoothly. For other kind of changes, I personally think they should go through 
rigorous EWS testing and merge queue.
That said, the speed of the commit queue and EWS has been going downhill and 
this is less and less practical IMO. The main reason for this seems to be that 
some tests are most of the time either plain failing or flaky on EWS, making 
processing much slower because the bot has to retry each patch. This can 
hopefully be addressed with much stricter / aggressive gardening / sheriffing.

>> 
>>> If you’re not ready to adopt the GitHub workflow for a reason or another, 
>>> git-svn / bugzilla patches is still a thing and will still work for now. 
>>> Only committing from pure SVN repositories would go away in a week.
>> 
>> 
>> Well, that's precisely my use case. I don't even write a patch in a pure 
>> Subversion checkout anymore these days.
>> 

Ok, seems like you are using GitHub checkouts for writing the patch and then 
separate Subversion checkout to commit the patch. I find it a bit surprising 
given that GitHub checkouts can still commit to SVN directly via git-svn 
(`git-webkit setup-git-svn` to set up as per 
https://github.com/WebKit/WebKit/wiki/Contributing#checking-out-WebKit).
Jonathan only mentioned that commits from pure SVN checkouts would be broken. 
He didn’t say anything about commits from git-svn checkouts so I assume those 
would still work (and should be more convenient for you?). @Jonathan, please 
correct me if I’m wrong.

>> - R. Niwa
>> 
>> --
>> - R. Niwa
> ___
> 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] Proposal: Immediate Deprecation of ChangeLogs

[Chris Dumez via webkit-dev]

[Not sure why Apple Mail sent Ryosuke’s replies to the Junk folder but I 
finally noticed.]

> On May 10, 2022, at 3:04 PM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
> On Tue, May 10, 2022 at 3:01 PM Jonathan Bedard via webkit-dev
>  wrote:
>> 
>>> On May 10, 2022, at 2:46 PM, Geoffrey Garen  wrote:
>>> 
>>> Do I undertand correctly that the proposal here is
>>> 
>>>  (a) Immediately Deprecate ChangeLogs
>> 
>> Yes
>> 
>>>  (b) Immediately end support for posting patches from Subversion 
>>> checkouts?
>> 
>> We would be immediately ending support for _landing_ patches posted from a 
>> Subversion checkout. EWS would continue to accept and test patches posted 
>> from Subversion checkouts.
> 
> Just this week, I landed 2~3 patches using a pure Subversion checkout.
> It's actually my primary method of landing patches in WebKit right
> now.

Do you feel like 1 week is not enough time for you to do a git checkout and 
familiarize yourself enough with GIT to upload patches? Is that the issue? If 
so, how long do you feel would be reasonable?

If you’re not ready to adopt the GitHub workflow for a reason or another, 
git-svn / bugzilla patches is still a thing and will still work for now. Only 
committing from pure SVN repositories would go away in a week.

> 
> - R. Niwa
> ___
> 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] Proposal: Immediate Deprecation of ChangeLogs

[Chris Dumez via webkit-dev]

Hi,

I think this is a step in the right direction.

I hope concerns from other contributors about change log reviews can be 
addressed in the near future. However, I don’t think it should prevent us from 
moving away from ChangeLog files, given that commenting on commit logs is still 
possible in GitHub, although not conveniently.

Thanks,

> On May 10, 2022, at 1:32 PM, Jonathan Bedard via webkit-dev 
>  wrote:
> 
> A few weeks ago, I started a discussion about deprecating ChangeLogs. In that 
> time, we’ve had more folks using the pull-request workflow and more folks 
> using newer versions of `git` which break automatic ChangeLog rebasing. I 
> propose that on Monday, May 16th, we implement the following policy changes 
> for the WebKit project:
> 
> - Commits no longer require ChangeLogs, they instead require commit messages
> - Commit messages are in the format of `prepare-ChangeLog --no-write`
> 
> Pull-request workflows based on `git-webkit` already support this workflow 
> well, and `git-webkit setup` creates a `prepare-commit-msg` hook that will 
> appropriately format commit messages. In addition, `git format-patch` allows 
> us to create a patch which contains a commit message. This means that 
> contributors still using patch workflows from a git or git-svn checkout will 
> be able to upload compliant patches to bugzilla.
> 
> This will, however, break contributors using pure-Subversion checkouts. This 
> is something that’s going to be happening in the very near future as we 
> deprecate Subversion entirely, so I think this is an acceptable cost in 
> exchange for fully supporting native git workflows.
> 
> The last thing I’d like to note is that a full git-native commit message 
> policy now is something we can modify in the future if we find that reviewing 
> commit messages with “Quote reply” comments is not sufficient, but resolving 
> project disagreements on how or if to address deficiencies in GitHub commit 
> message review don’t seem to be headed towards a resolution quickly.
> 
> Jonathan
> WebKit Continuous Integration
> 
> ___
> 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] ChangeLog Deprecation Plans

[Chris Dumez via webkit-dev]

I spent some time discussing this offline with Yusuke to better understand his 
proposal.

It sounds like Yusuke’s proposal is:
1. Have a separate COMMIT_MESSAGE file as part of the PR
2. When the merge queue commits the PR, it uses that COMMIT_MESSAGE content as 
commit log message and then drops the file from the commit.

I understand that this approach is a bit more flexible for people who like to 
make multiple commits locally for the same PR. I personally always have a 
single commit per PR that I keep amending but I think it is good to be flexible.
It also makes reviewing the changelog message a bit easier on GitHub.

I support Yusuke’s proposal. It is a bit more flexible and it still means that 
separate full/history ChangeLog files would be a thing of the past.

--
 Chris Dumez




> On Apr 22, 2022, at 2:10 PM, Chris Dumez via webkit-dev 
>  wrote:
> 
> I am strongly in favor of dropping the ChangeLog files and relying on the GIT 
> commit message instead. Not having to update ChangeLog files was actually a 
> big motivator for me to switch to GitHub, as I thought until now we didn’t 
> have to update ChangeLog files when switching Github PRs.
> 
> With the provided GIT commit hook, the changelog format is identical to what 
> we had in the ChangeLog files anyway. I don’t understand (yet) the need for 
> having the same message in two separate location.
> 
> Git commit logs don’t roll over (like ChangeLog files do), they are 
> searchable, they have the same format (you CAN add inline comments on a 
> per-file basis for e.g.). They are also easily modifiable from the GitHub 
> interface.
> 
> I will also note that ChangeLog files are a frequent source of conflicts when 
> using GIT. Yes, we do have a resolve-ChangeLogs script that’s supposed to 
> help with that. However, please note that this script hasn’t work reliably 
> for quite a while (at least for many of us at Apple with very recent SDKs).
> ChangeLog entries no longer show on top when rebasing, meaning I have to move 
> them back on top manually. Worse, if I fail to notice and use `webkit-patch 
> upload`, it will upload to the wrong bugzilla bug.
> 
> If people really still want to maintain separate ChangeLog files, I am hoping 
> this can be generated from my commit messages and done automatically for me 
> upon committing. I really don’t want that as part of my patch/PR.
> 
> --
>  Chris Dumez
> 
> 
> 
> 
>> On Apr 19, 2022, at 11:00 AM, Geoffrey Garen via webkit-dev 
>> mailto:webkit-dev@lists.webkit.org>> wrote:
>> 
>>>> Commit message is tied to a commit, so editing commit without breaking 
>>>> commit-message is hard (how to revert one change from one commit without 
>>>> rewriting commit message? It requires some git hack). Separate independent 
>>>> COMMIT_MESSAGE file can solve this problem and makes local development 
>>>> easy.
>>> 
>>> Developers who are used to git -- which nowadays is pretty much everyone 
>>> except WebKit devs -- are also used to rewriting commit messages.
>> 
>> I think it’s important for WebKit's git transition to take consideration of 
>> WebKit developers and WebKit workflows. I hope we can agree on this premise 
>> as we discuss various options for commit messages.
>> 
>> Thanks,
>> Geoff
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org <mailto: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

Re: [webkit-dev] ChangeLog Deprecation Plans

[Chris Dumez via webkit-dev]

I am strongly in favor of dropping the ChangeLog files and relying on the GIT 
commit message instead. Not having to update ChangeLog files was actually a big 
motivator for me to switch to GitHub, as I thought until now we didn’t have to 
update ChangeLog files when switching Github PRs.

With the provided GIT commit hook, the changelog format is identical to what we 
had in the ChangeLog files anyway. I don’t understand (yet) the need for having 
the same message in two separate location.

Git commit logs don’t roll over (like ChangeLog files do), they are searchable, 
they have the same format (you CAN add inline comments on a per-file basis for 
e.g.). They are also easily modifiable from the GitHub interface.

I will also note that ChangeLog files are a frequent source of conflicts when 
using GIT. Yes, we do have a resolve-ChangeLogs script that’s supposed to help 
with that. However, please note that this script hasn’t work reliably for quite 
a while (at least for many of us at Apple with very recent SDKs).
ChangeLog entries no longer show on top when rebasing, meaning I have to move 
them back on top manually. Worse, if I fail to notice and use `webkit-patch 
upload`, it will upload to the wrong bugzilla bug.

If people really still want to maintain separate ChangeLog files, I am hoping 
this can be generated from my commit messages and done automatically for me 
upon committing. I really don’t want that as part of my patch/PR.

--
 Chris Dumez




> On Apr 19, 2022, at 11:00 AM, Geoffrey Garen via webkit-dev 
>  wrote:
> 
>>> Commit message is tied to a commit, so editing commit without breaking 
>>> commit-message is hard (how to revert one change from one commit without 
>>> rewriting commit message? It requires some git hack). Separate independent 
>>> COMMIT_MESSAGE file can solve this problem and makes local development easy.
>> 
>> Developers who are used to git -- which nowadays is pretty much everyone 
>> except WebKit devs -- are also used to rewriting commit messages.
> 
> I think it’s important for WebKit's git transition to take consideration of 
> WebKit developers and WebKit workflows. I hope we can agree on this premise 
> as we discuss various options for commit messages.
> 
> Thanks,
> Geoff
> ___
> 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] How to make changes to website?

[Chris Dumez via webkit-dev]

Jon Davis is probably the right contact for Webkit.org .

--
 Chris Dumez




> On Feb 16, 2022, at 2:01 PM, Michael Catanzaro via webkit-dev 
>  wrote:
> 
> Hi,
> 
> I want to make a modification to:
> 
> https://webkit.org/contributing-code/
> 
> Suggested here: https://bugs.webkit.org/show_bug.cgi?id=232935#c6
> 
> It looks like this page is not part of WebKit/Websites/webkit.org. Anyone 
> know who can edit it?
> 
> Michael
> 
> 
> ___
> 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] How to set up Intellisense-ish code completion/suggestions for editing WebKit sources on macOS?

[Chris Dumez via webkit-dev]

The code completion is excellent with Qt Creator.

You can import the WebKit project like so:
New Project > Import existing project > Choose the WebKit folder > click import 
button

Once imported, no need to fiddle with any settings, it just works (after an 
initial project scan that may take a couple of minutes).

--
 Chris Dumez




> On Nov 11, 2021, at 7:49 AM, Patrick Griffis via webkit-dev 
>  wrote:
> 
> On 2021-11-11 02:48, Michael[tm] Smith via webkit-dev wrote:
>> Can anyone recommend a combination of text-editor/IDE, plugins/tooling
>> (e.g., language server), and settings/config that’ll enable me to have
>> usable code-(auto)completion/suggestions (like Intellisense, etc.) when
>> editing WebKit sources in a macOS environment?
>> 
>> Anyway, the last thing I’ve been trying is Visual Studio Code. In that I
>> tried with both the clangd extension and the ccls extension, but ended up
>> having basically the same problems I have with the vim integrations.
>> 
>> So I switched back to trying the Microsoft-provided C/C++ Intellisense
>> extension (cpptools), and found that seems to work better than the clangd
>> or ccls extensions — at least so far as it seems to be able to at last
>> partially resolve the header include references. But then it too seems to
>> stumble on not being able to find some headers it needs.
>> 
>> For example, I think I’ve been able to make it figure out #include 
>> "config.h" —
>> but then the next problem I hit is stuff like this:
>> 
>>  cannot open source file "JavaScriptCore/JSExportMacros.h"
>> (dependency of "config.h")
>> 
>> ...And then anyway, again, ultimately, no completion suggests when I put a 
>> dot
>> after a particular object name I want to get the function and data-member
>> suggestions for. (It seems to work for some objects, but not others.)
>> 
>> So I’m hoping others here might have something working successfully in
>> their environments that gives them proper completion suggestions on member-
>> function names and data-member names.
>> 
>>  –Mike
> 
> This may be of limited use to you because I am on Linux but may be
> useful information in general. It works here using Visual Studio Code
> with the Cmake Tools and ccls extensions installed.
> 
> Header detection works, references work (including basic refactoring),
> intellisense works (not super fast but functional), debugging with GDB
> works (connecting to gdbserver with Microsoft's C++ extension for that).
> 
> I had the exact same header issues as you with only Microsofts C++
> extension. I have done no configuration to ccls.
> ___
> 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] Removal of makeRef() / makeRefPtr()

[Chris Dumez via webkit-dev]

Hi,

With WebKit adopting C++17 a while back, there are no longer any benefits to 
using makeRef() / makeRefPtr() as far as I can tell.

Code that was written like so:
auto protectedThis = makeRef(*this);
auto protectedPage = makeRefPtr(page);
auto result = makeRef(foo->bar());
auto lambda = [protectedThis = makeRef(*this)] { };
m_node = makeRef(node); // m_node being a Ref
m_node = makeRefPtr(node); // m_node being a RefPtr

Can now be written in a more concise way:
Ref protectedThis { *this };
RefPtr protectedPage { page };
Ref result = foo->bar();
auto lambda = [protectedThis = Ref { *this }] { };
m_node = node;
`m_node = node;` or `m_node = &node; // depending if node is a reference or 
pointer.

I am currently in the process on transitioning our code from one style to the 
other and removing makeRef() / makeRefPtr() altogether.
I am sending this email so people are not confused when they try to use 
makeRef() / makeRefPtr() in new code.

--
 Chris Dumez




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

Re: [webkit-dev] Request for position: Cryptographically secure random UUIDs

[Chris Dumez via webkit-dev]

I forgot to follow-up on this email, sorry.

Support for this landed via https://bugs.webkit.org/show_bug.cgi?id=229240 
.

--
 Chris Dumez




> On Apr 10, 2021, at 7:00 PM, Benjamin Coe via webkit-dev 
>  wrote:
> 
> Hello WebKit folks,
> 
> This is a request for WebKit's position on introducing the JavaScript method 
> randomUUID(), for generating RFC 4122 version 4 identifiers, to the crypto 
> interface.
> 
> - Explainer: https://github.com/WICG/uuid/blob/gh-pages/explainer.md 
> 
> - Specification: https://wicg.github.io/uuid/ 
> - ChromeStatus: https://www.chromestatus.com/feature/5689159362543616 
> 
> 
> Look forward to feedback,
> 
> -- Ben.
> 
> ___
> 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] Request for position on self.reportError()

[Chris Dumez via webkit-dev]

It doesn’t seem like a controversial feature to me and it is indeed a fairly 
small amount of work to support.

I am working on it via Bug 228316 
.

--
 Chris Dumez




> On Aug 27, 2021, at 1:58 PM, Domenic Denicola via webkit-dev 
>  wrote:
> 
> Hi webkit-dev,
> 
> We recently added a small utility function, self.reportError(), to the HTML 
> Standard [1]. It is pretty simple and just lets developers appropriately send 
> errors to the "error" event handler and the console, like what happens when 
> the browser reports uncaught exceptions.
> 
> This is already implemented in Firefox and we're looking to ship it in Chrome 
> soon. Would you all be interested this feature as well? It should be pretty 
> simple to implement; it was for us at least. [2]
> 
> Thanks,
> -Domenic
> 
> [1]: https://github.com/whatwg/html/pull/1196
> 
> [2]: https://chromium-review.googlesource.com/c/chromium/src/+/3125854
> ___
> 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] Moving from WTF::Optional to std::optional

[Chris Dumez via webkit-dev]

Hi,

Another thing Darin didn’t mention but I think people should be careful about:
The move constructor for std::optional does not clear the is-set flag (while 
the one for WTF::Optional did).

As a result, you will be having a very bad time if you do a use-after-move of a 
std::optional. Please make sure to use std::exchange() instead of WTFMove() if 
you want to leave to std::optional in a clean state for reuse later.

Chris Dumez

> On Jun 1, 2021, at 8:54 PM, Darin Adler via webkit-dev 
>  wrote:
> 
> Hi folks.
> 
> We’re getting rid of the WTF::Optional class template, because, hooray, 
> std::optional is supported quite well by all the C++17 compilers we use, and 
> we don’t have to keep using our own special version. Generally we don’t want 
> to reimplement the C++ standard library when there is not a significant 
> benefit, and this is one of those times.
> 
> Here are a few considerations:
> 
> 1) Since https://commits.webkit.org/238290@main, if you use Optional<> by 
> mistake instead of std::optional<>, your code won’t compile. (Unless you are 
> writing code for ANGLE, which has its own separate Optional<>.)
> 
> 2) If you want to use std::optional, include the C++ standard header, 
> , or something that includes it. In a lot of cases, adding an 
> include will not be required since it’s included by widely-used headers like 
> WTFString.h and Vector.h, so if you include one of those are covered. Another 
> way to think about this is that if your base class already uses 
> std::optional, then you don’t need to include it.
> 
> 3) Once the patch in https://bugs.webkit.org/show_bug.cgi?id=226437 lands, 
> includes of  won’t forward declare optional, and includes of 
>  won’t do anything at all.
> 
> — Darin
> ___
> 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] Clang Thread Safety Analysis

[Chris Dumez via webkit-dev]

Hi,

Just a quick follow-up to let you know that both CheckedLock/CheckedCondition 
and UncheckedLock/UncheckedCondition have been removed from the tree.
The whole codebase has been ported to Lock/Condition, which have the thread 
safety analysis annotations.

--
 Chris Dumez




> On May 23, 2021, at 10:40 PM, Chris Dumez  wrote:
> 
> Clang Thread Safety Analysis
> WTF::Lock now supports Clang Thread Safety Analysis 
> . It is a C++ language 
> extension which warns about potential race conditions in code, at compile 
> time. It is the same great Lock, but with some extra help from clang to make 
> our multi-threaded code safer by giving out compilation errors when thread 
> unsafe code is detected.
> 
> Annotations
> Just by using WTF::Lock, Clang will already be able to do some basic 
> validation. However, to take full advantage of it, there are a few 
> annotations you should know about. Note that you'll see those annotations 
> throughout the code base already as a few of us have been busy adopting them. 
> All these annotations are declared in wtf/ThreadSafetyAnalysis.h 
> .
> 
> WTF_GUARDED_BY_LOCK()
> 
> WTF_GUARDED_BY_LOCK is an attribute on data members, which declares that the 
> data member is protected by the given lock. Read operations on the data 
> require shared access, while write operations require exclusive access.
> 
> WTF_POINTEE_GUARDED_BY_LOCK is similar, but is intended for use on pointers 
> and smart pointers. There is no constraint on the data member itself, but the 
> data that it points to is protected by the given lock.
> 
> class Foo {
> Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
> Lock m_stringsLock;
> };
> You will get a compiler error if you try to access or modify m_strings 
> without grabbing the m_stringsLock lock first.
> 
> WTF_REQUIRES_LOCK()
> 
> WTF_REQUIRES_LOCK is an attribute on functions or methods, which declares 
> that the calling thread must have exclusive access to the given locks. More 
> than one lock may be specified. The locks must be held on entry to the 
> function, and must still be held on exit.
> 
> class Foo {
> void addString(String&&) WTF_REQUIRES_LOCK(m_stringsLock);
> 
> Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
> Lock m_stringsLock;
> };
> You will get a compiler error if you try to call addString() without grabbing 
> the m_stringsLock lock first. This also allows to compiler to know that the 
> addString() implementation can modify the m_strings data member without 
> grabbing the lock first.
> 
> Another good use case:
> 
> static Lock globalCacheLock;
> static HashMap& globalCache() 
> WTF_REQUIRES_LOCK(globalCacheLock)
> {
> static NeverDestroyed> globalCache;
> return globalCache;
> }
> This will force all call sites to grab the globalCacheLock lock before 
> calling globalCache().
> 
> Previously, we may have passed a LockHolder& as parameter to try and convey 
> that. We have been updating code to stop passing such parameters though as it 
> is no longer useful.
> 
> WTF_ACQUIRES_LOCK() / WTF_RELEASES_LOCK()
> 
> WTF_ACQUIRES_LOCK is an attribute on functions or methods declaring that the 
> function acquires a lock, but does not release it. The given lock must not be 
> held on entry, and will be held on exit.
> 
> WTF_RELEASES_LOCK declares that the function releases the given lock. The 
> lock must be held on entry, and will no longer be held on exit.
> 
> class Foo {
> public:
> void suspend() WTF_ACQUIRES_LOCK(m_suspensionLock)
> {
> m_suspensionLock.lock();
> m_isSuspended = true;
> }
> void resume() WTF_RELEASE_LOCK(m_suspensionLock)
> {
> m_isSuspended = false;
> m_suspensionLock.unlock();
> }
> 
> private:
> Lock m_suspensionLock;
> bool m_isSuspended;
> };
> Without these annotations, you'd get a compiler error stating that you failed 
> to unlock m_suspensionLock before returning in suspend() and that you 
> unlocked m_suspensionLock in resume() even though it was not previously 
> locked.
> 
> WTF_RETURNS_LOCK()
> 
> WTF_RETURNS_LOCK is an attribute on functions or methods, which declares that 
> the function returns a reference to the given lock.
> 
> class Foo {
> public:
> Lock& lock() WTF_RETURNS_LOCK(m_lock) { return m_lock; }
> private:
> Lock m_lock;
> };
> WTF_EXCLUDES_LOCK()
> 
> WTF_EXCLUDES_LOCK is an attribute on functions or methods, which declares 
> that the caller must not hold the given lock. This annotation is used to 
> prevent deadlock. Many mutex implementations are not re-entrant, so deadlock 
> can occur if the function acquires the mutex a second time.
> 
> class Foo {
> void addString(String&& string) WTF_EXCLUDES_LOCK(m_stringsLock)
> {
> Locker locker { m_stringsLock };
> m_string.append(WTFMove(string));
>

[webkit-dev] Clang Thread Safety Analysis

[Chris Dumez via webkit-dev]

Clang Thread Safety Analysis
WTF::Lock now supports Clang Thread Safety Analysis 
. It is a C++ language 
extension which warns about potential race conditions in code, at compile time. 
It is the same great Lock, but with some extra help from clang to make our 
multi-threaded code safer by giving out compilation errors when thread unsafe 
code is detected.

Annotations
Just by using WTF::Lock, Clang will already be able to do some basic 
validation. However, to take full advantage of it, there are a few annotations 
you should know about. Note that you'll see those annotations throughout the 
code base already as a few of us have been busy adopting them. All these 
annotations are declared in wtf/ThreadSafetyAnalysis.h 
.

WTF_GUARDED_BY_LOCK()

WTF_GUARDED_BY_LOCK is an attribute on data members, which declares that the 
data member is protected by the given lock. Read operations on the data require 
shared access, while write operations require exclusive access.

WTF_POINTEE_GUARDED_BY_LOCK is similar, but is intended for use on pointers and 
smart pointers. There is no constraint on the data member itself, but the data 
that it points to is protected by the given lock.

class Foo {
Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
Lock m_stringsLock;
};
You will get a compiler error if you try to access or modify m_strings without 
grabbing the m_stringsLock lock first.

WTF_REQUIRES_LOCK()

WTF_REQUIRES_LOCK is an attribute on functions or methods, which declares that 
the calling thread must have exclusive access to the given locks. More than one 
lock may be specified. The locks must be held on entry to the function, and 
must still be held on exit.

class Foo {
void addString(String&&) WTF_REQUIRES_LOCK(m_stringsLock);

Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
Lock m_stringsLock;
};
You will get a compiler error if you try to call addString() without grabbing 
the m_stringsLock lock first. This also allows to compiler to know that the 
addString() implementation can modify the m_strings data member without 
grabbing the lock first.

Another good use case:

static Lock globalCacheLock;
static HashMap& globalCache() WTF_REQUIRES_LOCK(globalCacheLock)
{
static NeverDestroyed> globalCache;
return globalCache;
}
This will force all call sites to grab the globalCacheLock lock before calling 
globalCache().

Previously, we may have passed a LockHolder& as parameter to try and convey 
that. We have been updating code to stop passing such parameters though as it 
is no longer useful.

WTF_ACQUIRES_LOCK() / WTF_RELEASES_LOCK()

WTF_ACQUIRES_LOCK is an attribute on functions or methods declaring that the 
function acquires a lock, but does not release it. The given lock must not be 
held on entry, and will be held on exit.

WTF_RELEASES_LOCK declares that the function releases the given lock. The lock 
must be held on entry, and will no longer be held on exit.

class Foo {
public:
void suspend() WTF_ACQUIRES_LOCK(m_suspensionLock)
{
m_suspensionLock.lock();
m_isSuspended = true;
}
void resume() WTF_RELEASE_LOCK(m_suspensionLock)
{
m_isSuspended = false;
m_suspensionLock.unlock();
}

private:
Lock m_suspensionLock;
bool m_isSuspended;
};
Without these annotations, you'd get a compiler error stating that you failed 
to unlock m_suspensionLock before returning in suspend() and that you unlocked 
m_suspensionLock in resume() even though it was not previously locked.

WTF_RETURNS_LOCK()

WTF_RETURNS_LOCK is an attribute on functions or methods, which declares that 
the function returns a reference to the given lock.

class Foo {
public:
Lock& lock() WTF_RETURNS_LOCK(m_lock) { return m_lock; }
private:
Lock m_lock;
};
WTF_EXCLUDES_LOCK()

WTF_EXCLUDES_LOCK is an attribute on functions or methods, which declares that 
the caller must not hold the given lock. This annotation is used to prevent 
deadlock. Many mutex implementations are not re-entrant, so deadlock can occur 
if the function acquires the mutex a second time.

class Foo {
void addString(String&& string) WTF_EXCLUDES_LOCK(m_stringsLock)
{
Locker locker { m_stringsLock };
m_string.append(WTFMove(string));
}
Vector m_strings WTF_GUARDED_BY_LOCK(m_stringsLock);
Lock m_stringsLock;
};
WTF_IGNORES_THREAD_SAFETY_ANALYSIS

WTF_IGNORES_THREAD_SAFETY_ANALYSIS is an attribute on functions or methods, 
which turns off thread safety checking for that method. It provides an escape 
hatch for functions which are either (1) deliberately thread-unsafe, or (2) are 
thread-safe, but too complicated for the analysis to understand. Reasons for 
(2) are be described in the Known Limitations 
.

class Foo {
void unsafeI