Changeset 183803 in webkit


Ignore:
Timestamp:
May 5, 2015 6:23:52 AM (9 years ago)
Author:
youenn.fablet@crf.canon.fr
Message:

streams/readable-stream.html is very flaky
https://bugs.webkit.org/show_bug.cgi?id=144455

Reviewed by Darin Adler.

Source/WebCore:

Changed the link between readadable stream and controller.
Controller ref()/deref() now increments/decrements its stream ref counter.
This ensures that even if JS scripts do not keep track of the readable stream,
the readable stream will not be disposed as long as the JS script has access to its controller.

Test: streams/readable-stream-gc.html

  • Modules/streams/ReadableStreamController.h:

(WebCore::ReadableStreamController::ReadableStreamController):
(WebCore::ReadableStreamController::ref):
(WebCore::ReadableStreamController::deref):
(WebCore::ReadableStreamController::create): Deleted.
(WebCore::ReadableStreamController::stream): Deleted.

  • bindings/js/JSReadableStreamControllerCustom.cpp:

(WebCore::JSReadableStreamController::close):
(WebCore::JSReadableStreamController::enqueue):
(WebCore::JSReadableStreamController::error):

  • bindings/js/ReadableStreamJSSource.cpp:

(WebCore::ReadableStreamJSSource::~ReadableStreamJSSource):
(WebCore::ReadableStreamJSSource::start):
(WebCore::ReadableJSStream::jsController):

  • bindings/js/ReadableStreamJSSource.h:

LayoutTests:

Moved flaky test to streams/readable-stream-gc.html.
Updated flaky test to check that the controller methods work well even if readable stream reference is lost by script.

  • streams/readable-stream-expected.txt:
  • streams/readable-stream-gc-expected.txt: Added.
  • streams/readable-stream-gc.html: Added.
  • streams/readable-stream.html:
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r183802 r183803  
     12015-05-05  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
     2
     3        streams/readable-stream.html is very flaky
     4        https://bugs.webkit.org/show_bug.cgi?id=144455
     5
     6        Reviewed by Darin Adler.
     7
     8        Moved flaky test to streams/readable-stream-gc.html.
     9        Updated flaky test to check that the controller methods work well even if readable stream reference is lost by script.
     10
     11        * streams/readable-stream-expected.txt:
     12        * streams/readable-stream-gc-expected.txt: Added.
     13        * streams/readable-stream-gc.html: Added.
     14        * streams/readable-stream.html:
     15
    1162015-05-05  Marcos Chavarría Teijeiro  <chavarria1991@gmail.com>
    217
  • trunk/LayoutTests/streams/readable-stream-expected.txt

    r183107 r183803  
    33PASS ReadableStream start should be called with the proper parameters
    44PASS ReadableStream should be able to call start method within prototype chain of its source
    5 PASS A readable stream controller methods should throw if its readablestream is collected
    65
  • trunk/LayoutTests/streams/readable-stream.html

    r183107 r183803  
    22<script src='../resources/testharness.js'></script>
    33<script src='../resources/testharnessreport.js'></script>
    4 <script src='../resources/gc.js'></script>
    54<script>
    65test(function() {
     
    5958}, 'ReadableStream should be able to call start method within prototype chain of its source');
    6059
    61 t1 = async_test('A readable stream controller methods should throw if its readablestream is collected');
    62 t1.step(function() {
    63     var controller;
    64     new ReadableStream({
    65         start: function(c) {
    66             controller = c;
    67         }
    68     });
    69     setTimeout(t1.step_func(function() {
    70         window.gc();
    71         assert_throws(new TypeError(), function() { controller.close(); });
    72         assert_throws(new TypeError(), function() { controller.error(); });
    73         assert_throws(new TypeError(), function() { controller.enqueue(); });
    74         t1.done();
    75     }), 10);
    76 });
    7760</script>
  • trunk/Source/WebCore/ChangeLog

    r183799 r183803  
     12015-05-05  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
     2
     3        streams/readable-stream.html is very flaky
     4        https://bugs.webkit.org/show_bug.cgi?id=144455
     5
     6        Reviewed by Darin Adler.
     7
     8        Changed the link between readadable stream and controller.
     9        Controller ref()/deref() now increments/decrements its stream ref counter.
     10        This ensures that even if JS scripts do not keep track of the readable stream,
     11        the readable stream will not be disposed as long as the JS script has access to its controller.
     12
     13        Test: streams/readable-stream-gc.html
     14
     15        * Modules/streams/ReadableStreamController.h:
     16        (WebCore::ReadableStreamController::ReadableStreamController):
     17        (WebCore::ReadableStreamController::ref):
     18        (WebCore::ReadableStreamController::deref):
     19        (WebCore::ReadableStreamController::create): Deleted.
     20        (WebCore::ReadableStreamController::stream): Deleted.
     21        * bindings/js/JSReadableStreamControllerCustom.cpp:
     22        (WebCore::JSReadableStreamController::close):
     23        (WebCore::JSReadableStreamController::enqueue):
     24        (WebCore::JSReadableStreamController::error):
     25        * bindings/js/ReadableStreamJSSource.cpp:
     26        (WebCore::ReadableStreamJSSource::~ReadableStreamJSSource):
     27        (WebCore::ReadableStreamJSSource::start):
     28        (WebCore::ReadableJSStream::jsController):
     29        * bindings/js/ReadableStreamJSSource.h:
     30
    1312015-05-05  Myles C. Maxfield  <mmaxfield@apple.com>
    232
  • trunk/Source/WebCore/Modules/streams/ReadableStreamController.h

    r183107 r183803  
    3333#if ENABLE(STREAMS_API)
    3434
    35 #include <wtf/Ref.h>
    36 #include <wtf/RefCounted.h>
     35#include "ReadableStreamJSSource.h"
    3736
    3837namespace WebCore {
    39 
    40 class ReadableJSStream;
    4138
    4239// This class is only used for JS source readable streams to allow enqueuing, closing or erroring a readable stream.
    4340// Its definition is at https://streams.spec.whatwg.org/#rs-controller-class.
    4441// Note that its constructor is taking a ReadableJSStream as it should only be used for JS sources.
    45 class ReadableStreamController : public RefCounted<ReadableStreamController> {
     42class ReadableStreamController {
    4643public:
    47     static Ref<ReadableStreamController> create(ReadableJSStream& stream)
    48     {
    49         auto controller = adoptRef(*new ReadableStreamController(stream));
    50         return controller;
    51     }
    52     ~ReadableStreamController() { }
     44    explicit ReadableStreamController(ReadableJSStream& stream)
     45        : m_stream(stream) { }
    5346
    54     void resetStream() { m_stream = nullptr; }
    55     ReadableJSStream* stream() { return m_stream; }
     47    ReadableJSStream& stream() { return m_stream; }
     48
     49    void ref() { m_stream.ref(); }
     50    void deref() { m_stream.deref(); }
    5651
    5752private:
    58     ReadableStreamController(ReadableJSStream& stream) { m_stream = &stream; }
    59 
    60     ReadableJSStream* m_stream;
     53    ReadableJSStream& m_stream;
    6154};
    6255
  • trunk/Source/WebCore/bindings/js/JSReadableStreamControllerCustom.cpp

    r183395 r183803  
    4444JSValue JSReadableStreamController::close(ExecState* exec)
    4545{
    46     ReadableJSStream* stream = impl().stream();
    47     if (!stream)
    48         return exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Controller has no readablestream")));
     46    ReadableJSStream& stream = impl().stream();
    4947    // FIXME: Handle the case of draining.
    50     if (stream->internalState() != ReadableStream::State::Readable)
     48    if (stream.internalState() != ReadableStream::State::Readable)
    5149        return exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Calling close on a stream which is not readable")));
    52     stream->changeStateToClosed();
     50    stream.changeStateToClosed();
    5351    return jsUndefined();
    5452}
    5553
    56 JSValue JSReadableStreamController::enqueue(ExecState* exec)
     54JSValue JSReadableStreamController::enqueue(ExecState*)
    5755{
    58     ReadableJSStream* stream = impl().stream();
    59     if (!stream)
    60         return exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Controller has no readablestream")));
    6156    notImplemented();
    6257    return jsBoolean(false);
     
    6560JSValue JSReadableStreamController::error(ExecState* exec)
    6661{
    67     ReadableJSStream* stream = impl().stream();
    68     if (!stream)
    69         return exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Controller has no readablestream")));
    70     if (stream->internalState() != ReadableStream::State::Readable)
     62    ReadableJSStream& stream = impl().stream();
     63    if (stream.internalState() != ReadableStream::State::Readable)
    7164        return exec->vm().throwException(exec, createTypeError(exec, ASCIILiteral("Calling error on a stream which is not readable")));
    7265    notImplemented();
  • trunk/Source/WebCore/bindings/js/ReadableStreamJSSource.cpp

    r183744 r183803  
    3636#include "JSDOMPromise.h"
    3737#include "JSReadableStream.h"
     38#include "JSReadableStreamController.h"
    3839#include "NotImplemented.h"
    3940#include "ScriptExecutionContext.h"
     
    9192ReadableStreamJSSource::~ReadableStreamJSSource()
    9293{
    93     if (m_controller)
    94         m_controller.get()->impl().resetStream();
    9594}
    9695
     
    112111    JSLockHolder lock(&exec);
    113112
    114     Ref<ReadableStreamController> controller = ReadableStreamController::create(readableStream);
    115     m_controller.set(exec.vm(), jsDynamicCast<JSReadableStreamController*>(toJS(&exec, globalObject(), controller)));
    116 
    117113    JSValue startFunction = getPropertyFromObject(&exec, m_source.get(), "start");
    118114    if (!startFunction.isFunction()) {
     
    125121
    126122    MarkedArgumentBuffer arguments;
    127     arguments.append(m_controller.get());
     123    arguments.append(readableStream.jsController(exec, globalObject()));
    128124
    129125    JSValue exception;
     
    162158}
    163159
     160JSValue ReadableJSStream::jsController(ExecState& exec, JSDOMGlobalObject* globalObject)
     161{
     162    if (!m_controller)
     163        m_controller = std::make_unique<ReadableStreamController>(*this);
     164    return toJS(&exec, globalObject, m_controller.get());
     165}
     166
    164167Ref<ReadableJSStream::Reader> ReadableJSStream::Reader::create(ReadableJSStream& stream)
    165168{
  • trunk/Source/WebCore/bindings/js/ReadableStreamJSSource.h

    r183744 r183803  
    3333#if ENABLE(STREAMS_API)
    3434
    35 #include "JSReadableStreamController.h"
    3635#include "ReadableStream.h"
    3736#include "ReadableStreamReader.h"
     
    4443
    4544namespace WebCore {
     45
     46class JSDOMGlobalObject;
     47class ReadableJSStream;
     48class ReadableStreamController;
    4649
    4750class ReadableStreamJSSource: public ReadableStreamSource {
     
    5861    // Object passed to constructor.
    5962    JSC::Strong<JSC::JSObject> m_source;
    60 
    61     JSC::Strong<JSReadableStreamController> m_controller;
    6263};
    6364
     
    6667    static Ref<ReadableJSStream> create(JSC::ExecState&, ScriptExecutionContext&);
    6768    virtual Ref<ReadableStreamReader> createReader() override;
     69
    6870    ReadableStreamJSSource& jsSource();
     71    JSC::JSValue jsController(JSC::ExecState&, JSDOMGlobalObject*);
    6972
    7073private:
     
    7780        explicit Reader(ReadableJSStream&);
    7881    };
     82
     83    std::unique_ptr<ReadableStreamController> m_controller;
    7984};
    8085
Note: See TracChangeset for help on using the changeset viewer.