[wasm] switch response API implementation to async compile |
||
Issue descriptionThis gets us closer to the API shape we'd eventually need for streaming compilation, because both are promise-based.
,
Jul 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2f3d31477fc9b4e0f41242e7a63f0be4ee55d310 commit 2f3d31477fc9b4e0f41242e7a63f0be4ee55d310 Author: Mircea Trofin <mtrofin@chromium.org> Date: Thu Jul 27 23:36:07 2017 [wasm] Response APIs: use promise-based module building API The first implementation of the response-based APIs used sync compile as an implementation detail. As we're getting close to having a true streaming implementation, we're switching to a promise-based API, closer in shape and implementation to the final design. What differs from true streaming is that the implementation, just like before this change, still pre buffers data before starting compilation. The difference in this CL is that compilation is asynchronous, and is responsible for resolving/rejecting the promise. The case of network error is where we expect differences in the final implementation. Currently, no compilation is started, and the blink side rejects the promise. When we move to true streaming, we'll need to explicitly signal to the v8 side when the download is dropped, and probably let it (v8 side) reject the promise. Also added a regression test for v8:6619, which is implicitly fixed by the new design. Bug: chromium:747396 Bug: v8:6619 Change-Id: Ic47c948a7cd001e07926ee4d0ae70669bab680bb Reviewed-on: https://chromium-review.googlesource.com/580788 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Mircea Trofin <mtrofin@chromium.org> Cr-Commit-Position: refs/heads/master@{#490111} [add] https://crrev.com/2f3d31477fc9b4e0f41242e7a63f0be4ee55d310/third_party/WebKit/LayoutTests/http/tests/wasm/resources/invalid-wasm.wasm [modify] https://crrev.com/2f3d31477fc9b4e0f41242e7a63f0be4ee55d310/third_party/WebKit/LayoutTests/http/tests/wasm/resources/load-wasm.php [modify] https://crrev.com/2f3d31477fc9b4e0f41242e7a63f0be4ee55d310/third_party/WebKit/LayoutTests/http/tests/wasm_streaming/wasm_response_apis.html [modify] https://crrev.com/2f3d31477fc9b4e0f41242e7a63f0be4ee55d310/third_party/WebKit/LayoutTests/http/tests/wasm_streaming/wasm_response_apis.js [modify] https://crrev.com/2f3d31477fc9b4e0f41242e7a63f0be4ee55d310/third_party/WebKit/Source/bindings/modules/v8/wasm/WasmResponseExtensions.cpp
,
Jul 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0278717e4820176809cdc1fedd1c336167aba442 commit 0278717e4820176809cdc1fedd1c336167aba442 Author: meade_UTC10 <meade@chromium.org> Date: Fri Jul 28 01:11:06 2017 Revert "[wasm] Response APIs: use promise-based module building API" This reverts commit 2f3d31477fc9b4e0f41242e7a63f0be4ee55d310. Reason for revert: Suspected cause of failures on WebKit Linux Trusty Leak bot : Unexpected Failures: * http/tests/wasm_streaming/wasm_response_apis.html * virtual/enable_wasm_streaming/http/tests/wasm_streaming/wasm_response_apis.html * virtual/mojo-loading/http/tests/wasm_streaming/wasm_response_apis.html First failure: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/7364 Original change's description: > [wasm] Response APIs: use promise-based module building API > > The first implementation of the response-based APIs used > sync compile as an implementation detail. As we're getting close > to having a true streaming implementation, we're switching to > a promise-based API, closer in shape and implementation to the > final design. > > What differs from true streaming is that the implementation, just > like before this change, still pre buffers data before starting > compilation. The difference in this CL is that compilation is > asynchronous, and is responsible for resolving/rejecting the promise. > > The case of network error is where we expect differences in the final > implementation. Currently, no compilation is started, and the > blink side rejects the promise. When we move to true streaming, > we'll need to explicitly signal to the v8 side when the download is > dropped, and probably let it (v8 side) reject the promise. > > Also added a regression test for v8:6619, which is implicitly fixed by > the new design. > > > Bug: chromium:747396 > Bug: v8:6619 > Change-Id: Ic47c948a7cd001e07926ee4d0ae70669bab680bb > Reviewed-on: https://chromium-review.googlesource.com/580788 > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Commit-Queue: Mircea Trofin <mtrofin@chromium.org> > Cr-Commit-Position: refs/heads/master@{#490111} TBR=bradnelson@chromium.org,haraken@chromium.org,mtrofin@chromium.org,ahaas@chromium.org Change-Id: I6a7fdb9b3d49d2fb26bc070f99ba6d4247f0f04f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:747396 , v8:6619 Reviewed-on: https://chromium-review.googlesource.com/590389 Commit-Queue: meade_UTC10 <meade@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Cr-Commit-Position: refs/heads/master@{#490175} [delete] https://crrev.com/1d2472111e77064f80cbc2fd15f76b6746d9bab3/third_party/WebKit/LayoutTests/http/tests/wasm/resources/invalid-wasm.wasm [modify] https://crrev.com/0278717e4820176809cdc1fedd1c336167aba442/third_party/WebKit/LayoutTests/http/tests/wasm/resources/load-wasm.php [modify] https://crrev.com/0278717e4820176809cdc1fedd1c336167aba442/third_party/WebKit/LayoutTests/http/tests/wasm_streaming/wasm_response_apis.html [modify] https://crrev.com/0278717e4820176809cdc1fedd1c336167aba442/third_party/WebKit/LayoutTests/http/tests/wasm_streaming/wasm_response_apis.js [modify] https://crrev.com/0278717e4820176809cdc1fedd1c336167aba442/third_party/WebKit/Source/bindings/modules/v8/wasm/WasmResponseExtensions.cpp
,
Jul 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d36bea9f66b9bcb2543f4f541c929da1c49b99cc commit d36bea9f66b9bcb2543f4f541c929da1c49b99cc Author: Mircea Trofin <mtrofin@chromium.org> Date: Sat Jul 29 16:50:06 2017 Revert "Revert "[wasm] Response APIs: use promise-based module building API"" This reverts commit 0278717e4820176809cdc1fedd1c336167aba442. Reason for revert: Fixed leak by moving ownership of promise (when streaming has started) on the V8 side. Blink is still responsible, and owns, promises in setup error cases (like invalid MIME). Original change's description: > Revert "[wasm] Response APIs: use promise-based module building API" > > This reverts commit 2f3d31477fc9b4e0f41242e7a63f0be4ee55d310. > > Reason for revert: Suspected cause of failures on WebKit Linux Trusty Leak bot : > Unexpected Failures: > * http/tests/wasm_streaming/wasm_response_apis.html > * virtual/enable_wasm_streaming/http/tests/wasm_streaming/wasm_response_apis.html > * virtual/mojo-loading/http/tests/wasm_streaming/wasm_response_apis.html > > First failure: > https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/7364 > > Original change's description: > > [wasm] Response APIs: use promise-based module building API > > > > The first implementation of the response-based APIs used > > sync compile as an implementation detail. As we're getting close > > to having a true streaming implementation, we're switching to > > a promise-based API, closer in shape and implementation to the > > final design. > > > > What differs from true streaming is that the implementation, just > > like before this change, still pre buffers data before starting > > compilation. The difference in this CL is that compilation is > > asynchronous, and is responsible for resolving/rejecting the promise. > > > > The case of network error is where we expect differences in the final > > implementation. Currently, no compilation is started, and the > > blink side rejects the promise. When we move to true streaming, > > we'll need to explicitly signal to the v8 side when the download is > > dropped, and probably let it (v8 side) reject the promise. > > > > Also added a regression test for v8:6619, which is implicitly fixed by > > the new design. > > > > > > Bug: chromium:747396 > > Bug: v8:6619 > > Change-Id: Ic47c948a7cd001e07926ee4d0ae70669bab680bb > > Reviewed-on: https://chromium-review.googlesource.com/580788 > > Reviewed-by: Kentaro Hara <haraken@chromium.org> > > Commit-Queue: Mircea Trofin <mtrofin@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#490111} > > TBR=bradnelson@chromium.org,haraken@chromium.org,mtrofin@chromium.org,ahaas@chromium.org > > Change-Id: I6a7fdb9b3d49d2fb26bc070f99ba6d4247f0f04f > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: chromium:747396 , v8:6619 > Reviewed-on: https://chromium-review.googlesource.com/590389 > Commit-Queue: meade_UTC10 <meade@chromium.org> > Reviewed-by: meade_UTC10 <meade@chromium.org> > Cr-Commit-Position: refs/heads/master@{#490175} TBR=bradnelson@chromium.org,haraken@chromium.org,mtrofin@chromium.org,ahaas@chromium.org,meade@chromium.org Change-Id: I45d883fabf294efdcab87e5eb18ea7879a054aa9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:747396 , v8:6619 Reviewed-on: https://chromium-review.googlesource.com/591053 Commit-Queue: Mircea Trofin <mtrofin@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#490663} [add] https://crrev.com/d36bea9f66b9bcb2543f4f541c929da1c49b99cc/third_party/WebKit/LayoutTests/http/tests/wasm/resources/invalid-wasm.wasm [modify] https://crrev.com/d36bea9f66b9bcb2543f4f541c929da1c49b99cc/third_party/WebKit/LayoutTests/http/tests/wasm/resources/load-wasm.php [modify] https://crrev.com/d36bea9f66b9bcb2543f4f541c929da1c49b99cc/third_party/WebKit/LayoutTests/http/tests/wasm_streaming/wasm_response_apis.html [modify] https://crrev.com/d36bea9f66b9bcb2543f4f541c929da1c49b99cc/third_party/WebKit/LayoutTests/http/tests/wasm_streaming/wasm_response_apis.js [modify] https://crrev.com/d36bea9f66b9bcb2543f4f541c929da1c49b99cc/third_party/WebKit/Source/bindings/modules/v8/wasm/WasmResponseExtensions.cpp
,
Aug 7 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Jul 21 2017