[Streams API] Shield implementation from mangling then and catch promise methods
authorcalvaris@igalia.com <calvaris@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Nov 2015 16:12:37 +0000 (16:12 +0000)
committercalvaris@igalia.com <calvaris@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Nov 2015 16:12:37 +0000 (16:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=150934

Reviewed by Youenn Fablet.

Source/JavaScriptCore:

Since the prototype is not deletable and readonly we only have to care about ensuring that it has the right
@then and @catch internal methods.

* runtime/JSPromisePrototype.h:
* runtime/JSPromisePrototype.cpp:
(JSC::JSPromisePrototype::addOwnInternalSlots): Added to create the proper @then and @catch internal slots.
(JSC::JSPromisePrototype::create): Call addOwnInternalSlots.

Source/WebCore:

This is a first step to get streams code shielded from user replacing the then and catch methods in our
promises. We use newly introduced @then and @catch prototype internal slots and that should solve a lot of use
cases.

Test: streams/streams-promises.html.

* Modules/streams/ReadableStream.js:
(initializeReadableStream):
* Modules/streams/ReadableStreamInternals.js:
(teeReadableStream):
(teeReadableStreamPullFunction):
(cancelReadableStream):
* Modules/streams/WritableStream.js:
(initializeWritableStream):
(abort):
* Modules/streams/WritableStreamInternals.js:
(callOrScheduleWritableStreamAdvanceQueue):

LayoutTests:

* streams/streams-promises.html: Added tests from about replacing
the prototype, then and catch methods. Renamed all tests as well.
* streams/streams-promises-expected.txt: Added expectations.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@192157 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/streams/streams-promises-expected.txt
LayoutTests/streams/streams-promises.html
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSPromisePrototype.cpp
Source/JavaScriptCore/runtime/JSPromisePrototype.h
Source/WebCore/ChangeLog
Source/WebCore/Modules/streams/ReadableStream.js
Source/WebCore/Modules/streams/ReadableStreamInternals.js
Source/WebCore/Modules/streams/WritableStream.js
Source/WebCore/Modules/streams/WritableStreamInternals.js

index e049787..53b4105 100644 (file)
@@ -1,3 +1,14 @@
+2015-11-09  Xabier Rodriguez Calvar  <calvaris@igalia.com>
+
+        [Streams API] Shield implementation from mangling then and catch promise methods
+        https://bugs.webkit.org/show_bug.cgi?id=150934
+
+        Reviewed by Youenn Fablet.
+
+        * streams/streams-promises.html: Added tests from about replacing
+        the prototype, then and catch methods. Renamed all tests as well.
+        * streams/streams-promises-expected.txt: Added expectations.
+
 2015-11-02  Sergio Villar Senin  <svillar@igalia.com>
 
         [css-grid] Improve grid container sizing with size constraints and intrinsic sizes
index 204aaea..f3fce48 100644 (file)
@@ -1,5 +1,9 @@
 
-PASS Streams implementation is not affected if Promise constructor is replaced 
-PASS Streams implementation is not affected if Promise.resolve is replaced 
-PASS Streams implementation is not affected if Promise.reject is replaced 
+PASS Streams and promises: replace Promise constructor 
+PASS Streams and promises: replace Promise.resolve 
+PASS Streams and promises: replace Promise.reject 
+FAIL Streams and promises: replace prototype of a promise object undefined is not a function
+PASS Streams and promises: replace then method in Promise prototype 
+PASS Streams and promises: replace catch method in Promise prototype 
+PASS Streams and promises: replace then method in promise object 
 
index e559b1b..1aac46a 100644 (file)
@@ -2,7 +2,6 @@
 <script src='../resources/testharness.js'></script>
 <script src='../resources/testharnessreport.js'></script>
 <script>
-
 test(function() {
     const PromiseBackup = Promise;
 
@@ -14,7 +13,7 @@ test(function() {
     } finally {
         Promise = PromiseBackup;
     }
-}, 'Streams implementation is not affected if Promise constructor is replaced');
+}, 'Streams and promises: replace Promise constructor');
 
 test(function() {
     const PromiseResolveBackup = Promise.resolve;
@@ -27,7 +26,7 @@ test(function() {
     } finally {
         Promise.resolve = PromiseResolveBackup;
     }
-}, 'Streams implementation is not affected if Promise.resolve is replaced');
+}, 'Streams and promises: replace Promise.resolve');
 
 test(function() {
     const PromiseRejectBackup = Promise.reject;
@@ -40,5 +39,51 @@ test(function() {
     } finally {
         Promise.reject = PromiseRejectBackup;
     }
-}, 'Streams implementation is not affected if Promise.reject is replaced');
+}, 'Streams and promises: replace Promise.reject');
+
+test(function() {
+    function createMangledPromise() {
+        const promise = Promise.resolve();
+        Object.setPrototypeOf(promise, { constructor: Promise, then: function() { assert_unreached("streams should not use this promise then method"); } });
+        return promise;
+    }
+    new ReadableStream({ start: function() { return createMangledPromise(); } })
+    new WritableStream({ start: function() { return createMangledPromise(); } })
+}, 'Streams and promises: replace prototype of a promise object');
+
+test(function() {
+    const PromiseThenBackup = Promise.prototype.then;
+
+    try {
+        Promise.prototype.then = function() { assert_unreached("streams should not use this Promise.prototype.then method"); };
+
+        new ReadableStream();
+        new WritableStream();
+    } finally {
+        Promise.prototype.then = PromiseThenBackup;
+    }
+}, 'Streams and promises: replace then method in Promise prototype');
+
+test(function() {
+    const PromiseCatchBackup = Promise.prototype.catch;
+
+    try {
+        Promise.prototype.catch = function() { assert_unreached("streams should not use this Promise.prototype.catch method"); };
+
+        const rs = new ReadableStream();
+        rs.tee();
+    } finally {
+        Promise.prototype.catch = PromiseCatchBackup;
+    }
+}, 'Streams and promises: replace catch method in Promise prototype');
+
+test(function() {
+    function createMangledPromise() {
+        const promise = Promise.resolve();
+        promise.then = function() { assert_unreached("streams should not use this promise then method"); };
+        return promise;
+    }
+    new ReadableStream({ start: function() { return createMangledPromise(); } })
+    new WritableStream({ start: function() { return createMangledPromise(); } })
+}, 'Streams and promises: replace then method in promise object');
 </script>
index 685d24e..727abd9 100644 (file)
@@ -1,3 +1,18 @@
+2015-11-09  Xabier Rodriguez Calvar  <calvaris@igalia.com>
+
+        [Streams API] Shield implementation from mangling then and catch promise methods
+        https://bugs.webkit.org/show_bug.cgi?id=150934
+
+        Reviewed by Youenn Fablet.
+
+        Since the prototype is not deletable and readonly we only have to care about ensuring that it has the right
+        @then and @catch internal methods.
+
+        * runtime/JSPromisePrototype.h:
+        * runtime/JSPromisePrototype.cpp:
+        (JSC::JSPromisePrototype::addOwnInternalSlots): Added to create the proper @then and @catch internal slots.
+        (JSC::JSPromisePrototype::create): Call addOwnInternalSlots.
+
 2015-11-09  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 
         JS Built-ins functions should be able to assert
index 1bc5a11..5c7eb5a 100644 (file)
 #include "config.h"
 #include "JSPromisePrototype.h"
 
+#include "BuiltinNames.h"
 #include "Error.h"
 #include "JSCBuiltins.h"
 #include "JSCJSValueInlines.h"
 #include "JSCellInlines.h"
+#include "JSFunction.h"
 #include "JSGlobalObject.h"
 #include "JSPromise.h"
 #include "Microtask.h"
@@ -58,6 +60,7 @@ JSPromisePrototype* JSPromisePrototype::create(VM& vm, JSGlobalObject*, Structur
 {
     JSPromisePrototype* object = new (NotNull, allocateCell<JSPromisePrototype>(vm.heap)) JSPromisePrototype(vm, structure);
     object->finishCreation(vm, structure);
+    object->addOwnInternalSlots(vm, structure->globalObject());
     return object;
 }
 
@@ -77,6 +80,12 @@ void JSPromisePrototype::finishCreation(VM& vm, Structure*)
     putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, jsString(&vm, "Promise"), DontEnum | ReadOnly);
 }
 
+void JSPromisePrototype::addOwnInternalSlots(VM& vm, JSGlobalObject* globalObject)
+{
+    JSC_BUILTIN_FUNCTION(vm.propertyNames->builtinNames().thenPrivateName(), promisePrototypeThenCodeGenerator, DontEnum | DontDelete | ReadOnly);
+    JSC_BUILTIN_FUNCTION(vm.propertyNames->builtinNames().catchPrivateName(), promisePrototypeCatchCodeGenerator, DontEnum | DontDelete | ReadOnly);
+}
+
 bool JSPromisePrototype::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
     return getStaticFunctionSlot<Base>(exec, promisePrototypeTable, jsCast<JSPromisePrototype*>(object), propertyName, slot);
index a624192..3128d30 100644 (file)
@@ -45,6 +45,9 @@ public:
 protected:
     void finishCreation(VM&, Structure*);
     JSPromisePrototype(VM&, Structure*);
+
+private:
+    void addOwnInternalSlots(VM&, JSGlobalObject*);
 };
 
 } // namespace JSC
index 3c6fbc8..face0b6 100644 (file)
@@ -1,3 +1,28 @@
+2015-11-09  Xabier Rodriguez Calvar  <calvaris@igalia.com>
+
+        [Streams API] Shield implementation from mangling then and catch promise methods
+        https://bugs.webkit.org/show_bug.cgi?id=150934
+
+        Reviewed by Youenn Fablet.
+
+        This is a first step to get streams code shielded from user replacing the then and catch methods in our
+        promises. We use newly introduced @then and @catch prototype internal slots and that should solve a lot of use
+        cases.
+
+        Test: streams/streams-promises.html.
+
+        * Modules/streams/ReadableStream.js:
+        (initializeReadableStream):
+        * Modules/streams/ReadableStreamInternals.js:
+        (teeReadableStream):
+        (teeReadableStreamPullFunction):
+        (cancelReadableStream):
+        * Modules/streams/WritableStream.js:
+        (initializeWritableStream):
+        (abort):
+        * Modules/streams/WritableStreamInternals.js:
+        (callOrScheduleWritableStreamAdvanceQueue):
+
 2015-11-09  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-grid] Refactor cachedGridCoordinate() to cachedGridSpan()
index 686b356..76b5c67 100644 (file)
@@ -56,7 +56,7 @@ function initializeReadableStream(underlyingSource, strategy)
 
     var result = @invokeOrNoop(underlyingSource, "start", [this.@controller]);
     var _this = this;
-    @Promise.@resolve(result).then(function() {
+    @Promise.@resolve(result).@then(function() {
         _this.@started = true;
         @requestReadableStreamPull(_this);
     }, function(error) {
index 1dbf0e4..55d17fe 100644 (file)
@@ -106,7 +106,7 @@ function teeReadableStream(stream, shouldClone)
     let branch1 = new ReadableStream(underlyingSource1);
     let branch2 = new ReadableStream(underlyingSource2);
 
-    reader.closed.catch(function(e) {
+    reader.closed.@catch(function(e) {
         if (teeState.closedOrErrored)
             return;
         @errorReadableStream(branch1, e);
@@ -124,7 +124,7 @@ function teeReadableStream(stream, shouldClone)
 function teeReadableStreamPullFunction(teeState, reader, shouldClone)
 {
     return function() {
-        reader.read().then(function(result) {
+        reader.read().@then(function(result) {
             if (result.done && !teeState.closedOrErrored) {
                 @closeReadableStream(teeState.branch1);
                 @closeReadableStream(teeState.branch2);
@@ -237,7 +237,7 @@ function requestReadableStreamPull(stream)
     stream.@pulling = true;
 
     var promise = @promiseInvokeOrNoop(stream.@underlyingSource, "pull", [stream.@controller]);
-    promise.then(function() {
+    promise.@then(function() {
         stream.@pulling = false;
         if (stream.@pullAgain) {
             stream.@pullAgain = false;
@@ -280,7 +280,7 @@ function cancelReadableStream(stream, reason)
         return @Promise.@reject(stream.@storedError);
     stream.@queue = @newQueue();
     @finishClosingReadableStream(stream);
-    return @promiseInvokeOrNoop(stream.@underlyingSource, "cancel", [reason]).then(function() { });
+    return @promiseInvokeOrNoop(stream.@underlyingSource, "cancel", [reason]).@then(function() { });
 }
 
 function finishClosingReadableStream(stream)
index ad9d110..e5e400f 100644 (file)
@@ -57,7 +57,7 @@ function initializeWritableStream(underlyingSink, strategy)
     var startResult = @invokeOrNoop(underlyingSink, "start", [error]);
     this.@startedPromise = @Promise.@resolve(startResult);
     var _this = this;
-    this.@startedPromise.then(function() {
+    this.@startedPromise.@then(function() {
         _this.@started = true;
         _this.@startedPromise = undefined;
     }, error);
@@ -82,7 +82,7 @@ function abort(reason)
 
     const sinkAbortPromise = @promiseInvokeOrFallbackOrNoop(this.@underlyingSink, "abort", [reason], "close", []);
 
-    return sinkAbortPromise.then(function() { return undefined; });
+    return sinkAbortPromise.@then(function() { return undefined; });
 }
 
 function close()
index 5fe3732..9858de6 100644 (file)
@@ -77,7 +77,7 @@ function errorWritableStream(e)
 function callOrScheduleWritableStreamAdvanceQueue(stream)
 {
     if (!stream.@started)
-        stream.@startedPromise.then(function() { @writableStreamAdvanceQueue(stream); });
+        stream.@startedPromise.@then(function() { @writableStreamAdvanceQueue(stream); });
     else
         @writableStreamAdvanceQueue(stream);
 
@@ -101,7 +101,7 @@ function writableStreamAdvanceQueue(stream)
     }
 
     stream.@writing = true;
-    @promiseInvokeOrNoop(stream.@underlyingSink, "write", [writeRecord.chunk]).then(
+    @promiseInvokeOrNoop(stream.@underlyingSink, "write", [writeRecord.chunk]).@then(
         function() {
             if (stream.@state === @streamErrored)
                 return;
@@ -123,7 +123,7 @@ function closeWritableStream(stream)
 {
     // FIXME
     // assert(stream.@state === @streamClosing);
-    @promiseInvokeOrNoop(stream.@underlyingSink, "close").then(
+    @promiseInvokeOrNoop(stream.@underlyingSink, "close").@then(
         function() {
             if (stream.@state === @streamErrored)
                 return;