[webkit-dev] Problem with a crash using JSC code

[Benjamin Poulain]

There is not formal size limit for any patch, some changes are 
inherently big.

Experience shows that big patch tends to have more hidden bugs and 
poorer code coverage. It is better to go forward by little step while 
having huge test coverage at every step.

https://bugs.webkit.org/attachment.cgi?id=245350&action=review looks 
hard to split.

Maybe do a first patch with an empty shell + the bindings. You can test 
passing all kind of valid and invalid values to the API; test the 
property of the object and every function/properties, test creating 
thousands of ReadableStream objects, etc.

Then a follow up patch implementing the ReadableStream and have an other 
set of test checking the behavior?


On 1/26/15 3:22 AM, youenn fablet wrote:
> The latest patch at https://bugs.webkit.org/show_bug.cgi?id=138967
> resolves the crash (some JSC::Strong<> were missing).
>
> I fear that the patch may be a bit too big to get a thorough review though.
> The patch could be split into meaningful but not testable sub-patches
> (module stuff, JS integration stuff and then tests and more tests).
> Would that make sense?
> What is a reasonable patch size limit?
>
> Any advice well appreciated.
>
> Thanks,
>     Youenn
>
>
> 2015-01-21 19:40 GMT+01:00 Xabier Rodríguez Calvar <calvaris at igalia.com>:
>>          Hi!
>>
>> I am now implementing with Youenn the Streams API standard [1] in
>> WebKit. You can find the first patch at [2] (it's r? now). While we get
>> that patch reviewed and landed we are adding more tests and working out
>> the problems. One of them is one crash that I cannot hunt, with the
>> following backtrace:
>>
>> http://fpaste.org/172619/60635142/
>>
>> You can find the code under the lines to make it easier. What is going
>> on is:
>>
>>       1. There's a call to the ReadableStream object, delegated to the
>>          JSReadableStreamSource as a result of the object creation.
>>       2. There's a call to the JSReadableStream::read method, delegating
>>          in the ReadableStream that ends up pulling again and that second
>>          call crashes.
>>
>> It is probably something stupid I am not taking into account, but I have
>> already been fighting this for a couple of days and cannot make it work
>> properly.
>>
>> Any help? Thanks a lot in advance!
>>
>> [1] https://streams.spec.whatwg.org/
>> [2] https://bugs.webkit.org/show_bug.cgi?id=138967
>>
>>
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>