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

2015-01-27 Thread 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 :

 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@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] Problem with a crash using JSC code

2015-01-26 Thread youenn fablet
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 :
> 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@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] Problem with a crash using JSC code

2015-01-21 Thread Xabier Rodríguez Calvar
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



signature.asc
Description: This is a digitally signed message part
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev