As was done for ReadableStreamReader/ReadableStreamDefaultReader, ReadableStreamController should be replaced by ReadableStreamDefaultController to align with Streams API spec.
Created attachment 284693 [details] Patch
Attachment 284693 [details] did not pass style-queue: ERROR: Source/WebCore/bindings/js/ReadableStreamDefaultController.h:46: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 1 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 284693 [details] Patch r=me. It might be good to improve the controller code. See below for all possible improvements. This can be done now or in a later patch. Wdyt romain? View in context: https://bugs.webkit.org/attachment.cgi?id=284693&action=review > Source/WebCore/bindings/js/ReadableStreamDefaultController.cpp:36 > +#include "JSReadableStreamSource.h" These two includes may no longer be needed > Source/WebCore/bindings/js/ReadableStreamDefaultController.cpp:77 > + auto isLocked = globalObject()->builtinInternalFunctions().readableStreamInternals().m_isReadableStreamLockedFunction.get(); Can we add an ASSERT(isLocked.isFunction())? > Source/WebCore/bindings/js/ReadableStreamDefaultController.h:55 > + void close() { invoke(*globalObject()->globalExec(), *m_jsController, "close", JSC::jsUndefined()); } Can we add a jsConstroller() method helper that would do ASSERT(m_jsController); Return m_jsController; > Source/WebCore/bindings/js/ReadableStreamDefaultController.h:63 > + JSDOMGlobalObject* globalObject() const; Might be better to return a reference than a pointer. > Source/WebCore/bindings/js/ReadableStreamDefaultController.h:72 > + return static_cast<JSDOMGlobalObject*>(m_jsController->globalObject()); Might be better to add an ASSERT and return a JSDOMGlobalObject& > Source/WebCore/bindings/js/ReadableStreamDefaultController.h:77 > + JSC::ExecState& state = *globalObject()->globalExec(); Might be good to add an ASSERT(globalObject()->globalExec()) > Source/WebCore/bindings/js/ReadableStreamDefaultController.h:96 > + JSC::JSLockHolder locker(state); Ditto. > Source/WebCore/bindings/js/WebCoreBuiltinNames.h:100 > + macro(ReadableStreamDefaultController) \ Switch reader and controller lines?
Created attachment 284781 [details] Patch
Attachment 284781 [details] did not pass style-queue: ERROR: Source/WebCore/bindings/js/ReadableStreamDefaultController.h:46: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 1 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #3) > Comment on attachment 284693 [details] > Patch > > r=me. > It might be good to improve the controller code. > See below for all possible improvements. > This can be done now or in a later patch. > Wdyt romain? For this patch I was focusing on the JS part. I just uploaded a new patch that fixes alphabetical order in WebCoreBuiltinNames.h, but I let other remarks aiming at improving ReadableStreamDefaultController.h/cpp for another patch. I'm going to create a bug about that.
Comment on attachment 284781 [details] Patch Clearing flags on attachment: 284781 Committed r203818: <http://trac.webkit.org/changeset/203818>
All reviewed patches have been landed. Closing bug.