Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(63)

Issue 2762163002: Revert of [wasm] Transferrable modules (Closed)

Created:
3 years, 9 months ago by Michael Achenbach
Modified:
3 years, 9 months ago
CC:
esprehn, Michael Hablich, Paweł Hajdan Jr., v8-merges_googlegroups.com, v8-reviews_googlegroups.com
Target Ref:
refs/heads/master
Project:
v8
Visibility:
Public.

Description

Revert of [wasm] Transferrable modules (patchset #13 id:280001 of https://codereview.chromium.org/2748473004/ ) Reason for revert: Breaks layout tests: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/14312 See https://github.com/v8/v8/wiki/Blink-layout-tests Original issue's description: > [wasm] Transferrable modules > > We want to restrict structured cloning in Chrome to: > - postMessage senders and receivers that are co-located > in the same process > - indexedDB (just https). > > For context, on the Chrome side, we will achieve the postMessage part > by using a mechanism similar to transferrables: the > SerializedScriptValue will have a list of wasm modules, separate from > the serialized data stream; and this list won't be copied cross > process boundaries. The IDB part is achieved by explicitly opting in > reading/writing to the serialization stream. To block attack vectors > in IPC cases, the default for deserialization will be to expect data > in the wasm transfers list. > > This change is the V8 side necessary to enabling this design. We > introduce TransferrableModule, an opaque datatype exposed to the > embedder. Internally, TransferrableModules are just serialized data, > because we don't have a better mechanism, at the moment, for > de-contextualizing/re-contextualizing wasm modules (wrt Isolate and > Context). > > The chrome defaults will be implemented in the > serialization/deserialization delegates on that side. For the v8 side > of things, in the absence of a serialization delegate, the V8 > serializer will write to serialization stream. In the absence of a > deserialization delegate, the deserializer won't work. This asymmetry > is intentional - it communicates to the embedder the need to make a > policy decision, otherwise wasm serialization/deserialization won't > work "out of the box". > > BUG=v8:6079 > > Review-Url: https://codereview.chromium.org/2748473004 > Cr-Commit-Position: refs/heads/master@{#43955} > Committed: https://chromium.googlesource.com/v8/v8/+/99743ad460ea5b9795ba9d70a074e75d7362a3d1 TBR=jbroman@chromium.org,bradnelson@chromium.org,mtrofin@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:6079 Review-Url: https://codereview.chromium.org/2762163002 Cr-Commit-Position: refs/heads/master@{#43981} Committed: https://chromium.googlesource.com/v8/v8/+/e538b70e1a45289dfe0fa9789563f023a5e9c22b

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -478 lines) Patch
M include/v8.h View 6 chunks +0 lines, -51 lines 0 comments Download
M src/api.cc View 4 chunks +0 lines, -48 lines 0 comments Download
M src/value-serializer.h View 5 chunks +0 lines, -7 lines 0 comments Download
M src/value-serializer.cc View 5 chunks +7 lines, -49 lines 0 comments Download
M test/cctest/wasm/test-run-wasm-module.cc View 1 chunk +0 lines, -37 lines 0 comments Download
M test/unittests/value-serializer-unittest.cc View 7 chunks +31 lines, -286 lines 0 comments Download

Messages

Total messages: 9 (4 generated)
Michael Achenbach
Created Revert of [wasm] Transferrable modules
3 years, 9 months ago (2017-03-21 13:54:00 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2762163002/1
3 years, 9 months ago (2017-03-21 13:54:04 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/v8/v8/+/e538b70e1a45289dfe0fa9789563f023a5e9c22b
3 years, 9 months ago (2017-03-21 13:54:30 UTC) #7
Michael Achenbach
CC hablich This also blocked rolling
3 years, 9 months ago (2017-03-21 13:54:31 UTC) #8
Mircea Trofin
3 years, 9 months ago (2017-03-21 17:47:46 UTC) #9
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2762273002/ by mtrofin@chromium.org.

The reason for reverting is: Temporarily disabled tests on chromium side
(https://codereview.chromium.org/2764933002).

Powered by Google App Engine
This is Rietveld 408576698