Re: [webkit-dev] [Styling] () for a lambda without arguments (Was Space between [] and () in C++ lambdas)

2019-11-02 Thread Chris Dumez


> On Nov 2, 2019, at 7:38 PM, Ryosuke Niwa  wrote:
> 
> 
> 
>> On Sat, Nov 2, 2019 at 1:23 AM Antti Koivisto  wrote:
>> 
>>> On Sat, Nov 2, 2019 at 1:38 AM Ryosuke Niwa  wrote:
 On Fri, Nov 1, 2019 at 11:53 AM Michael Catanzaro  
 wrote:
>>> 
 On Fri, Nov 1, 2019 at 11:19 am, Ryosuke Niwa  wrote:
 > Namely, some people write a lambda as:
 > auto x = [] () { }
 > 
 > with a space between [] and () while others would write it as:
 > 
 > auto x = []() { }
 
 : I omit the () when there are no parameters, as in these examples.
>>> 
>>> I guess that's another thing we should decide. Should we, or should we not 
>>> have () when there are no arguments.
>> 
>> I think this is easily settled by voting via exiting practice. We have 1287 
>> instances of [&] { and 107 instances of [&]() { and &] () { across the whole 
>> WebKit.
> 
> That’s good to know. Why don’t we go with the status quo then.
> 
> In this case, we do put a space between ] or ) and {, right?

How is this the conclusion from Antti’s comment?

Based on the discussion so far, it thought no space had a slight lead.

> 
> I guess this is also consistent with the way people write objective C blocks: 
> https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/WorkingwithBlocks/WorkingwithBlocks.html
> 
> For JavaScript, this rule probably doesn’t apply because arrow function and 
> regular anonymous function both require ().
> 
> - 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] [Styling] Space between [] and () in C++ lambdas

2019-11-01 Thread Chris Dumez
I mildly prefer without the space :)

--
 Chris Dumez




> On Nov 1, 2019, at 11:19 AM, Ryosuke Niwa  wrote:
> 
> Hi all,
> 
> There seems to be inconsistency in our coding style with regards to spacing 
> in lambdas.
> 
> Namely, some people write a lambda as:
> auto x = [] () { }
> 
> with a space between [] and () while others would write it as:
> 
> auto x = []() { }
> 
> without a space between the two. I'd like to require either style in our 
> guideline so that things are consistent in our codebase. Which one should we 
> use?
> 
> FWIW, I mildly prefer having a space between [] and ().
> 
> - 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] 2019 WebKit Contributors Meeting - Registration is Open

2019-09-18 Thread Chris Dumez
Did not work for me either, got a page with text “None” shortly after logging 
in.

--
 Chris Dumez




> On Sep 18, 2019, at 2:12 PM,  
>  wrote:
> 
> Seems like the login form isn't working—I see:
> 
>   MultipleObjectsReturned at /auth/wlogin/
>   get() returned more than one MyUser -- it returned 2!
> 
> Perhaps I just need to be more patient? :)
> 
> Thanks,
> Ross
> 
> On 9/18/19, 1:49 PM, "webkit-dev on behalf of Jonathan Davis" 
>  wrote:
> 
>Hello WebKit Contributors,
> 
>You are invited to attend the annual WebKit Contributors Meeting to be 
> held at the Hilton Santa Clara hotel on Friday, November 1st, 2019 from 8 AM 
> to 6 PM Pacific.
> 
>Breakfast and sign-in will begin at 8 AM. Presentations will run between 9 
> AM to 6 PM. Lunch will be provided with all day coffee, tea and a 
> mid-afternoon snack break. A reception is planned at the venue after the 
> meeting with dinner and drinks from 6 PM to 9 PM.
> 
>To attend you must be an active WebKit contributor. The meeting will be 
> free of charge, and registration is open. Register at 
> https://webkit.org/meeting/ 
> 
>The event format will include a mix of prepared talks around 40 minutes 
> long with 10-15 minutes of questions, and spontaneous lightning talks about 
> 5-10 minutes long. If you have a topic idea that you want to present, please 
> email me directly.
> 
>We look forward to seeing you there!
> 
>Thank you,
>Jon Davis
>___
>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] IPC implementation help in haiku's webkit port

2019-05-28 Thread Chris Dumez
Alex is likely right that you are ending up with some kind of IPC deadlock 
since the IPC is synchronous.

But also,

> On May 27, 2019, at 11:49 PM, Rajagopalan Gangadharan  
> wrote:
> 
> 
> Please check the source references for more info:
> NetworkProcessProxy: [ 
> https://github.com/RAJAGOPALAN-GANGADHARAN/webkit/blob/6bf81d79669be06b4efd9d8ced4139cbe07216b2/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp#L150
>  
> ]

You’re passing 0 instead of the webPID with the IPC here.___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Crash When Running run-safari

2019-04-10 Thread Chris Dumez
Bug 196777 <https://bugs.webkit.org/show_bug.cgi?id=196777> will fix this.

--
 Chris Dumez




> On Apr 10, 2019, at 10:54 AM, Chris Dumez  wrote:
> 
> Yes, this commit broke it: http://trac.webkit.org/r243320 
> <http://trac.webkit.org/r243320>
> 
> --
>  Chris Dumez
> 
> 
> 
> 
>> On Apr 10, 2019, at 10:47 AM, Assaf Inbal > <mailto:shmuel...@gmail.com>> wrote:
>> 
>> Thanks for the update!
>> Since I don’t really need the bleeding-edge, can you tell me which 
>> tag/commit of WebKit I can use which should be compatible?
>> 
>> Thanks in advance!
>> 
>> On 10 Apr 2019, at 20:36, Chris Dumez > <mailto:cdu...@apple.com>> wrote:
>> 
>>> Oh, I am responsible for this. This is due to an incompatibility between 
>>> the very recent WebKit you built and the older system’s Safari.
>>> I will look into fixing this shortly, thanks for letting me know.
>>> 
>>> --
>>>  Chris Dumez
>>> 
>>> 
>>> 
>>> 
>>>> On Apr 10, 2019, at 4:44 AM, Assaf Inbal >>> <mailto:shmuel...@gmail.com>> wrote:
>>>> 
>>>> Hey,
>>>> 
>>>> I’m trying to build and run (top-of-branch) WebKit to debug some weird 
>>>> behaviour and am running into some issues.
>>>> I compiled everything on macOS with `./Tools/Scripts/build-webkit --debug` 
>>>> which seemed to work just find but when I go ahead and run Safari via 
>>>> `./Tools/Scripts/run-safari --debug` I get an exception and the program 
>>>> exits:
>>>> 2019-04-10 14:34:38.256 SafariForWebKitDevelopment[644:9602] *** 
>>>> Terminating app due to uncaught exception 'NSInvalidArgumentException', 
>>>> reason: 'Related web view  has data store 
>>>>  but configuration specifies a 
>>>> different data store '
>>>> Not sure what this means exactly and couldn’t find anything similar online.
>>>> 
>>>> Any help would be appreciated.
>>>> 
>>>> Thanks!
>>>> 
>>>> ___
>>>> webkit-dev mailing list
>>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>>>> <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] Crash When Running run-safari

2019-04-10 Thread Chris Dumez
Yes, this commit broke it: http://trac.webkit.org/r243320 
<http://trac.webkit.org/r243320>

--
 Chris Dumez




> On Apr 10, 2019, at 10:47 AM, Assaf Inbal  wrote:
> 
> Thanks for the update!
> Since I don’t really need the bleeding-edge, can you tell me which tag/commit 
> of WebKit I can use which should be compatible?
> 
> Thanks in advance!
> 
> On 10 Apr 2019, at 20:36, Chris Dumez  <mailto:cdu...@apple.com>> wrote:
> 
>> Oh, I am responsible for this. This is due to an incompatibility between the 
>> very recent WebKit you built and the older system’s Safari.
>> I will look into fixing this shortly, thanks for letting me know.
>> 
>> --
>>  Chris Dumez
>> 
>> 
>> 
>> 
>>> On Apr 10, 2019, at 4:44 AM, Assaf Inbal >> <mailto:shmuel...@gmail.com>> wrote:
>>> 
>>> Hey,
>>> 
>>> I’m trying to build and run (top-of-branch) WebKit to debug some weird 
>>> behaviour and am running into some issues.
>>> I compiled everything on macOS with `./Tools/Scripts/build-webkit --debug` 
>>> which seemed to work just find but when I go ahead and run Safari via 
>>> `./Tools/Scripts/run-safari --debug` I get an exception and the program 
>>> exits:
>>> 2019-04-10 14:34:38.256 SafariForWebKitDevelopment[644:9602] *** 
>>> Terminating app due to uncaught exception 'NSInvalidArgumentException', 
>>> reason: 'Related web view  has data store 
>>>  but configuration specifies a 
>>> different data store '
>>> Not sure what this means exactly and couldn’t find anything similar online.
>>> 
>>> Any help would be appreciated.
>>> 
>>> Thanks!
>>> 
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>>> <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] Crash When Running run-safari

2019-04-10 Thread Chris Dumez
Oh, I am responsible for this. This is due to an incompatibility between the 
very recent WebKit you built and the older system’s Safari.
I will look into fixing this shortly, thanks for letting me know.

--
 Chris Dumez




> On Apr 10, 2019, at 4:44 AM, Assaf Inbal  wrote:
> 
> Hey,
> 
> I’m trying to build and run (top-of-branch) WebKit to debug some weird 
> behaviour and am running into some issues.
> I compiled everything on macOS with `./Tools/Scripts/build-webkit --debug` 
> which seemed to work just find but when I go ahead and run Safari via 
> `./Tools/Scripts/run-safari --debug` I get an exception and the program exits:
> 2019-04-10 14:34:38.256 SafariForWebKitDevelopment[644:9602] *** Terminating 
> app due to uncaught exception 'NSInvalidArgumentException', reason: 'Related 
> web view  has data store  0x7fbc872856e0> but configuration specifies a different data store 
> '
> Not sure what this means exactly and couldn’t find anything similar online.
> 
> Any help would be appreciated.
> 
> Thanks!
> 
> ___
> 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] Code Style: Opinion on returning void

2019-02-20 Thread Chris Dumez

> On Feb 20, 2019, at 1:14 PM, Maciej Stachowiak  wrote:
> 
> 
> It seems like `return foo();` where foo() is a void function can always be 
> replaced with `foo(); return;` for greater clarity at the cost of one extra 
> line break. For people who prefer the one-line style, can you say why you 
> don’t like the other way?

We do not allow more than one statement per line so it would be:
foo();
return;

Also, since we favor early returns in WebKit, things like:
If (!nok)
return completionHandler(Failed);

Would become:
If (!nok) {
completionHandler(Failed);
return;
}

It is not a big deal but I personally prefer the most concise version. 
Especially, it is not uncommon to have multiple early returns.
I think more concise is better and I personally do not see a readability issue 
here. It does not really matter what the completion handler is returning.

> 
>  - Maciej
> 
>> On Feb 20, 2019, at 10:33 AM, Simon Fraser > <mailto:simon.fra...@apple.com>> wrote:
>> 
>> I find it mind bending. It makes me wonder if the author made a coding error.
>> 
>> Simon
>> 
>>> On Feb 20, 2019, at 7:48 AM, Daniel Bates >> <mailto:dba...@webkit.org>> wrote:
>>> 
>>> Thanks for the opinion!
>>> 
>>> Dan
>>> 
>>> On Feb 20, 2019, at 7:26 AM, Saam Barati >> <mailto:sbar...@apple.com>> wrote:
>>> 
>>>> I prefer it as well.
>>>> 
>>>> - Saam
>>>> 
>>>> On Feb 20, 2019, at 6:58 AM, Chris Dumez >>> <mailto:cdu...@apple.com>> wrote:
>>>> 
>>>>> I also prefer allowed returning void. 
>>>>> 
>>>>> Chris Dumez
>>>>> 
>>>>> On Feb 19, 2019, at 10:35 PM, Daniel Bates >>>> <mailto:dba...@webkit.org>> wrote:
>>>>> 
>>>>>> 
>>>>>> 
>>>>>> On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa >>>>> <mailto:rn...@webkit.org>> wrote:
>>>>>> 
>>>>>>> On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates >>>>>> <mailto:dba...@webkit.org>> wrote:
>>>>>>> > On Feb 7, 2019, at 12:47 PM, Daniel Bates >>>>>> > <mailto:dba...@webkit.org>> 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();
>>>>>>> > }
>>>>>>> >
>>>>>>> Based on the responses it seems there is sufficient leaning to codify
>>>>>>> the latter style.
>>>>>>> 
>>>>>>> I don't think there is a sufficient consensus as far as I can tell. 
>>>>>>> Geoff
>>>>>> 
>>>>>> I didn't get this from Geoff's remark. Geoff wrote:
>>>>>> 
>>>>>> ***“return f()” when f returns void is a bit mind bending.***
>>>>>> Don't want to put words in Geoff's mouth. So, Geoff can you please 
>>>>>> confirm: for the former style, for the latter style, no strong opinion.
>>>>>> 
>>>>>>> and Alex both expressed preferences for being able to return void,
>>>>>> 
>>>>>> I got this from Alex's message
>>>>>> 
>>>>>>> and Saam pointed out that there is a lot of existing code which does 
>>>>>>> this.
>>>>>> 
>>>>>> I did not get this. He wrote emphasis mine:
>>>>>> 
>>>>>> I've definitely done this in JSC. ***I don't think it's super common***, 
>>>>>> but I've also seen code in JSC not written by me that also does this.
>>>>>> 
>>>>>>> Zalan also said he does th

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

2019-02-20 Thread Chris Dumez

> On Feb 20, 2019, at 7:48 AM, Daniel Bates  wrote:
> 
> Okay, you’ve changed your mind from your earlier email of not having a strong 
> opinion. Would have been good to know from the get-go. Better late than never 
> knowing :/

I did not change my mind. I said I was using this pattern in my code. So did 
other people. If we use it in our code, it is because we prefer it.
What I meant to say is that if a majority of people felt strongly that we 
should not allow this pattern, then I would not stand in the way.

I don’t think this mail thread showed that people strongly feel that we should 
not allow this pattern. Therefore, I was also surprised by your email saying 
that we’d reached a consensus.

> 
> Dan
> 
> On Feb 20, 2019, at 6:58 AM, Chris Dumez  <mailto:cdu...@apple.com>> wrote:
> 
>> I also prefer allowed returning void. 
>> 
>> Chris Dumez
>> 
>> On Feb 19, 2019, at 10:35 PM, Daniel Bates > <mailto:dba...@webkit.org>> wrote:
>> 
>>> 
>>> 
>>> On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa >> <mailto:rn...@webkit.org>> wrote:
>>> 
>>>> On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates >>> <mailto:dba...@webkit.org>> wrote:
>>>> > On Feb 7, 2019, at 12:47 PM, Daniel Bates >>> > <mailto:dba...@webkit.org>> 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();
>>>> > }
>>>> >
>>>> Based on the responses it seems there is sufficient leaning to codify
>>>> the latter style.
>>>> 
>>>> I don't think there is a sufficient consensus as far as I can tell. Geoff
>>> 
>>> I didn't get this from Geoff's remark. Geoff wrote:
>>> 
>>> ***“return f()” when f returns void is a bit mind bending.***
>>> Don't want to put words in Geoff's mouth. So, Geoff can you please confirm: 
>>> for the former style, for the latter style, no strong opinion.
>>> 
>>>> and Alex both expressed preferences for being able to return void,
>>> 
>>> I got this from Alex's message
>>> 
>>>> and Saam pointed out that there is a lot of existing code which does this.
>>> 
>>> I did not get this. He wrote emphasis mine:
>>> 
>>> I've definitely done this in JSC. ***I don't think it's super common***, 
>>> but I've also seen code in JSC not written by me that also does this.
>>> 
>>>> Zalan also said he does this in his layout code.
>>> 
>>> I did not get this, quoting, emphasis mine:
>>> 
>>> 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)***
>>> 
>>> By the way, you even acknowledged that "WebKit ... tend[s] to have a 
>>> separate return.". So, I inferred you were okay with it. But from this 
>>> email I am no longer sure what your position is. Please state it clearly.
>>> 
>>>> - R. Niwa
>>>> 
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>>> <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] Code Style: Opinion on returning void

2019-02-20 Thread Chris Dumez
I also prefer allowed returning void. 

Chris Dumez

> On Feb 19, 2019, at 10:35 PM, Daniel Bates  wrote:
> 
> 
> 
>> On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa  wrote:
>> 
>>> On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates  wrote:
>> 
>>> > 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();
>>> > }
>>> >
>>> Based on the responses it seems there is sufficient leaning to codify
>>> the latter style.
>> 
>> I don't think there is a sufficient consensus as far as I can tell. Geoff
> 
> I didn't get this from Geoff's remark. Geoff wrote:
> 
> ***“return f()” when f returns void is a bit mind bending.***
> Don't want to put words in Geoff's mouth. So, Geoff can you please confirm: 
> for the former style, for the latter style, no strong opinion.
> 
>> and Alex both expressed preferences for being able to return void,
> 
> I got this from Alex's message
> 
>> and Saam pointed out that there is a lot of existing code which does this.
> 
> I did not get this. He wrote emphasis mine:
> 
> I've definitely done this in JSC. ***I don't think it's super common***, but 
> I've also seen code in JSC not written by me that also does this.
> 
>> Zalan also said he does this in his layout code.
> 
> I did not get this, quoting, emphasis mine:
> 
> 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)***
> 
> By the way, you even acknowledged that "WebKit ... tend[s] to have a separate 
> return.". So, I inferred you were okay with it. But from this email I am no 
> longer sure what your position is. Please state it clearly.
> 
>> - 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] Code Style: Opinion on returning void

2019-02-07 Thread Chris Dumez
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.

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


Re: [webkit-dev] Rename AtomicString to AtomString

2019-01-30 Thread Chris Dumez
Fine by me.

--
 Chris Dumez




> On Jan 30, 2019, at 9:35 AM, Antti Koivisto  wrote:
> 
> Sounds good to me.
> 
> 
>   antti
> 
> On Wed, Jan 30, 2019 at 6:33 PM Darin Adler  <mailto:da...@apple.com>> wrote:
> So, did we reach consensus that we should rename AtomicString to AtomString?
> 
> — Darin
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> <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] the name "AtomicString"

2018-12-19 Thread Chris Dumez


> On Dec 19, 2018, at 9:41 PM, Chris Dumez  wrote:
> 
>> 
>> On Dec 19, 2018, at 9:17 PM, Maciej Stachowiak > <mailto:m...@apple.com>> wrote:
>> 
>> 
>> 
>>> On Dec 19, 2018, at 8:06 PM, Ryosuke Niwa >> <mailto:rn...@webkit.org>> wrote:
>>> 
>>> On Wed, Dec 19, 2018 at 1:13 PM Simon Fraser >> <mailto:simon.fra...@apple.com>> wrote:
>>> > On Dec 19, 2018, at 12:33 PM, Michael Catanzaro >> > <mailto:mcatanz...@igalia.com>> wrote:
>>> > 
>>> > On Tue, Dec 18, 2018 at 9:31 PM, Darin Adler >> > <mailto:da...@apple.com>> wrote:
>>> >> I’ve gotten used to the name AtomicString over the years, but I wouldn’t 
>>> >> strongly object to changing it if other programmers are often confused 
>>> >> by it’s similarity to the term “atomic operations”.
>>> > 
>>> > Well there were two other developers in the thread Ryosuke linked to who 
>>> > made the exact same mistake as me, so I do think the current name is 
>>> > problematic. A change wouldn't need to be drastic, though. I think 
>>> > suggestions from the old thread like "StringAtom" or "AtomString" would 
>>> > be unproblematic. The problem is the specific word "atomic" carries an 
>>> > expectation that the object be safe to access concurrently across threads 
>>> > without locks; I think that expectation doesn't exist if not for the "ic" 
>>> > at the end.
>>> > 
>>> > FWIW I've only ever heard the "interned string" terminology prior to now.
>>> 
>>> SingletonString?
>>> UniquedString?
>>> 
>>> I do like UniquedString. That conveys what AtomicString really is. 
>>> SingletonString isn't so great since AtomicString table is still per thread.
>> 
>> So hard to pronounce though! Why not UniqueString? It’s not quite as 
>> explicit but close enough. 
> 
> Wouldn’t it be confusing to use UniqueString type for a string that is 
> *common* in order to save memory?
> 
> Personally, I like the AtomString proposal as it is close to the naming we 
> are used to and addresses the issue raised (atomic has a different meaning 
> with threading).

Or I also like “AtomizedString”.

> Also, I had never heard of interned strings before.
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> <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] the name "AtomicString"

2018-12-19 Thread Chris Dumez

> On Dec 19, 2018, at 9:17 PM, Maciej Stachowiak  wrote:
> 
> 
> 
>> On Dec 19, 2018, at 8:06 PM, Ryosuke Niwa > > wrote:
>> 
>> On Wed, Dec 19, 2018 at 1:13 PM Simon Fraser > > wrote:
>> > On Dec 19, 2018, at 12:33 PM, Michael Catanzaro > > > wrote:
>> > 
>> > On Tue, Dec 18, 2018 at 9:31 PM, Darin Adler > > > wrote:
>> >> I’ve gotten used to the name AtomicString over the years, but I wouldn’t 
>> >> strongly object to changing it if other programmers are often confused by 
>> >> it’s similarity to the term “atomic operations”.
>> > 
>> > Well there were two other developers in the thread Ryosuke linked to who 
>> > made the exact same mistake as me, so I do think the current name is 
>> > problematic. A change wouldn't need to be drastic, though. I think 
>> > suggestions from the old thread like "StringAtom" or "AtomString" would be 
>> > unproblematic. The problem is the specific word "atomic" carries an 
>> > expectation that the object be safe to access concurrently across threads 
>> > without locks; I think that expectation doesn't exist if not for the "ic" 
>> > at the end.
>> > 
>> > FWIW I've only ever heard the "interned string" terminology prior to now.
>> 
>> SingletonString?
>> UniquedString?
>> 
>> I do like UniquedString. That conveys what AtomicString really is. 
>> SingletonString isn't so great since AtomicString table is still per thread.
> 
> So hard to pronounce though! Why not UniqueString? It’s not quite as explicit 
> but close enough. 

Wouldn’t it be confusing to use UniqueString type for a string that is *common* 
in order to save memory?

Personally, I like the AtomString proposal as it is close to the naming we are 
used to and addresses the issue raised (atomic has a different meaning with 
threading).
Also, I had never heard of interned strings before.___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Watch out for std::optional's move constructor

2018-12-17 Thread Chris Dumez

> On Dec 17, 2018, at 3:55 PM, Alex Christensen  wrote:
> 
> Let me give a concrete example on why, even with our nice-to-use WTF types, 
> the state of a C++ object is undefined after being moved from:
> 
> #include 
> #include 
> #include 
> 
> class Test : public RefCounted { };
> 
> void useParameter(RefPtr&& param)
> {
>   RefPtr usedParam = WTFMove(param);
> }
> 
> void dontUseParameter(RefPtr&&) { }
> 
> int main() {
>   RefPtr a = adoptRef(new Test);
>   RefPtr b = adoptRef(new Test);
>   std::cout << "a null? " << !a << std::endl;
>   std::cout << "b null? " << !b << std::endl;
>   useParameter(WTFMove(a));
>   dontUseParameter(WTFMove(b));
>   std::cout << "a null? " << !a << std::endl;
>   std::cout << "b null? " << !b << std::endl;
>   return 0;
> }
> 
> // clang++ test.cpp -I Source/WTF -L WebKitBuild/Debug -l WTF -framework 
> Foundation -L /usr/lib -l icucore --std=c++17 && ./a.out  
>  
> // a null? 0  
>   
> // b null? 0  
>   
> // a null? 1  
>   
> // b null? 0  
>  
> 
> 
> As you can see, the internals of callee dontUseParameter (which could be in a 
> different translation unit) affects the state of the local variable b in this 
> function.  This is one of the reasons why the state of a moved-from variable 
> is intentionally undefined, and we can’t fix that by using our own 
> std::optional replacement.  If we care about the state of a moved-from 
> object, that is what std::exchange is for.  I think we should do something to 
> track and prevent the use of moved-from values instead of introducing our own 
> std::optional replacement.

Yes, this is a good example of case where std::exchange should be used or where 
the parameter should be a RefPtr and not a RefPtr&&. But as I 
mentioned earlier, the issue here is that the caller of WTFMove() is not 
actually calling any move constructor (it is merely doing a cast) and therefore 
cannot rely on the effects or a move constructor.
I do not think that this example is a good reason why we would not want 
optional to have a more predictable move constructor.


> 
>> On Dec 17, 2018, at 2:47 PM, Ryosuke Niwa > <mailto:rn...@webkit.org>> wrote:
>> 
>> Yeah, it seems like making std::optional more in line with our own 
>> convention provides more merits than downsides here. People are using 
>> WTFMove as if it's some sort of a swap operation in our codebase, and as 
>> Maciej pointed out, having rules where people have to think carefully as to 
>> when & when not to use WTFMove seems more troublesome than the proposed fix, 
>> which would mean this work for optional.
>> 
>> - R. Niwa
>> 
>> On Mon, Dec 17, 2018 at 2:24 PM Geoffrey Garen > <mailto:gga...@apple.com>> wrote:
>> I don’t understand the claim about “undefined behavior” here. As Maciej 
>> pointed out, these are our libraries. We are free to define their behaviors.
>> 
>> In general, “undefined behavior” is an unwanted feature of programming 
>> languages and libraries, which we accept begrudgingly simply because there 
>> are practical limits to what we can define. This acceptance is not a mandate 
>> to carry forward undefined-ness as a badge of honor. In any case where it 
>> would be practical to define a behavior, that defined behavior would be 
>> preferable to undefined behavior.
>> 
>> I agree that the behavior of move constructors in the standard library is 
>> undefined. The proposal here, as I understand it, is to (a) define the 
>> behaviors move constructors in WebKit and (b) avoid std::optional and use an 
>> optional class with well-defined behavior instead.
>> 
>> Because I do not ❤️ security updates, I do ❤️ defined behavior, and so I ❤️ 
>> this proposal.
>> 
>> Geoff
>> 
>>> On Dec 17, 2018, at 12:50 PM, Alex Christensen >> <mailto:achristen...@apple.com>> wrote:
>>> 
>>> This one and the many others like it ar

Re: [webkit-dev] Watch out for std::optional's move constructor

2018-12-17 Thread Chris Dumez


> On Dec 17, 2018, at 11:10 AM, Chris Dumez  wrote:
> 
> 
> 
>> On Dec 17, 2018, at 10:27 AM, Alex Christensen > <mailto:achristen...@apple.com>> wrote:
>> 
>>>>>> On Dec 14, 2018, at 1:37 PM, Chris Dumez >>>>> <mailto:cdu...@apple.com>> wrote:
>>>>> 
>>>>>> 
>>>>>> As far as I know, our convention in WebKit so far for our types has been 
>>>>>> that types getting moved-out are left in a valid “empty” state.
>> This is not necessarily true.  When we move out of an object to pass into a 
>> function parameter, for example, the state of the moved-from object depends 
>> on the behavior of the callee.  If the callee function uses the object, we 
>> often have behavior that leaves the object in an “empty” state of some kind, 
>> but we are definitely relying on fragile undefined behavior when we do so 
>> because changing the callee to not use the parameter changes the state of 
>> the caller.  We should never assume that WTFMove or std::move leaves the 
>> object in an empty state.  That is always a bug that needs to be replaced by 
>> std::exchange.
> 
> Feel like we’re taking about different things. I am talking about move 
> constructors (and assignment operators), which have a well defined behavior 
> in WebKit. And it seems you are talking about WTFMove(), which despite the 
> name does not “move” anything, it is merely a cast.
> In the case you’re talking about the caller does NOT call the move 
> constructor, it merely does a cast so I do not think your comment invalidates 
> my statement. Note that in my patch, I was nearly WTFMove()ing the data 
> member and assigning it to a local variable right away, calling the move 
> constructor.

Also note that may of us already rely on our move constructors’ behavior, just 
search for WTFMove(m_responseCompletionHandler) in:
https://trac.webkit.org/changeset/236463/webkit___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Watch out for std::optional's move constructor

2018-12-17 Thread Chris Dumez


> On Dec 17, 2018, at 10:27 AM, Alex Christensen  wrote:
> 
>>>>> On Dec 14, 2018, at 1:37 PM, Chris Dumez >>>> <mailto:cdu...@apple.com>> wrote:
>>>> 
>>>>> 
>>>>> As far as I know, our convention in WebKit so far for our types has been 
>>>>> that types getting moved-out are left in a valid “empty” state.
> This is not necessarily true.  When we move out of an object to pass into a 
> function parameter, for example, the state of the moved-from object depends 
> on the behavior of the callee.  If the callee function uses the object, we 
> often have behavior that leaves the object in an “empty” state of some kind, 
> but we are definitely relying on fragile undefined behavior when we do so 
> because changing the callee to not use the parameter changes the state of the 
> caller.  We should never assume that WTFMove or std::move leaves the object 
> in an empty state.  That is always a bug that needs to be replaced by 
> std::exchange.

Feel like we’re taking about different things. I am talking about move 
constructors (and assignment operators), which have a well defined behavior in 
WebKit. And it seems you are talking about WTFMove(), which despite the name 
does not “move” anything, it is merely a cast.
In the case you’re talking about the caller does NOT call the move constructor, 
it merely does a cast so I do not think your comment invalidates my statement. 
Note that in my patch, I was nearly WTFMove()ing the data member and assigning 
it to a local variable right away, calling the move constructor.


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


Re: [webkit-dev] Watch out for std::optional's move constructor

2018-12-16 Thread Chris Dumez

> On Dec 16, 2018, at 7:43 PM, Fujii Hironori  wrote:
> 
> I don't like the proposal because it encourages misuse of move.
> We can use move only for values about to be destroyed.

Just for reference, there are close to 400 matches for "WTFMove(m_” in our code 
base. People do seem to rely on the state of objects after being moved out.
I totally agree that the state of the object being moved out is not defined by 
the C++ standard. However, so far, in WebKit, we’ve been careful with our 
move-constructors.

I think that if we do not update std::optional’s move constructor, then I worry 
we’ll keep having to fix bugs in the future due to its misuse. Although, maybe 
this mail thread will help.

That being said, I agree with your and Daniel and we should use std::exchange 
more. I think all the "WTFMove(m_” lines in our code bases should probably be 
replaced with std::exchange.


> 
> I like Dan's suggestion. We should use std::exchange or std::optional::swap 
> for the cases.
> Or, what about adding a new method WTF::Optional::release() for the case?
> 
> ___
> 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] Watch out for std::optional's move constructor

2018-12-14 Thread Chris Dumez
So to be clear, it is often not truly about using the value after it is moved. 
It is about expecting that the variable / member has been nulled out after 
moving it.
If I WTFMove() out a data member m_dataMember, I expect later on `if 
(m_dataMember)` to be false.

--
 Chris Dumez




> On Dec 14, 2018, at 3:45 PM, Chris Dumez  wrote:
> 
> 
>> On Dec 14, 2018, at 3:39 PM, Fujii Hironori > <mailto:fujii.hiron...@gmail.com>> wrote:
>> 
>> 
>> On Sat, Dec 15, 2018 at 6:38 AM Chris Dumez > <mailto:cdu...@apple.com>> wrote:
>> 
>> I have now been caught twice by std::optional’s move constructor. 
>> 
>>  I don't understand how this could be useful? Do you want to use the value 
>> after it is moved? I'd like to see these your code. Could you show me these 
>> two patches?
> 
> This is the latest one: https://trac.webkit.org/changeset/239228/webkit 
> <https://trac.webkit.org/changeset/239228/webkit>
> 
> 
> ___
> 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] Watch out for std::optional's move constructor

2018-12-14 Thread Chris Dumez

> On Dec 14, 2018, at 3:39 PM, Fujii Hironori  wrote:
> 
> 
> On Sat, Dec 15, 2018 at 6:38 AM Chris Dumez  <mailto:cdu...@apple.com>> wrote:
> 
> I have now been caught twice by std::optional’s move constructor. 
> 
>  I don't understand how this could be useful? Do you want to use the value 
> after it is moved? I'd like to see these your code. Could you show me these 
> two patches?

This is the latest one: https://trac.webkit.org/changeset/239228/webkit


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


Re: [webkit-dev] Watch out for std::optional's move constructor

2018-12-14 Thread Chris Dumez

> On Dec 14, 2018, at 1:56 PM, Saam Barati  wrote:
> 
> 
> 
>> On Dec 14, 2018, at 1:54 PM, Saam Barati > <mailto:sbar...@apple.com>> wrote:
>> 
>> 
>> 
>>> On Dec 14, 2018, at 1:37 PM, Chris Dumez >> <mailto:cdu...@apple.com>> wrote:
>>> 
>>> Hi,
>>> 
>>> I have now been caught twice by std::optional’s move constructor. It turns 
>>> out that it leaves the std::optional being moved-out *engaged*, it merely 
>>> moves its value.
>>> 
>>> For example, testOptional.cpp:
>>> #include 
>>> #include 
>>> 
>>> int main()
>>> {
>>> std::optional a = 1;
>>> std::optional b = std::move(a);
>>> std::cout << "a is engaged? " << !!a << std::endl;
>>> std::cout << "b is engaged? " << !!b << std::endl;
>>> return 0;
>>> }
>>> 
>>> $ clang++ testOptional.cpp -o testOptional -std=c++17
>>> $ ./testOptional
>>> a is engaged? 1
>>> b is engaged? 1
>>> 
>>> I would have expected:
>>> a is engaged? 0
>>> b is engaged? 1
>> I would have expected this too.
>> 
>>> 
>>> This impacts the standard std::optional implementation on my machine as 
>>> well as the local copy in WebKit’s wtf/Optional.h.
>>> 
>>> As far as I know, our convention in WebKit so far for our types has been 
>>> that types getting moved-out are left in a valid “empty” state.
>> I believe it's UB to use an object after it has been moved.
> I'm wrong here.
> Apparently objects are left in a "valid but unspecified state".
> 
> https://stackoverflow.com/questions/32346143/undefined-behavior-with-stdmove 
> <https://stackoverflow.com/questions/32346143/undefined-behavior-with-stdmove>
I believe in WebKit, however, we’ve made sure our types are left in a valid 
“empty” state, thus my proposal for our own optional type that would be less 
error-prone / more convenient to use.

> 
> - Saam
>> 
>> - Saam
>> 
>>> As such, I find that std::optional’s move constructor behavior is 
>>> error-prone.
>>> 
>>> I’d like to know how do other feel about this behavior? If enough people 
>>> agree this is error-prone, would we consider having our
>>> own optional type in WTF which resets the engaged flag (and never allow the 
>>> std::optional)?
>>> 
>>> Thanks,
>>> --
>>>  Chris Dumez
>>> 
>>> 
>>> 
>>> 
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev>
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> <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] Watch out for std::optional's move constructor

2018-12-14 Thread Chris Dumez
Hi,

I have now been caught twice by std::optional’s move constructor. It turns out 
that it leaves the std::optional being moved-out *engaged*, it merely moves its 
value.

For example, testOptional.cpp:
#include 
#include 

int main()
{
std::optional a = 1;
std::optional b = std::move(a);
std::cout << "a is engaged? " << !!a << std::endl;
std::cout << "b is engaged? " << !!b << std::endl;
return 0;
}

$ clang++ testOptional.cpp -o testOptional -std=c++17
$ ./testOptional
a is engaged? 1
b is engaged? 1

I would have expected:
a is engaged? 0
b is engaged? 1

This impacts the standard std::optional implementation on my machine as well as 
the local copy in WebKit’s wtf/Optional.h.

As far as I know, our convention in WebKit so far for our types has been that 
types getting moved-out are left in a valid “empty” state.
As such, I find that std::optional’s move constructor behavior is error-prone.

I’d like to know how do other feel about this behavior? If enough people agree 
this is error-prone, would we consider having our
own optional type in WTF which resets the engaged flag (and never allow the 
std::optional)?

Thanks,
--
 Chris Dumez




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


Re: [webkit-dev] PSA: String shouldn't be a member of a ThreadSafeRefCounted class

2018-02-24 Thread Chris Dumez
Actually, it is my understanding that isolatedCopy() does the right thing here. 
If you look at the implementation:

String String::isolatedCopy() const &
{
// FIXME: Should this function, and the many others like it, be inlined?
return m_impl ? m_impl->isolatedCopy() : String { };
}

String String::isolatedCopy() &&
{
if (isSafeToSendToAnotherThread()) {
// Since we know that our string is a temporary that will be destroyed
// we can just steal the m_impl from it, thus avoiding a copy.
return { WTFMove(*this) };
}

return m_impl ? m_impl->isolatedCopy() : String { };
}

The isolatedCopy() optimization (i.e. isSafeToSendToAnotherThread) can only be 
used if the String is a temporary. If you example, m_name is not a temporary 
and will be deep copied.

--
 Chris Dumez




> On Feb 23, 2018, at 11:45 PM, Ryosuke Niwa <rn...@webkit.org> wrote:
> 
> Hi all,
> 
> This is a remainder that our String class is NOT thread safe, and should NOT 
> be used inside an object shared across multiple threads. In particular, it's 
> not necessarily safe to have it as a member of ThreadSafeRefCounted class, 
> which can be accessed from multiple threads.
> 
> Let's consider the following example.
> 
> class A : public ThreadSafeRefCounted {
> public:
> A(const String& name)
> : m_name(name)
> { }
> String name() { return m_name.isolatedCopy(); }
> 
> private:
> String m_name;
> }
> 
> This code is NOT thread safe depending on how name() is used.
> 
> For example, if it's ever inserted or looked up in a hash table as the key, 
> or if it's ever converted into an AtomicString, then it would lead to memory 
> corruption. This is because String::hash() would mutate m_hashAndFlags member 
> variable without any lock, and isolatedCopy() doesn't make a copy if there is 
> exactly one reference to a given StringImpl (String is basically just a 
> RefPtr of StringImpl).
> 
> - 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] Upstreaming from LayoutTests to web-platform-tests, coordinating Blink+WebKit

2017-11-17 Thread Chris Dumez


> On Nov 17, 2017, at 9:18 AM, youenn fablet  wrote:
> 
> Chris recently noticed that some heavily used files (testharness*) were 
> cacheable through Apache but not WPT.

Wasn’t me. Alexey reported this via Description 
.___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] buildbot / ios-sim failing on serviceworket tests?

2017-11-06 Thread Chris Dumez
These are likely flaky tests, unrelated to your change.

--
 Chris Dumez




> On Nov 6, 2017, at 5:38 AM, Colin Bendell | +1.613.914.3387 
> <co...@bendell.ca> wrote:
> 
> Can someone provide guidance on how I should interpret serviceworker
> failures on the ios-sim queue? The same buildbot error appears in
> other patches:
> 
> https://webkit-queues.webkit.org/results/5118974 (bug 179231)
> https://webkit-queues.webkit.org/results/5121120 (bug 179285)
> 
> thoughts?
> 
> /colin
> ___
> 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] Get rid of RefPtr, replace with std::optional?

2017-09-01 Thread Chris Dumez
I think std::optional<Ref> looks ugly. Also, unlike RefPtr<>, I do not 
think it is copyable. It is pretty neat to be able to capture a RefPtr<> by 
value in a lambda.
Also, how do you convert it to a raw pointer? myOptionalRef.value_or(nullptr) 
would not work. Not sure there would be a nice way to do so.

Finally, the storage space argument from Maciej is a good one.
 
--
 Chris Dumez




> On Sep 1, 2017, at 9:46 AM, Maciej Stachowiak <m...@apple.com> wrote:
> 
> 
> 
>> On Sep 1, 2017, at 9:30 AM, Brady Eidson <beid...@apple.com> wrote:
>> 
>> I recently worked on a patch where - because of the organic refactoring of 
>> the patch over its development - I ended up with a std::optional 
>> instead of a RefPtr.
>> 
>> A followup review after it had already landed pointed this out, and it got 
>> me to thinking:
>> 
>> Does RefPtr do anything for us today that std::optional doesn’t?
> 
> The obvious things would be: uses less storage space, has a shorter name.
> 
>> 
>> I kind of like the idea of replacing RefPtr with std::optional. It 
>> makes it explicitly clear what object is actually holding the reference, and 
>> completely removes some of the confusion of “when should I use Ref vs 
>> RefPtr?"
>> 
>> Thoughts?
>> 
>> Thanks,
>> ~Brady
>> ___
>> 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] Growing tired of long build times? Check out this awesome new way to speed up your build... soon (HINT: It's not buying a new computer)

2017-08-29 Thread Chris Dumez


> On Aug 29, 2017, at 9:24 AM, Keith Miller <keith_mil...@apple.com> wrote:
> 
>> * We could not use any namespaces and just fix name clashes as they arise, 
>> but only if we have some more predictable way to determine which files will 
>> be built together regardless of port or build configuration. Say files in 
>> small directories all get built together, while files in large directories 
>> get built together if their filenames start with the same letter. That might 
>> be too confusing, but a scheme like that would make it possible for us to 
>> ensure we don't have name clashes regardless of port configuration, while 
>> avoiding the need to add namespaces all over the place.
> 
> I had considered something like this. Ultimately, I decided the FILENAME 
> solution was better because it lets us use descriptive names without having 
> to unique the name. When new cpp files are added to the project, if we don’t 
> use a namespace solution new errors in totally unrelated files may appear due 
> to reordering of the unified source lists. For new developers, errors 
> exclusively in .cpp files they didn’t touch would be particularly frustrating 
> and confusing. e.g. They add Foo.cpp which causes Bar.cpp and Baz.cpp to get 
> bundled together causing a redefinition error in Bar/Baz.cpp. While, new 
> developers might also be annoyed with the FILENAME solution, it’s a pretty 
> simple rule to follow and it doesn’t lead to the same surprises. 


Keep in mind that if you do this, you may not get a build error and still run 
into trouble. If several static functions end up having the same name but 
different parameters, your code may build but may end up crashing or with 
different behavior because we end up calling a different overload. This would 
seem very risky for a big project such as WebKit.

--
 Chris Dumez

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


Re: [webkit-dev] Growing tired of long build times? Check out this awesome new way to speed up your build... soon (HINT: It's not buying a new computer)

2017-08-29 Thread Chris Dumez
I worry about adopting unity build because while it makes clean builds faster, 
it also slows down incremental builds. As a developer, I rarely do clean 
builds, I mostly do incremental builds so this would likely make my experience 
worse?
Unity builds also require a lot of code churn to resolve naming conflicts and 
the resulting code does not look as nice.

Finally, the statement that the slow down of incremental build is acceptable 
because we spend most of our time resolving dependencies seems to assume we 
keep using make. Using Ninja would speed up incremental builds a lot by not
re-resolving dependencies so much.

--
 Chris Dumez




> On Aug 29, 2017, at 9:05 AM, Darin Adler <da...@apple.com> wrote:
> 
>> On Aug 28, 2017, at 8:34 PM, Ryosuke Niwa <rn...@webkit.org 
>> <mailto:rn...@webkit.org>> wrote:
>> 
>>> Wherever possible, we are going to want to convert file-static functions
>>> into private C++ class member functions.
>> 
>> I don't think we should make this change. It would mean that whenever
>> the function signature changes, we'd have to modify the header file,
>> which in turn triggers rebuild of every cpp file which includes that
>> header file.
> 
> 
> That was my first thought as well. In fact, I typically ask people to do the 
> opposite: Whenever a function does not require access to private members, 
> convert from a member function that has to be in the header file to a 
> function that is private to the source file.
> 
> More recently I have started doing something different for the many functions 
> that only have to be used on one place; I use lambdas to create nested 
> functions. This has a couple nice properties: The lambda shares access to 
> private members and anything else that the surrounding function has access 
> to. We have the option of capturing rather than passing arguments, which can 
> be clearer in some cases. It’s not quite as good as a separate function for 
> visual separation; the lambda is often mashed together with the rest of the 
> surrounding function.
> 
> — 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] Unified source builds: A new rule for static variables

2017-08-29 Thread Chris Dumez
I indeed think this will require “using” statements or explicit namespace at 
call sites.

I don’t think anonymous namespaces are suitable for resolving naming conflicts 
due to unity builds since the functions and up in the same compilation unit.

--
 Chris Dumez




> On Aug 29, 2017, at 9:00 AM, Darin Adler <da...@apple.com> wrote:
> 
> How does this work? Without a “using” how does it know to search this 
> namespace? Is this superior to using anonymous namespaces instead of “static”?
> 
> — 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] How we enable template functions

2017-08-22 Thread Chris Dumez
I personally prefer std::enable_if<>. For e.g.

template::value>
Class Foo { }

I don’t like that something inside the body of a class / function would cause a 
template to be enabled or not.

--
 Chris Dumez




> On Aug 22, 2017, at 8:34 PM, Keith Miller <keith_mil...@apple.com> wrote:
> 
> Hello fellow WebKittens,
> 
> I’ve noticed over time that we don’t have standard way that we enable 
> versions of template functions/classes (flasses?). For the most part it seems 
> that people use std::enable_if, although, it seems like it is attached to 
> every possible place in the function/class.
> 
> I propose that we choose a standard way to conditionally enable a template.
> 
> There are a ton of options; my personal favorite is to add the following 
> macro:
> 
> #define ENABLE_TEMPLATE_IF(condition) static_assert(condition, “template 
> disabled”)
> 
> Then have every function do:
> 
> template
> void foo(…)
> {
>ENABLE_TEMPLATE_IF(std::is_same<T, int>::value);
>…
> }
> 
> And classes:
> 
> template
> class Foo {
>ENABLE_TEMPLATE_IF(std::is_same<T, int>::value);
> };
> 
> I like this proposal because it doesn’t obstruct the signature/declaration of 
> the function/class but it’s still obvious when the class is enabled. 
> Obviously, I think we should require that this macro is the first line of the 
> function or class for visibility. Does anyone else have thoughts or ideas?
> 
> Cheers,
> Keith
> 
> P.S. in case you are wondering why this macro works (ugh C++), it’s because 
> if there is any compile time error in a template it cannot be selected as the 
> final candidate. In my examples, if you provided a type other than int 
> foo/Foo could not be selected because the static_assert condition would be 
> false, which is a compile error.
> ___
> 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] Does someone know how to fix WTF::Function on Windows

2017-07-15 Thread Chris Dumez
I seem to remember I fixed it last time by calling the WTF::Function 
constructor explicitly.

Chris Dumez

> On Jul 15, 2017, at 9:14 AM, Yusuke SUZUKI <utatane@gmail.com> wrote:
> 
> I'm not 100% confident, but can you try it `` instead?
> 
> Regards,
> Yusuke Suzuki
> 
>> On Sun, Jul 16, 2017 at 1:13 AM, Darin Adler <da...@apple.com> wrote:
>> Hi folks.
>> 
>> On Windows, WTF::Function doesn’t quite work right. Code that is something 
>> like this:
>> 
>> void addCallback(WTF::Function<void()>&&);
>> 
>> void testFunction()
>> {
>> // ...
>> }
>> 
>> void addTestFunction()
>> {
>> addCallback(testFunction);
>> }
>> 
>> Leads to errors like this:
>> 
>> error C2664: 'void addCallback(WTF::Function &&)': cannot 
>> convert argument 1 from 'void (__cdecl *)(void)' to 'WTF::Function> (void)> &&'
>> note: You cannot bind an lvalue to an rvalue reference
>> 
>> The problem might have something to do with cdecl vs. stdcall functions, but 
>> I am not sure that is the problem. It could be some other problem with how 
>> WTF::Function is written. Or it might even be a bug in the Visual Studio 
>> compiler. Since I don’t have a Windows machine myself, I was trying to use 
>> EWS to figure this out but that was slow. Then I tried using 
>> http://webcompiler.cloudapp.net but I could not reproduce any error there 
>> when I pasted in cut down code; it just compiled fine.
>> 
>> Is there someone who knows how to fix this?
>> 
>> Another way to put this is: We want to take off the explicit WTF::Function 
>> conversions in functions like canUseWithReason in SimpleLineLayout.cpp, 
>> Page::Page in Page.cpp, and Worker::Worker in Worker.cpp and have it still 
>> compile and work on Windows. Other platforms seem to compile fine without 
>> the explicit WTF::Function.
>> 
>> — 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
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Should we ever use std::function instead of WTF::Function?

2017-06-13 Thread Chris Dumez

> On Jun 13, 2017, at 12:51 PM, Filip Pizlo <fpi...@apple.com> wrote:
> 
>> 
>> On Jun 13, 2017, at 12:37 PM, Alex Christensen <achristen...@apple.com 
>> <mailto:achristen...@apple.com>> wrote:
>> 
>> Ok, maybe we can get rid of std::function, then!  I hadn’t used BlockPtr as 
>> much as Chris.  I’d be opposed to adding a copy constructor to WTF::Function 
>> because the non-copyability of WTF::Function is why we made it, and it has 
>> prevented many bugs.
> 
> I agree that the copy semantics of std::function are strange - each copy gets 
> its own view of the state of the closure.  This gets super weird when you 
> implement an algorithm that accidentally copies the function - the act of 
> copying invokes copy constructors on all of the lambda-lifted state, which is 
> probably not what you wanted.  So I’m not surprised that moving to 
> WTF::Function avoided bugs.  What I’m proposing also prevents many of the 
> same bugs because the lambda-lifted state never gets copied in my world.

I understand SharedTask avoids many of the same issues. My issue is that it 
adds an extra data member for refcounting that is very rarely needed in 
practice. Also, because this type is copyable, people can create refcounting 
churn by inadvertently copying.
Because most of the time, we do not want to share and WTF::Function is 
sufficient as it stands, I do not think it is worth making WTF::Function 
refcounted.

> 
> Do you think that code that uses ObjC blocks encounters the kind of bugs that 
> you saw WTF::Function preventing?  Or are the bugs that Function prevents 
> more specific to std::function?  I guess I’d never heard of a need to change 
> block semantics to avoid bugs, so I have a hunch that the bugs you guys 
> prevented were specific to the fact that std::function copies instead of 
> sharing.

To be cleared, we viewed this as “copies instead of truly moving”, not sharing. 
We never really needed sharing when WTF::Function was initially called 
WTF::NonCopyableFunction.
Yes, the bugs we were trying to avoid were related to using std::function and 
copying things implicitly, even if you WTFMove() it around. Because we started 
using WTF::Function instead of std::function in more places though, having 
BlockPtr::fromCallable() to be able to pass a WTF::Function to an ObjC function 
expecting a block became handy though. 

> 
>> 
>> I’ve also seen many cases where I have a WTF::Function that I want to make 
>> sure is called once and only once before destruction.  I wouldn’t mind 
>> adding a WTF::Callback subclass that just asserts that it has been called 
>> once.  That would’ve prevented some bugs, too, but not every use of 
>> WTF::Function has such a requirement.
>> 
>>> On Jun 13, 2017, at 12:31 PM, Chris Dumez <cdu...@apple.com 
>>> <mailto:cdu...@apple.com>> wrote:
>>> 
>>> We already have BlockPtr for passing a Function as a lambda block.
>>> 
>>> Chris Dumez
>>> 
>>> On Jun 13, 2017, at 12:29 PM, Alex Christensen <achristen...@apple.com 
>>> <mailto:achristen...@apple.com>> wrote:
>>> 
>>>> std::function, c++ lambda, and objc blocks are all interchangeable.  
>>>> WTF::Functions cannot be used as objc blocks because the latter must be 
>>>> copyable.  Until that changes or we stop using objc, we cannot completely 
>>>> eliminate std::function from WebKit.
>>>> ___
>>>> webkit-dev mailing list
>>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev>
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> <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] Should we ever use std::function instead of WTF::Function?

2017-06-13 Thread Chris Dumez
We already have BlockPtr for passing a Function as a lambda block.

Chris Dumez

> On Jun 13, 2017, at 12:29 PM, Alex Christensen <achristen...@apple.com> wrote:
> 
> std::function, c++ lambda, and objc blocks are all interchangeable.  
> WTF::Functions cannot be used as objc blocks because the latter must be 
> copyable.  Until that changes or we stop using objc, we cannot completely 
> eliminate std::function from WebKit.
> ___
> 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] Should we ever use std::function instead of WTF::Function?

2017-06-13 Thread Chris Dumez
In most cases in WebCore at least, we don’t actually want to share ownership of 
the lambda so we don’t need RefCounting / SharedTask. Because of this, I don’t 
think we should merge SharedTask into Function. I think that as it stands, 
WTF::Function is a suitable replacement for most uses in WebCore since we 
actually very rarely need copying (either it just builds or the code can be 
refactored very slightly to avoid the copying).

--
 Chris Dumez




> On Jun 13, 2017, at 9:34 AM, Filip Pizlo <fpi...@apple.com> wrote:
> 
> We should have a better story here.  Right now the story is too complicated.  
> We have:
> 
> - ScopedLambda or ScopedLambdaRef if you have a stack-allocated function that 
> outlives its user
> - SharedTask if you have a heap-allocated function that you want to share and 
> ref-count
> - WTF::Function if you have a heap-allocated function that you want to 
> transfer ownership (move yes, copy no)
> - std::function if you have a heap-alloated function that you want to pass by 
> copy
> 
> Only std::function and WTF::Function do the magic that lets you say:
> 
> std::function f = 
> 
> Also, std::function has the benefit that it does copying.  None of the others 
> do that.
> 
> ScopedLambda serves a specific purpose: it avoids allocation.  Probably we 
> want to keep that one even if we merge the others.
> 
> IMO SharedTask has the nicest semantics.  I don’t ever want the activation of 
> the function to be copied.  In my experience I always want sharing if more 
> than one reference to the function exists.  I think that what we really want 
> in most places is a WTF::Function that has sharing semantics like SharedTask. 
>  That would let us get rid of std::function and SharedTask.
> 
> In the current status quo, it’s not always correct to convert std::function 
> to the others because:
> 
> - Unlike ScopedLambda and SharedTask, std::function has the magic constructor 
> that allows you to just assign a lambda to it.
> - Unlike ScopedLambda, std::function is safe if the use is not scoped.
> - Unlike WTF::Function, std::function can be copied.
> 
> -Filip
> 
> 
>> On Jun 13, 2017, at 9:24 AM, Darin Adler <da...@apple.com> wrote:
>> 
>> I’ve noticed many patches switching us from std::function to WTF::Function 
>> recently, to fix problems with copying and thread safety.
>> 
>> Does std::function have any advantages over WTF::Function? Should we ever 
>> prefer std::function, or should we use WTF::Function everywhere in WebKit 
>> where we would otherwise use std::function?
>> 
>> — 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

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


Re: [webkit-dev] Should we ever use std::function instead of WTF::Function?

2017-06-13 Thread Chris Dumez
The only advantage of std::function that I know of is that it is copyable. 
Unless you really need to copy your lambda for a reason or another, I don’t see 
a good reason to ever use std::function instead of WTF::Function.

I have been slowly replacing std::function by WTF::Function in the code base 
but there are still a lot of uses of std::function.

--
 Chris Dumez




> On Jun 13, 2017, at 9:24 AM, Darin Adler <da...@apple.com> wrote:
> 
> I’ve noticed many patches switching us from std::function to WTF::Function 
> recently, to fix problems with copying and thread safety.
> 
> Does std::function have any advantages over WTF::Function? Should we ever 
> prefer std::function, or should we use WTF::Function everywhere in WebKit 
> where we would otherwise use std::function?
> 
> — 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] "ReflectOnly" IDL equivalent

2017-06-09 Thread Chris Dumez

> On Jun 9, 2017, at 11:47 AM, Sam Weinig <wei...@apple.com> wrote:
> 
> 
> 
>> On Jun 2, 2017, at 11:32 AM, Ryosuke Niwa <rn...@webkit.org> wrote:
>> 
>> On Fri, Jun 2, 2017 at 9:18 AM, Chris Dumez <cdu...@apple.com> wrote:
>>> Hi,
>>> 
>>> No, I do not believe WebKit supports ReflectOnly and this is not standard
>>> IDL either.
>>> 
>>> The way to do it in WebKit would be to use a regular DOMString attribute, as
>>> in the specification and implement this logic in the c++ getter for this
>>> attribute. See HTMLElement::dir() for an example.
>>> 
>>> We could also consider adding support for something like ReflectOnly in our
>>> bindings generator considering that this seems to be used quite a bit in the
>>> HTML specification and it would decrease code complexity a little.
>>> I’d actually be in favor of that.
>> 
>> I'd suggest other names like "ReflectEnum" or even "Reflect"
>> where EnumType is the name of enum that defines the list of values.
>> 
>> "ReflectOnly" doesn't tell us on what "only" applies. If I didn't know
>> the context, it sounds like something that does less work than regular
>> "Reflect”.
> 
> 
> I don’t see a good reason to complicate the bindings until this becomes more 
> common place.  For now, I would just implement HTMLLinkElement::as() to 
> behave as you want and leave the IDL unannotated, and we can revisit it at a 
> later time.

As I said, this is already used in quite a few places in the HTML spec:
- https://html.spec.whatwg.org/#dom-dir <https://html.spec.whatwg.org/#dom-dir>
- https://html.spec.whatwg.org/#dom-link-as 
<https://html.spec.whatwg.org/#dom-link-as>
- https://html.spec.whatwg.org/#dom-link-referrerpolicy 
<https://html.spec.whatwg.org/#dom-link-referrerpolicy>
- https://html.spec.whatwg.org/#dom-link-updateviacache 
<https://html.spec.whatwg.org/#dom-link-updateviacache>
- https://html.spec.whatwg.org/#dom-a-referrerpolicy 
<https://html.spec.whatwg.org/#dom-a-referrerpolicy>
- https://html.spec.whatwg.org/#dom-img-referrerpolicy 
<https://html.spec.whatwg.org/#dom-img-referrerpolicy>
- https://html.spec.whatwg.org/#dom-iframe-referrerpolicy 
<https://html.spec.whatwg.org/#dom-iframe-referrerpolicy>
- https://html.spec.whatwg.org/#dom-track-kind 
<https://html.spec.whatwg.org/#dom-track-kind>
- https://html.spec.whatwg.org/#dom-media-preload 
<https://html.spec.whatwg.org/#dom-media-preload>
- https://html.spec.whatwg.org/#dom-area-referrerpolicy 
<https://html.spec.whatwg.org/#dom-area-referrerpolicy>
- https://html.spec.whatwg.org/#dom-th-scope 
<https://html.spec.whatwg.org/#dom-th-scope>
- https://html.spec.whatwg.org/#dom-form-autocomplete 
<https://html.spec.whatwg.org/#dom-form-autocomplete>
- https://html.spec.whatwg.org/#dom-input-type 
<https://html.spec.whatwg.org/#dom-input-type>
- https://html.spec.whatwg.org/#dom-input-inputmode 
<https://html.spec.whatwg.org/#dom-input-inputmode>
- https://html.spec.whatwg.org/#dom-button-type 
<https://html.spec.whatwg.org/#dom-button-type>
- https://html.spec.whatwg.org/#dom-textarea-inputmode 
<https://html.spec.whatwg.org/#dom-textarea-inputmode>
- https://html.spec.whatwg.org/#dom-fs-method 
<https://html.spec.whatwg.org/#dom-fs-method>
- https://html.spec.whatwg.org/#dom-fs-enctype 
<https://html.spec.whatwg.org/#dom-fs-enctype>
- https://html.spec.whatwg.org/#dom-fs-formenctype 
<https://html.spec.whatwg.org/#dom-fs-formenctype>
- https://html.spec.whatwg.org/#dom-fs-formmethod 
<https://html.spec.whatwg.org/#dom-fs-formmethod>

Having a per-standard implementation in the bindings would likely be better 
than many potentially non-compliant ones.

--
 Chris Dumez

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


Re: [webkit-dev] "ReflectOnly" IDL equivalent

2017-06-02 Thread Chris Dumez
Hi,

No, I do not believe WebKit supports ReflectOnly and this is not standard IDL 
either.

The way to do it in WebKit would be to use a regular DOMString attribute, as in 
the specification and implement this logic in the c++ getter for this 
attribute. See HTMLElement::dir() for an example.

We could also consider adding support for something like ReflectOnly in our 
bindings generator considering that this seems to be used quite a bit in the 
HTML specification and it would decrease code complexity a little.
I’d actually be in favor of that.

--
 Chris Dumez




> On Jun 1, 2017, at 10:57 PM, Yoav Weiss <y...@yoav.ws> wrote:
> 
> I'm working on the link preload feature and as part of that want to align it 
> to the spec from an IDL perspective.
> The `as` attribute is defined <https://html.spec.whatwg.org/#attr-link-as> as 
> an "enumerated attribute", which reflects only a finite set of known values. 
> That is important for feature detection purposes, so that developers can know 
> which `as` values are supported by the implementation.
> 
> In Blink this is done using `ReflectOnly 
> <https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/Source/bindings/IDLExtendedAttributes.md#ReflectOnly_list_a>`
>  (pending CL <https://codereview.chromium.org/2903653005/>), and I'm 
> wondering what is the WebKit equivalent to implement attributes that are 
> limited to known values 
> <https://html.spec.whatwg.org/#limited-to-only-known-values>.
> 
> Thanks! :)
> Yoav
> ___
> 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] Another WPT bite

2017-05-12 Thread Chris Dumez
Our test importer script is perfectly able to rewrite those paths to use 
relative paths. However, Youenn, who imports and re-syncs most tests does not 
like this option I believe.
I think, part of the issue is that *some* tests do not do the right thing when 
loading over file:// (e.g. security ones) and it is not necessarily obvious 
just by looking at the test.

--
 Chris Dumez




> On May 12, 2017, at 2:10 PM, Simon Fraser <simon.fra...@apple.com> wrote:
> 
>> 
>> On May 12, 2017, at 12:04 PM, Alexey Proskuryakov <a...@webkit.org 
>> <mailto:a...@webkit.org>> wrote:
>> 
>> 
>>> 12 мая 2017 г., в 11:52, Ben Kelly <b...@wanderview.com 
>>> <mailto:b...@wanderview.com>> написал(а):
>>> 
>>> On Fri, May 12, 2017 at 2:26 PM, Rick Byers <rby...@chromium.org 
>>> <mailto:rby...@chromium.org>> wrote:
>>> On Fri, May 12, 2017 at 2:06 PM, Alexey Proskuryakov <a...@webkit.org 
>>> <mailto:a...@webkit.org>> wrote:
>>> Since imported WPT tests are very flaky, and are not necessarily written to 
>>> defend against important regressions, investigating issues with them is 
>>> relatively lower priority than investigating issues observed with WebKit 
>>> tests. So I would recommend not mixing tests for WebKit regressions with 
>>> WPT tests - if your test eventually ends up in LayoutTests/imported, it 
>>> will become a lot less effective.
>>> 
>>> FWIW this is absolutely NOT how we're treating this in chromium.  If this 
>>> is how things end up in practice then we will have failed massively in this 
>>> effort.
>>> 
>>> We figure if we want the web to behave consistently, we really have no 
>>> choice but to treat web-platform-tests as first class with all the 
>>> discipline we give to our own tests.  As such we are actively moving 
>>> <https://codereview.chromium.org/2877673004> many of our LayoutTests to 
>>> web-platform-tests and depending entirely on the regression prevention they 
>>> provide us from there.  Obviously there will be hiccups, but because our 
>>> product quality will depend on web-platform-tests being an effective and 
>>> non-flaky testsuite (and because we're starting to require most new 
>>> features have web-platform-tests before they ship), I'm confident that 
>>> we've got the incentives in place to lead to constant ratcheting up the 
>>> engineering discipline and quality of the test suite.
>>> 
>>> FWIW, mozilla also treats WPT as first class tests.  We're not actively 
>>> moving old tests to WPT like google, but all new tests (at least in DOM) 
>>> are being written in WPT.  Of course, we do have exceptions for some tests 
>>> that require gecko-specific features (forcing GC, etc).
>> 
>> 
>> We don't have a concept of "first class", but I hope that when choosing 
>> between looking into a simple test that was added for a known important bug, 
>> and looking into an imported test whose importance is unclear, any WebKit 
>> engineer will pick the former. And since no one can fix all the things, such 
>> prioritization makes imported tests less effective.
> 
> I just ran into a classic example of how a WPT incurred more overhead. I made 
> a code change that broke 
> LayoutTests/imported/w3c/web-platform-tests/cssom-view/elementFromPoint.html. 
> I tried loading it in Safari and it doesn't run the test code because it 
> can't find /resources/testharness.js when loaded from a local file.
> 
> So then I have to figure out how to fire up the WPT server 
> (run-webkit-httpd), then figure out which host to use (localhost or 
> 128.0.0.1?) and which port, and finally to figure out the right path to the 
> test.
> 
> There's no reason this test should not work when loaded from file://.
> 
> Simon
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> <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] Another WPT bite

2017-05-08 Thread Chris Dumez


> On May 8, 2017, at 9:44 PM, Geoffrey Garen  wrote:
> 
>> Is it time to make testharness.js the recommended way of writing LayoutTests?
> 
> What are the costs and benefits of testharness.js?

Benefit: 
- Tests would be more easily upstreamable to web-platform-tests, which are run 
by all major browser engines. This would help a lot in terms of 
interoperability. As previously discussed, Gecko and Blink already do automated 
export of tests to web-platform-tests. I believe we should do in the same 
direction and contribute more tests back.
- It is quite powerful (see documentation [1], has things like Promise tests, 
Worker tests and advanced assertions.

Cost:
- People are not necessarily used to it so there is a little bit of a learning 
curve. It is well documented though [1].

> 
> We usually try to make regression tests reductions of some larger problem to 
> aid debugging and to make testing fast. But testharness.js is 95kB. That's 
> kind of the opposite of a reduction.

It is proposed as a replacement to js-test.js, which a lot of us are already 
using in our layout tests. Using js-test.js or testharness.js has rarely 
interfered in my reductions, although it has happened to me.
For some tests, we’ll probably not use any framework. However, for most of 
them, I personally don’t see an issue.

[1] http://testthewebforward.org/docs/testharness-library.html 



> 
> Geoff
> 
>> 
>> To continue moving forward, some of us are proposing to serve all tests in 
>> LayoutTests/wpt through the WPT server [1].
>> This would serve some purposes like increasing the use of WPT goodies: 
>> file-specific headers, templated tests (*.any.js), IDLParser, server-side 
>> scripts...
>> It could also ease test migration from WebKit to W3C WPT.
>> 
>> Some rules can guide whether adding tests to LayoutTests/wpt or 
>> LayoutTests/imported/w3c/web-platform-tests:
>> - WebKit specific tests (crash tests, tests using internals...) in 
>> LayoutTests/wpt
>> - Spec conformance/interoperability tests in 
>> LayoutTests/imported/w3c/web-platform-tests
>> 
>>y
>> 
>> [1]: bug 171479 
>> 
>> 
>> ___
>> 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] !!Tests for equality comparison

2017-04-28 Thread Chris Dumez



> On Apr 28, 2017, at 8:10 AM, Geoffrey Garen <gga...@apple.com> wrote:
> 
> Tests for numeric values where 0 indicates falsiness or emptiness should also 
> use if (x) / if (!x).
> 
>If (!collection.size())
>return;


I think if (collection.isEmpty()) looks even better :)

--
 Chris Dumez

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


Re: [webkit-dev] !!Tests for equality comparison

2017-04-27 Thread Chris Dumez
I also do not like this rule when it comes to integers.

I personally think JF’s proposal to allow == 0 sounds nice. I don’t think JF 
was suggesting rewriting existing code (which would indeed cause a lot of 
churn).

--
 Chris Dumez




> On Apr 27, 2017, at 4:30 PM, Geoffrey Garen <gga...@apple.com> wrote:
> 
> I’ve never really liked this style rule, and I’ve always felt like it snuck 
> into the style document without much discussion.
> 
> Even so, I usually tolerate it.
> 
> Geoff
> 
>> On Apr 27, 2017, at 4:06 PM, JF Bastien <jfbast...@apple.com 
>> <mailto:jfbast...@apple.com>> wrote:
>> 
>> Hello C++ fans!
>> 
>> The C++ style check currently say:
>> Tests for true/false, null/non-null, and zero/non-zero should all be done 
>> without equality comparisons
>> 
>> I totally agree for booleans and pointers… but not for integers. I know it’s 
>> pretty much the same thing, but I it takes me slightly longer to process 
>> code like this:
>> 
>> int numTestsForEqualityComparison = 0:
>> // Count ‘em!
>> // …
>> if (!numTestsForEqualityComparison)
>>   printf(“Good job!”);
>> 
>> I read it as “if not number of tests for equality comparison”. That's weird. 
>> It takes me every slightly longer to think about, and I’ve gotten it wrong a 
>> bunch of times already. I’m not trying to check for “notness", I’m trying to 
>> say “if there were zero tests for equality comparison”, a.k.a.:
>> 
>> if (numTestsForEqualityComparison == 0)
>>   printf(“Good job!”);
>> 
>> So how about the C++ style let me just say that? I’m not suggesting we 
>> advise using that style for integers everywhere, I’m just saying it should 
>> be acceptable to check zero/non-zero using equality comparison.
>> 
>> 
>> !!Thanks (i.e. many thanks),
>> 
>> JF
>> 
>> p.s.: With you I am, fans of Yoda comparison, but for another day this will 
>> be.
>> ___
>> 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] bugs.webkit.org - Maintenance: 10.00 - 11.00am

2017-03-30 Thread Chris Dumez
Any update? It’s been a few minutes :)

--
 Chris Dumez




> On Mar 30, 2017, at 9:55 AM, Ling Ho <lin...@apple.com> wrote:
> 
> Hello WebKit developers,
> 
> We are flipping a switch to restart bugs.webkit.org on a new server between 
> 10-11am this morning. There should only be a few minutes of interruptions 
> while DNS records are being changed. Please delay saving any change till we 
> are back on the new server.
> 
> Thanks,
> ...
> ling
> ___
> 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] Bugzilla down?

2017-03-10 Thread Chris Dumez
Bugzilla seems down for me. I am getting certificate errors:


--
 Chris Dumez




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


Re: [webkit-dev] SVN trouble

2017-02-26 Thread Chris Dumez
It also seems bugzilla mail notifications may be down.

Chris Dumez

> On Feb 25, 2017, at 8:40 AM, Simon Fraser <simon.fra...@apple.com> wrote:
> 
> EWS is still down. Do we have an ETA?
> 
> Simon
> 
>>> On Feb 24, 2017, at 10:25 PM, Alexey Proskuryakov <a...@webkit.org> wrote:
>>> 
>>> 
>>> 24 февр. 2017 г., в 19:50, Chris Dumez <cdu...@apple.com> написал(а):
>>> 
>>> 
>>> 
>>> 
>>>> On Feb 24, 2017, at 11:41 AM, Alexey Proskuryakov <a...@webkit.org> wrote:
>>>> 
>>>> I believe that all infrastructure has recovered. We are currently looking 
>>>> into one unrelated issue with webkit-queues, so EWS and commit queue don't 
>>>> work.
>>>> 
>>>> - Alexey
>>> 
>>> 
>>> It looks like EWS is still down. Did it break again or is it just not fixed 
>>> yet?
>> 
>> 
>> It did work for a period of time, but no conclusive fix yet.
>> 
>> - Alexey
> 
> ___
> 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] SVN trouble

2017-02-24 Thread Chris Dumez



> On Feb 24, 2017, at 11:41 AM, Alexey Proskuryakov <a...@webkit.org> wrote:
> 
> I believe that all infrastructure has recovered. We are currently looking 
> into one unrelated issue with webkit-queues, so EWS and commit queue don't 
> work.
> 
> - Alexey


It looks like EWS is still down. Did it break again or is it just not fixed yet?

--
 Chris Dumez

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


Re: [webkit-dev] SVN trouble

2017-02-24 Thread Chris Dumez


> On Feb 24, 2017, at 11:41 AM, Alexey Proskuryakov <a...@webkit.org> wrote:
> 
> The only thing I did was block access to 
> cache/disk-cache/resources/shattered-2.pdf using authz (this is the file with 
> collision). I think that the reason why mirrors only updated now is that 
> someone committed to trunk, and thus invoked post-commit hooks.


Yes, I confirm manual commits work now :)

--
 Chris Dumez

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


Re: [webkit-dev] Upstreaming Tests from WebKit to Web Platform Tests

2017-02-05 Thread Chris Dumez
I also think we should do 2-way sync-ing of WPT tests, similarly to other 
browser engines. These tests are extremely useful for interoperability. Every 
major browser engine is involved.

Google also has a nice dashboard showing the pass rates for each WPT test in 
each major browser. It makes it easier to identify areas we should improve on.

I - for one - would be happy to write tests using testharness.js when it makes 
sense if it means I can just land them in WebKit with my code change and have 
them being automagically upstreamed, and run by other browsers as well.

Note that I am not advocating we use testharness.js for everything (although I 
would be OK with this). I propose we make it opt-in. We have a lot of tests 
that are not necessarily interesting to upstream. However, the ones that are, 
using testharness.js makes a lot of sense.

Chris Dumez

> On Feb 5, 2017, at 5:04 PM, youenn fablet <youe...@gmail.com> wrote:
> 
> I too am a big proponent of us moving more and more towards WPT.
> As part of the streams and fetch API implementation, most of the related 
> functional tests have been made as WPT.
> The benefits of having tests being improved and updated largely outweighs the 
> testharness.js initial cost.
> Somehow, these tests are becoming part of their related specs.
> 
> The two complaints I heard against testharness.js are:
> - They are less easy to debug as test harness.js does not print out messages 
> for each assert. There might be room for updating testharness to support that
> - Async tests are less easy to write. While this is probably true, 
> testharness has support for promise-based tests which are not verbose.
> WPT has also some benefits of its own, like the possibility to run easily the 
> same tests in worker/window environments, the flexible python-based server...
> 
> One complaint I heard against WPT tests is that they require being run behind 
> a server.
> This is becoming more and more true. 
> There is still a large portion of tests that could be run locally if we 
> update the paths to test harness.js/testharnessreport,js.
> I am not a big fan of modify any WPT test but maybe we should consider 
> switching back to modifying these paths at import and export time.
> 
> I would also like to go in a direction where writing WPT tests is the default 
> for WebKit layout test.
> For that to happen, we need an easy way to upstream tests from WebKit to WPT 
> GitHub.
> 
> I would tend to do the following:
> - Write/submit WPT tests in WebKit as regular WebKit testharness.js layout 
> tests through bugzilla
> - At commit queue time, automatically create a PR to WPT GitHub containing 
> the changes of the WebKit patch
> - Ask committer to fix the WPT PR should there be any issue with it 
> (committer being informed of such issues through bugzilla).
> 
>> Le dim. 5 févr. 2017 à 15:42, Ryosuke Niwa <rn...@webkit.org> a écrit :
>> Hi all,
>> 
>> Background
>> 
>> Web Platform Tests is a suite of tests written for W3C specifications=,
>> and Blink, Edge, Gecko, and WebKit all run them as a part of their 
>> continuous build system.
>> 
>> Historically, each working group had their own repository for tests,
>> but with CSS WG's tests finally getting merged into the Web Platform Tests,
>> we will have a single repository with all the tests from W3C.
>> 
>> A few of us attended a meeting to discuss the future of this test suite last 
>> Monday.
>> One of the major topic was that Blink and Gecko have adopted the process to 
>> write the tests
>> using testharness.js in their own test suites, and automatically merge into 
>> Web Platform Tests
>> without having to go through another round of code review for Web Platform 
>> Tests.
>> 
>> Given we do test-driven development in WebKit, I think WebKit should do the 
>> same.
>> This will benefit other browser vendors to catch their bugs with our tests, 
>> and we will also benefit
>> from other browser vendors "reviewing" our tests against their 
>> implementation and specifications.
>> 
>> In the long term, I believe this will drastically improve the 
>> interoperability of the Web,
>> and dramatically improve the quality of the tests we run against WebKit.
>> 
>> In fact, there was a general consensus that we should even upstream 
>> pass-if-not-crash tests
>> as well as style and layout invalidation tests to web-platform-tests as they 
>> tend to be undertested
>> right now and almost all engines have bugs in this area.
>> 
>> Problems & Questions
>> 
>> In order to auto-merge tests from WebKit to Web Platform Tests, there are a 
>> fe

Re: [webkit-dev] [webkit-reviewers] usage of auto

2017-01-10 Thread Chris Dumez
I usually like using auto / auto* as much as possible.

The one exception where I have found using auto confusing was for functions 
returning an std::optional. 

E.g.
auto value = maximum();
if (!value)
return;

I find that the check is confusing because it returns early if value is 0 in 
the case where maximum() returns an integer but checks if the value is set in 
the case the function returns an std::optional.

Chris Dumez

On Jan 10, 2017, at 9:51 PM, Darin Adler <da...@apple.com> wrote:

>> On Jan 10, 2017, at 9:49 PM, Darin Adler <da...@apple.com> wrote:
>> 
>>> On Jan 10, 2017, at 9:46 PM, Simon Fraser <simon.fra...@apple.com> wrote:
>>> 
>>> auto countOfThing = getNumberOfThings();
>>> ASSERT(countOfThing >= 0);  // Can’t tell by reading whether the ASSERT is 
>>> assured at compile time if countOfThing is unsigned
>> 
>> I understand wanting to know, but I am not certain this is a bad thing.
> 
> Sorry, let me say something different, but related:
> 
>int countOfThing = getNumberOfThings();
> 
> Can’t tell from the above code if getNumberOfThings() returns int or unsigned.
> 
> — 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


[webkit-dev] Can no longer commit manually or via CQ?

2016-06-20 Thread Chris Dumez
Hi,

I can no longer commit locally (I am using git-svn). It complains about me 
having 30 or so local commits (which is not true).

Therefore, I tried using the commit queue but it fails to commit as well:
Comment 3 <https://bugs.webkit.org/show_bug.cgi?id=158943#c3>

ailed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', 
'--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 
'land-attachment', '--force-clean', '--non-interactive', 
'--parent-command=commit-queue', 281652, '--port=mac']" exit_code: 2 cwd: 
/Volumes/Data/EWS/WebKit

Last 500 characters of output:
1 Try to fix the iOS build after r202142 and r202224.
The copy of the patch that failed is found in:
   /Volumes/Data/EWS/WebKit/.git/rebase-apply/patch

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at /Volumes/Data/EWS/WebKit/Tools/Scripts/webkitdirs.pm line 2661.

Kr,
--
 Chris Dumez




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


Re: [webkit-dev] Removing ENABLE(DATA_TRANSFER_ITEMS)

2016-01-29 Thread Chris Dumez
Hi,

It is part of the HTML specification:
https://html.spec.whatwg.org/multipage/interaction.html#the-datatransferitemlist-interface
https://html.spec.whatwg.org/multipage/interaction.html#datatransferitem

Chrome seems to support it. It is also covered by the W3C web-platform tests.
Should we work on enabling this feature rather than removing it?

Kr,
--
 Chris Dumez




> On Dec 21, 2015, at 10:02 PM, Gyuyoung Kim <gyuyoung@webkit.org> wrote:
> 
> Hello,
> 
> It looks like no ports have used a data transfer items feature. Even the 
> feature hasn't been updated since 2011. If anyone doesn't have a plan to use 
> it in future, I plan to remove it. Any objections ?
> 
> Gyuyoung. 
> ___
> 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] [Proposal] Remove support for 'multipart/x-mixed-replace' main resources

2015-04-21 Thread Chris Dumez
Hi,

I would like to suggest we remove support for 'multipart/x-mixed-replace’ main 
resources while keeping support for multipart images.

Based on Chrome usage data, this feature is extremely rarely used by Web sites 
(less than 0.1% of page loads) [1]. This feature adds complexity to the 
loader and is a source of (security) bugs (e.g. [2] recently), current support 
also seems buggy.

Current support in Safari / WebKit:
- Support is not great is WebKit. If you load a Motion JPEG main resource for 
example, it will keep creating a new ImageDocument and all its DOM tree for 
every frame (tested on Safari / Mac).
- It looks like support is broken on Safari on iOS (I tried a Motion JPEG main 
resource on iOS8, I see the first frame then a blank page that never finishes 
loading).

Other browsers:
- Never supported by IE (including IE11) for any resource
- Chrome already dropped support for this (main resources only) almost 2 years 
ago [3].
- Firefox 37 still supports this based on local testing.

Again, I am only proposing dropping support for main resources. For e.g., 
having an IMG element in a page whose src attribute points to a Motion JPEG 
would still work as intended.

[1] https://code.google.com/p/chromium/issues/detail?id=249132 
https://code.google.com/p/chromium/issues/detail?id=249132
[2] https://bugs.webkit.org/show_bug.cgi?id=143979 
https://bugs.webkit.org/show_bug.cgi?id=143979
[3] http://src.chromium.org/viewvc/blink?view=revisionrevision=152363 
http://src.chromium.org/viewvc/blink?view=revisionrevision=152363

Kr,
--
 Chris Dumez - Apple Inc. - Cupertino, CA




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


Re: [webkit-dev] Safari browser on Mac OSX complains AudioContext.createMediaStreamSource is undefined !

2015-03-19 Thread Chris Dumez
Jer or Eric would know more about this but 
AudioContext.createMediaStreamSource() is behind a MEDIA_STREAM compile-time 
flag. It appears the mac port does not turn this flag on so Safari does not 
currently support the Media Stream API.

Kr,
--
 Chris Dumez - Apple Inc. - Cupertino, CA




 On Mar 19, 2015, at 9:35 AM, Sasi San sasikumar.gan...@gmail.com wrote:
 
 Hi-
 
 I am trying to get the live audio input from microphone using AudioContext. 
 Safari browser complains that the createMediaStreamSource is undefined. 
 here is my sample of JavaScript code. It's not able to create Audio source 
 node. So I am not able to get the audio sample from microphone in the 
 OnAudioProcess event handler.
 
 var micGain = audioContext.createGain();
 if(!audioContext.createScriptProcessor){
 micAudioProcessor = audioContext.createJavaScriptNode(bufferSize, 
 1, 1);
 console.log('createJavaScriptNode Done');
 } else {
 micAudioProcessor = 
 audioContext.createScriptProcessor(bufferSize, 1, 1);
console.log('createScriptProcessor Done');
 }
 
 // Create an AudioNode from the stream.
 if(typeof(audioContext.createMediaStreamSource) === 'function') {
 micInput = audioContext.createMediaStreamSource(inAudioStream);
 micInput.connect(micGain);
 }
 else {
 console.log('method createMediaStreamSource unavailable');
 }
 
 Please let me know why Safari is complaining on this API 
 createMediaStreamSource . Is there any other way I can get the live audio 
 sample from microphone on Safari browser???
 
 Thanks,
 Sasi
 
 ___
 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] run-webkit-tests and run-perf-tests scripts now use WebKitTestRunner by default

2015-03-05 Thread Chris Dumez
Hi,

I just wanted to let you know that run-webkit-tests and run-perf-tests now use 
WebKitTestRunner by default after [1].
Passing —webkit-test-runner or -2 to these scripts is no longer needed (and no 
longer works).

If you wish to run DumpRenderTree, you should pass —dump-render-tree or -1 to 
these scripts.

Kr,
--
 Chris Dumez - Apple Inc. - Cupertino, CA


[1] https://trac.webkit.org/r181093
 https://trac.webkit.org/r181093___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Pattern for singleton classes instance getters

2015-01-30 Thread Chris Dumez
Hi,

I made the changes in:
http://trac.webkit.org/changeset/179401
http://trac.webkit.org/changeset/179409

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA




 On Jan 29, 2015, at 10:23 AM, Chris Dumez cdu...@apple.com wrote:
 
 Hi all,
 
 Thanks for the feedback.
 
 I submitted a coding style update proposal at Bug 141040 
 https://bugs.webkit.org/show_bug.cgi?id=141040.
 Based on the mailing list feedback, the proposal is to use a static member 
 function named singleton()”.
 
 I will also upload a patch to align our existing code unless there is any 
 objection.
 
 Kr,
 --
 Chris Dumez - Apple Inc.
 Cupertino, CA
 
 
 
 
 On Jan 28, 2015, at 9:26 PM, Ryosuke Niwa rn...@webkit.org 
 mailto:rn...@webkit.org wrote:
 
 On Wed, Jan 28, 2015 at 9:19 PM, Maciej Stachowiak m...@apple.com 
 mailto:m...@apple.com wrote:
 
 This may be a question of what jargon we’ve encountered, but to me, 
 “singleton clearly means the one unique instance of this class while 
 “instance means any instance of this class. If I hadn’t seen this thread, 
 I would interpret Class::instance() to mean “create a brand new instance of 
 this class” rather than “return the unique singleton instance of this class, 
 creating if necessary.
 
 Agreed.  I also prefer Class::singleton() over Class::instance().
 
 - R. Niwa
 ___
 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] Pattern for singleton classes instance getters

2015-01-29 Thread Chris Dumez
Hi all,

Thanks for the feedback.

I submitted a coding style update proposal at Bug 141040 
https://bugs.webkit.org/show_bug.cgi?id=141040.
Based on the mailing list feedback, the proposal is to use a static member 
function named singleton()”.

I will also upload a patch to align our existing code unless there is any 
objection.

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA




 On Jan 28, 2015, at 9:26 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 On Wed, Jan 28, 2015 at 9:19 PM, Maciej Stachowiak m...@apple.com 
 mailto:m...@apple.com wrote:
 
 This may be a question of what jargon we’ve encountered, but to me, 
 “singleton clearly means the one unique instance of this class while 
 “instance means any instance of this class. If I hadn’t seen this thread, 
 I would interpret Class::instance() to mean “create a brand new instance of 
 this class” rather than “return the unique singleton instance of this class, 
 creating if necessary.
 
 Agreed.  I also prefer Class::singleton() over Class::instance().
 
 - 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] Pattern for singleton classes instance getters

2015-01-28 Thread Chris Dumez
Yes, instance() is what I’ve seen mostly outside WebKit as well. This would be 
my preference.

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA

 On Jan 28, 2015, at 8:44 PM, Michael Catanzaro mcatanz...@igalia.com wrote:
 
 On Wed, Jan 28, 2015 at 8:11 PM, Maciej Stachowiak m...@apple.com wrote:
 Yet another possibility is finding a better name than ‘shared’ for the 
 singleton pattern function, but I don’t have any better ideas. 
 Class::getSingleton() is more explicit but the extra verbosity doesn’t seem 
 helpful to me.
 
 I recommend Class::instance(), which is what I've seen used almost 
 exclusively outside of WebKit. It's what Scott Meyers used, and it's very 
 similar to the Gang of Four's choice of Class::Instance() and the Java 
 pattern Class.INSTANCE.
 
 That said, Class::singleton() is very attractive too.
 
 I've never seen Class::shared() used anywhere except WebKit. The first time I 
 saw this I had no clue what it was until I looked up the implementation.
 
 All of these can get quite annoying to type, especially when the name of the 
 class is long. (Sometimes I will keep a reference to the instance in a local 
 variable with a shorter name.) But I think they're easier to read than free 
 functions, and we should optimize for reading code, not writing it. Not a big 
 deal either way.
 ___
 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] Pattern for singleton classes instance getters

2015-01-28 Thread Chris Dumez
Fair enough, singleton() is very explicit indeed. I would be happy with this 
naming too.

--
Chris Dumez - Apple Inc.
Cupertino, CA

 On Jan 28, 2015, at 9:19 PM, Maciej Stachowiak m...@apple.com wrote:
 
 
 This may be a question of what jargon we’ve encountered, but to me, 
 “singleton clearly means the one unique instance of this class while 
 “instance means any instance of this class. If I hadn’t seen this thread, 
 I would interpret Class::instance() to mean “create a brand new instance of 
 this class” rather than “return the unique singleton instance of this class, 
 creating if necessary.
 
 Regards,
 Maciej
 
 On Jan 28, 2015, at 8:54 PM, Chris Dumez cdu...@apple.com 
 mailto:cdu...@apple.com wrote:
 
 Yes, instance() is what I’ve seen mostly outside WebKit as well. This would 
 be my preference.
 
 Kr,
 --
 Chris Dumez - Apple Inc.
 Cupertino, CA
 
 On Jan 28, 2015, at 8:44 PM, Michael Catanzaro mcatanz...@igalia.com 
 mailto:mcatanz...@igalia.com wrote:
 
 On Wed, Jan 28, 2015 at 8:11 PM, Maciej Stachowiak m...@apple.com 
 mailto:m...@apple.com wrote:
 Yet another possibility is finding a better name than ‘shared’ for the 
 singleton pattern function, but I don’t have any better ideas. 
 Class::getSingleton() is more explicit but the extra verbosity doesn’t 
 seem helpful to me.
 
 I recommend Class::instance(), which is what I've seen used almost 
 exclusively outside of WebKit. It's what Scott Meyers used, and it's very 
 similar to the Gang of Four's choice of Class::Instance() and the Java 
 pattern Class.INSTANCE.
 
 That said, Class::singleton() is very attractive too.
 
 I've never seen Class::shared() used anywhere except WebKit. The first time 
 I saw this I had no clue what it was until I looked up the implementation.
 
 All of these can get quite annoying to type, especially when the name of 
 the class is long. (Sometimes I will keep a reference to the instance in a 
 local variable with a shorter name.) But I think they're easier to read 
 than free functions, and we should optimize for reading code, not writing 
 it. Not a big deal either way.
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org mailto:webkit-dev@lists.webkit.org
 https://lists.webkit.org/mailman/listinfo/webkit-dev 
 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] Pattern for singleton classes instance getters

2015-01-28 Thread Chris Dumez
Hi,

I noticed that we are currently not very consistent in WebKit in the way we 
implement singleton classes instance getters.
- Some classes use free functions (like MemoryCache, and PageCache until I 
updated it yesterday). e.g. memoryCache().xxx()
- Some classes are using static functions in the class (e.g. 
DatabaseProcess::shared(), PluginProcess::shared()).

As I said, I landed a patch yesterday so that the global page cache is now 
accessed via PageCache::shared() because I thought this was the currently 
preferred pattern (given it seems very common in WebKit2 code).
However, I thought I would email webkit-dev to make sure this is actually the 
case and make sure we agree on a given pattern (one way or another) for current 
and future code. We could then maybe document this
as part of our coding style.

Any feedback on this matter?

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA




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


Re: [webkit-dev] EFL EWS bot failures

2015-01-13 Thread Chris Dumez
FYI, this still seems to be happening:
https://webkit-queues.appspot.com/results/6182699735711744

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA

 On Jan 9, 2015, at 6:55 AM, Gyuyoung Kim gyuyoung@webkit.org wrote:
 
 Hello Ossy,
 
 Now I'm running two machines for EFL EWS and buildbot. However one of the 
 machines has poor performance.
 It looks the poor machine has caused this problem so far. Unfortunately I'm 
 not able to replace it with new machine now.
 But let me try to replace/upgrade the poor performance machine in near future.
 
 Thank you for your effort and notification about this issue.
 
 br,
 Gyuyoung.
 
 
 On Fri, Jan 9, 2015 at 8:52 PM, Osztrogonác Csaba o...@inf.u-szeged.hu 
 mailto:o...@inf.u-szeged.hu wrote:
 Hi Gyuyoung,
 
 thanks for fixing the bots.
 
 I noticed similar problem on the EFL EWS and buildbot many times.
 Unfortunately it can cause not only orange bubbles, but false
 positive redness too.
 
 I saw the same error long long time ago locally, and the reason
 was OOM on one of an overloaded icecc slave many times. And once
 a memory module error caused the same error.
 
 Nowadays I noticed another strange error on the EFL buildbot (2 or 3
 times):  kill old processes buildstep stucked in an infinite loop
 and killed after the 20 minutes timeout for several builds.
 
 Is there any chance to check and fix/replace the hardware of
 the EFL EWS and buildbot to make it as stable as previously?
 
 br,
 Ossy
 
 Gyuyoung Kim írta:
 Hello Darin,
 
 I'm sorry for the inconvenience about that. EFL EWS seems to work again.
 
 Gyuyoung.
 
 On Fri, Jan 9, 2015 at 4:02 AM, Darin Adler da...@apple.com 
 mailto:da...@apple.com mailto:da...@apple.com mailto:da...@apple.com 
 wrote:
 
 A lot of times recently the EFL bot has reported a failure that
 shows up as an orange bubble:
 
 c++: internal compiler error: Killed (program cc1plus)
 
 Like in this bug https://bugs.webkit.org/show_bug.cgi?id=140166 
 https://bugs.webkit.org/show_bug.cgi?id=140166.
 What's going on with that? Is there someone who can fix it?
 
 --- Darin
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org mailto:webkit-dev@lists.webkit.org
 https://lists.webkit.org/mailman/listinfo/webkit-dev 
 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] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Chris Dumez
The corresponding Blink bug did contain some performance data:
CrBug#229226 https://code.google.com/p/chromium/issues/detail?id=229226

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA




 On Nov 20, 2014, at 11:32 AM, Geoffrey Garen gga...@apple.com wrote:
 
 I wonder what the downsides are to this approach.  Footprint of Vector?
 
 It looks like the original change was motivated by shrinking Vector:
 
   https://bugs.webkit.org/show_bug.cgi?id=97268 
 https://bugs.webkit.org/show_bug.cgi?id=97268
 
 Sadly, it didn’t include any data on the observed benefit :(.
 
 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


[webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Chris Dumez
Hi all,

I recently started updating the WTF::Vector API to use unsigned types instead 
of size_t [1][2], because:
- WTF::Vector is already using unsigned type for size/capacity internally to 
save memory on 64-bit, causing a mismatch between the API and the internal 
representation [3]
- Some reviewers have asked me to use unsigned for loop counters iterating over 
vectors (which looks unsafe as the Vector API, e.g. size(), returns a size_t).
- I heard from Joseph that this type mismatch is forcing us (and other projects 
using WTF) to disable some build time warnings
- The few people I talked to before making that change said we should do it

However, Alexey recently raised concerns about this change. it doesn't strike 
him as the right direction. 4Gb is not much, and we should have more of WebKit 
work with the right data types, not less.”.
I did not initially realize that this change was controversial, but now that it 
seems it is, I thought I would raise the question on webkit-dev to see what 
people think about this.

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA


[1] http://trac.webkit.org/changeset/176275 
http://trac.webkit.org/changeset/176275
[2] http://trac.webkit.org/changeset/176293 
http://trac.webkit.org/changeset/176293
[3] http://trac.webkit.org/changeset/148891 
http://trac.webkit.org/changeset/148891
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Chris Dumez
Hi,

I believe the Vector class is already checking for overflows. I looked quickly 
and it seems that when trying to allocate / reallocate the Vector buffer, we 
will call CRASH() if:
(newCapacity  std::numeric_limitsunsigned::max() / sizeof(T))

Kr, 
--
Chris Dumez - Apple Inc.
Cupertino, CA




 On Nov 19, 2014, at 1:57 PM, Geoffrey Garen gga...@apple.com wrote:
 
 I don’t haver a specific opinion on size_t vs unsigned, but I have a related 
 point:
 
 If Vector uses unsigned, then it must RELEASE_ASSERT that it does not 
 overflow unsigned when growing. 
 
 Even if most normal content allocates  4GB, exploit content surely will try 
 to allocate  4GB.
 
 Chris, maybe you can make this change in trunk, since trunk uses unsigned?
 
 Geoff
 
 On Nov 19, 2014, at 1:50 PM, Alexey Proskuryakov a...@webkit.org 
 mailto:a...@webkit.org wrote:
 
 
 That makes good sense for internal implementation, do you think that class 
 API should also use a custom type, or should it use size_t?
 
 With Vector though, I don't know how we would differentiate code paths that 
 need large allocations from ones that don't. Nearly anything that is exposed 
 as a JS API or deals with external world can hit sizes over 4Gb. That's not 
 out of reach in most scenarios, not even for resources loaded from network.
 
 - Alexey
 
 
 19 нояб. 2014 г., в 13:19, Filip Pizlo fpi...@apple.com 
 mailto:fpi...@apple.com написал(а):
 
 ArrayBuffers are very special because they are part of ECMAScript.
 
 We use unsigned for the length, because once upon a time, that would have 
 been the right type; these days the right type would be a 53-bit integer.  
 So, size_t would be wrong.  I believe that ArrayBuffers should be changed 
 to use a very special size type; probably it wouldn't even be a primitive 
 type but a class that carefully checked that you never overflowed 53 bits.
 
 -Filip
 
 
 On Nov 19, 2014, at 12:54 PM, Alexey Proskuryakov a...@webkit.org 
 mailto:a...@webkit.org wrote:
 
 
 This is not exactly about Vector, but if one uses 
 FileReader.prototype.readAsArrayBuffer() on a large file, I think that it 
 overflows ArrayBuffer. WebKit actually crashes when uploading 
 multi-gigabyte files to YouTube, Google Drive and other similar services, 
 although I haven't checked whether it's because of ArrayBuffers, or 
 because of a use of int/unsigned in another code path.
 
 I think that we should use size_t everywhere except for perhaps a few key 
 places where memory impact is critical (and of course we need explicit 
 checks when casting down to an unsigned). Or perhaps the rule can be even 
 simpler, and unsigned may never be used for indices and sizes, period.
 
 - Alexey
 
 
 19 нояб. 2014 г., в 12:32, Filip Pizlo fpi...@apple.com 
 mailto:fpi...@apple.com написал(а):
 
 Whatever we do, the clients of Vector should be consistent about what 
 type they use.  I'm actually fine with Vector internally using unsigned 
 even if the API uses size_t, but right now we have lots of code that uses 
 a mix of size_t and unsigned when indexing into Vectors.  That's 
 confusing.
 
 If I picked one type to use for all Vector indices, it would be unsigned 
 rather than size_t.  Vector being limited to unsigned doesn't imply 4GB 
 unless you do Vectorchar.  Usually we have Vectors of pointer-width 
 things, which means 32GB on 64-bit systems (UINT_MAX * sizeof(void*)).  
 Even in a world where we had more than 32GB of memory to use within a 
 single web process, I would hope that we wouldn't use it all on a single 
 Vector and that if we did, we would treat that one specially for a bunch 
 of other sensible reasons (among them being that allocating a contiguous 
 slab of virtual memory of that size is rather taxing).  So, size_t would 
 buy us almost nothing since if we had a vector grow large enough to need 
 it, we would be having a bad time already.
 
 I wonder: are there cases that anyone remembers where we have tried to 
 use Vector to store more than UINT_MAX things?  Another interesting 
 question is: What's the largest number of things that we store into any 
 Vector?  We could use such a metric to project how big Vectors might get 
 in the future.
 
 -Filip
 
 
 On Nov 19, 2014, at 12:20 PM, Chris Dumez cdu...@apple.com 
 mailto:cdu...@apple.com wrote:
 
 Hi all,
 
 I recently started updating the WTF::Vector API to use unsigned types 
 instead of size_t [1][2], because:
 - WTF::Vector is already using unsigned type for size/capacity 
 internally to save memory on 64-bit, causing a mismatch between the API 
 and the internal representation [3]
 - Some reviewers have asked me to use unsigned for loop counters 
 iterating over vectors (which looks unsafe as the Vector API, e.g. 
 size(), returns a size_t).
 - I heard from Joseph that this type mismatch is forcing us (and other 
 projects using WTF) to disable some build time warnings
 - The few people I talked to before making that change said we should do 
 it
 
 However, Alexey recently

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Chris Dumez
If we don’t want to crash on overflow, the callers can use the try*() API I 
believe (e.g. tryAppend()). This returns false (and does not resize the vector) 
instead of crashing, when we reach the size limit.

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA




 On Nov 19, 2014, at 2:58 PM, Alexey Proskuryakov a...@webkit.org wrote:
 
 
 19 нояб. 2014 г., в 13:58, Filip Pizlo fpi...@apple.com 
 mailto:fpi...@apple.com написал(а):
 
 With Vector though, I don't know how we would differentiate code paths that 
 need large allocations from ones that don't. Nearly anything that is 
 exposed as a JS API or deals with external world can hit sizes over 4Gb. 
 That's not out of reach in most scenarios, not even for resources loaded 
 from network.
 
 Can you provide an example?
 
 Yes. XMLHttpRequest::m_binaryResponseBuilder keeps the downloaded data in a 
 Vector, so any time there is much data, something bad will happen. This is a 
 case that we should support, and not just crash as we would when we think 
 that only exploits would try to use as much memory.
 
 All code that is Blob related also uses Vectors, and of course Blobs can 
 legitimately be large.
 
 Crypto code uses Vectors internally for the data.
 
 These and related uses are all over the place - see also Vectors in 
 FormDataBuilder, data returned from FrameLoader::loadResourceSynchronously, 
 plug-in code that loads from network, SharedBuffer etc.
 
 - Alexey
 

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


[webkit-dev] Transition to is() / downcast() is complete

2014-10-20 Thread Chris Dumez
Hi all,

I just wanted to let you know that I have just landed the last patch porting 
the code base to using is() / downcast().
I have managed to remove the NODE_TYPE_CASTS() / TYPE_CASTS_BASE() macros so 
there should be no code using it anymore.

If you need to add is() / downcast() support for a specific class, please 
use the SPECIALIZE_TYPE_TRAITS_*() macro that is in
wtf/TypeCasts.h. Note however, that the template specializations are already 
generated for most HTML/SVG/MathML elements by
default so you rarely need manual traits specialization for those.

If you find remaining toXXX() casting functions in the code, please let me know 
as we don’t want to mix toXXX() and downcast().
Do let me know if you have improvement suggestions as well. One that is on my 
TODO list already is to support is(RefPtr), there are
not that many call sites that would benefit from this but it would be nice IMHO.

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA

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


[webkit-dev] Behavior change: is(T*) is now doing a null check on the pointer argument

2014-10-02 Thread Chris Dumez
Hi,

I would like to bring attention to a behavior change I have just landed [1]. 
The is(T*) type checking function is now doing a null-check on the pointer 
argument instead of simply asserting that the pointer argument should not be 
null.

Please keep this in mind in your future patches:
- If you know the pointer cannot be null (because a null check was done earlier 
in the function), then call the is(T) overload taking a reference in 
argument to avoid the null check: e.g. “if (isHTMLDivElement(*node)) …
- Do not do an explicit null check on the pointer before calling is(T*). “if 
(node  isHTMLDivElement(node))” - if (isHTMLDivElement(node))

This change was made because:
- It simplifies the code a bit as it avoids a lot of explicit null checks.
- It is more consistent with downcast(T*) which will correctly cast a null 
pointer.
- Having a is(T*) overload in addition to is(T) was previously only 
helpful to avoid having to dereference the pointer. Dereferencing is just one 
‘*’ character and doesn’t decrease code readability much IMHO. However, getting 
rid of a lot of explicit null checks does simplify the code quite a bit.

FYI, all Node subclasses should now support is() / downcast() already. I am 
in the process of porting non-Node classes over as well for consistency 
(WorkerGlobalScope class, File class and Event subclasses were already ported 
for e.g.).
If you try to use is() / downcast() for a class that doesn’t support it 
yet, you should now get a clear build error (instead of the obscure linking 
error we would get previously).

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA

[1]  http://trac.webkit.org/changeset/174225 
http://trac.webkit.org/changeset/174225___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] Type checking / casting helpers

2014-09-25 Thread Chris Dumez
Hi all,

I started working on automatically generating the type casting helpers for 
HTML/SVG/MathML Elements (e.g. toHTMLDivElement()). Until now, we were 
generating only the type checking helpers using make_names.pl (e.g. 
isHTMLDivElement()). The type casting helpers had to be manually defined using 
NODE_TYPE_CASTS() macro.

The type casting helpers are now automatically generated for most types. Part 
of the solution involved using a templated function for type casting because 
the types are forward-declared and we needed to do a static_cast() (a 
reinterpret_cast() could be used with forward declarations but wouldn’t be 
safe due to multiple inheritance).

I initially had macros in place so that toHTMLDivElement() would still work and 
would be equivalent to downcastHTMLDivElement(). The feedback I received is 
that we should get rid of these macros and just use isHTMLDivElement() / 
downcastHTMLDivElement() everywhere.
The new style is very close to C++’s is_classT() and Boost’s 
polymorphic_downcastT().

I actually started updating the code to do this but I should have emailed 
webkit-dev about this beforehand. I apologize for sending this message a bit 
late.

Please let me know if you have feedback / concerns / questions about this 
change. I hope that this email gives you a better understanding of why I am 
making this change.

As I said before, the code base is not fully ported yet so the current 
situation is not necessarily pretty. I will try and go through the transition 
as fast as I can, provided that people don’t raise any concerns about this.

Please also note that these new helpers still catch unnecessary type checks / 
casts. As a matter of fact, those are now caught at build time instead of 
linking time and should give you a nice “Unnecessary type check” / “Unnecessary 
type cast” static assertion.

Also note that the plan is to get rid of TYPE_CAST_BASE() macro entirely and 
extend is() / downcast() to all types, not just Nodes.

Kr,
--
Chris Dumez - Apple Inc.
Cupertino, CA




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