[wasm] response-based compile APIs

third_party/WebKit/Source/modules/fetch/Response.cpp

Index: third_party/WebKit/Source/modules/fetch/Response.cpp
diff --git a/third_party/WebKit/Source/modules/fetch/Response.cpp b/third_party/WebKit/Source/modules/fetch/Response.cpp
index d5319f453f05869de885768408521d50bf6970f7..240b2864e9e332af30f08245b8ebab636245d9d7 100644
--- a/third_party/WebKit/Source/modules/fetch/Response.cpp
+++ b/third_party/WebKit/Source/modules/fetch/Response.cpp
@@ -1,3 +1,4 @@
+
 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
@@ -16,6 +17,7 @@
 #include "bindings/core/v8/V8HiddenValue.h"
 #include "bindings/core/v8/V8URLSearchParams.h"
 #include "bindings/modules/v8/ByteStringSequenceSequenceOrDictionaryOrHeaders.h"
+#include "bindings/modules/v8/V8Response.h"
 #include "core/dom/DOMArrayBuffer.h"
 #include "core/dom/DOMArrayBufferView.h"
 #include "core/dom/URLSearchParams.h"
@@ -120,8 +122,128 @@ bool isValidReasonPhrase(const String& statusText) {
   return true;
 }
 
+// TODO(mtrofin): WasmDataLoaderClient is necessary so we may provide an
+// argument to BodyStreamBuffer::startLoading, however, it fulfills
+// a very small role. Consider refactoring to avoid it.
+class WasmDataLoaderClient final
+    : public GarbageCollectedFinalized<WasmDataLoaderClient>,
+      public FetchDataLoader::Client {
+  WTF_MAKE_NONCOPYABLE(WasmDataLoaderClient);
+  USING_GARBAGE_COLLECTED_MIXIN(WasmDataLoaderClient);
+
+ public:
+  explicit WasmDataLoaderClient(ScriptPromiseResolver* resolver)
+      : m_resolver(resolver) {}
+
+  void didFetchDataLoadedStream() override {}
+  void didFetchDataLoadFailed() override {
+    ScriptState::Scope scope(m_resolver->getScriptState());
+    m_resolver->reject(V8ThrowException::createTypeError(
+        m_resolver->getScriptState()->isolate(), "Failed to fetch"));
+  }
+  DEFINE_INLINE_TRACE() {
+    visitor->trace(m_resolver);
+    FetchDataLoader::Client::trace(visitor);
+  }
+
+ private:
+  Member<ScriptPromiseResolver> m_resolver;
+};
+
+// This callback may be entered as a promise is resolved, or directly
+// from the overload callback.
+void compileFromResponseCallback(

[haraken, 2017/04/04 06:05:31]

I'd prefer moving these wasm-specific methods to
Source/bindings/core/v8/WASMHelper.cpp or something.

[Mircea Trofin, 2017/04/04 06:48:07]

Won't layering cause an issue? These implementations take a dependency on
Response APIs.

+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  ExceptionState exceptionState(args.GetIsolate(),
+                                ExceptionState::ExecutionContext, "WebAssembly",
+                                "compile");
+  ExceptionToRejectPromiseScope rejectPromiseScope(args, exceptionState);
+
+  ScriptState* scriptState = ScriptState::forReceiverObject(args);
+  if (!scriptState->getExecutionContext()) {
+    args.GetReturnValue().Set(ScriptPromise().v8Value());
+    return;
+  }
+
+  if (args.Length() < 1 || !args[0]->IsObject() ||
+      !V8Response::hasInstance(args[0], args.GetIsolate())) {
+    args.GetReturnValue().Set(
+        ScriptPromise::reject(
+            scriptState, V8ThrowException::createTypeError(
+                             scriptState->isolate(),
+                             "Promise argument must produce a Response object"))
+            .v8Value());
+    return;
+  }
+
+  Response* response = V8Response::toImpl(v8::Local<v8::Object>::Cast(args[0]));
+  ScriptPromise promise;
+  if (response->isBodyLocked() || response->bodyUsed()) {
+    promise = ScriptPromise::reject(
+        scriptState,
+        V8ThrowException::createTypeError(
+            scriptState->isolate(),
+            "Cannot compile WebAssembly.Module from an already read Response"));
+  } else {
+    ScriptPromiseResolver* resolver =
+        ScriptPromiseResolver::create(scriptState);
+    if (response->bodyBuffer()) {
+      promise = resolver->promise();
+      response->bodyBuffer()->startLoading(
+          FetchDataLoader::createLoaderAsWasmModule(args.GetIsolate(), resolver,
+                                                    scriptState),
+          new WasmDataLoaderClient(resolver));
+    } else {
+      promise = ScriptPromise::reject(
+          scriptState, V8ThrowException::createTypeError(
+                           scriptState->isolate(),
+                           "Promise argument must produce a Response object"));
+    }
+  }
+  args.GetReturnValue().Set(promise.v8Value());
+}
+
+bool wasmCompileOverload(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  if (args.Length() < 1 || !args[0]->IsObject())
+    return false;
+
+  if (args[0]->IsPromise()) {
+    ScriptState* scriptState = ScriptState::forReceiverObject(args);
+    ScriptPromise sp = ScriptPromise(scriptState, args[0]);
+    ScriptPromise thenCompile = sp.then(
+        v8::Function::New(args.GetIsolate(), compileFromResponseCallback));
+    args.GetReturnValue().Set(thenCompile.v8Value());
+    return true;
+  }
+  if (V8Response::hasInstance(args[0], args.GetIsolate())) {
+    compileFromResponseCallback(args);
+    return true;
+  }
+  return false;
+}
+
 }  // namespace
 
+v8::Local<v8::FunctionTemplate> Response::installerOverride(
+    v8::Isolate* isolate,
+    const DOMWrapperWorld& world) {
+  isolate->SetWasmCompileCallback(wasmCompileOverload);

[haraken, 2017/04/04 06:05:31]

Just to confirm: This statement is the only reason you need to override the
wrapper type info, right?

Also would Response the only object that will need the statement? Or do you have
a plan to use it in other objects as well? If yes, we should consider
introducing an IDL extended attribute like [SupportWebAssembly] and let the IDL
compiler generate the statement.

That said, I'm confused. Why do we not have WebAssembly.idl in the first place?
I'm feeling that this CL is trying to implement WebAssembly.compile without
explicitly exposing it to the IDL. That's why you need to implement it yourself
without relying on the IDL comipler. Which seems strange to me.

[Mircea Trofin, 2017/04/04 06:48:07]

Yes. It'll be followed by a similar statement for the WebAssembly.instantiate
API, but that's all.
We don't have more plans, but (as we were chatting on the offline thread), the
scenario limiting synchronous wasm compilation to certain sizes also "feels"
like overriding, so would benefit from a similar mechanism. Currently, it's done
the same way (injection), just that there was a natural, layering-preserving
location for that initialization (V8Initializer).

I'd feel better with a streamlined solution here, too, and if that's enhancing
the IDL, sgtm. For completeness -
 we also had on the table the idea of using "V8 Extras", but that has its own
challenges. 

To conclude - happy to do the IDL (or "V8 Extras"?) work in a subsequent CL! 

[haraken, 2017/04/04 07:05:15]

Yeah, I was confused about this CL because it's introducing a new programming
pattern to the V8 - Blink bindings :) I'd prefer sticking to the
well-established IDL pattern. I'm fine with introducing a new IDL extended
attribute like [WebAssembly] if necessary.

+  return Response::s_wrapperTypeInfo.domTemplateFunction(isolate, world);
+}
+
+const WrapperTypeInfo Response::s_wrapperTypeInfoOverride = {
+    gin::kEmbedderBlink,
+    Response::installerOverride,
+    V8Response::trace,
+    V8Response::traceWrappers,
+    nullptr,
+    "Response",
+    0,
+    WrapperTypeInfo::WrapperTypeObjectPrototype,
+    WrapperTypeInfo::ObjectClassId,
+    WrapperTypeInfo::InheritFromActiveScriptWrappable,
+    WrapperTypeInfo::Dependent};
+
 Response* Response::create(ScriptState* scriptState,
                            ExceptionState& exceptionState) {
   return create(scriptState, nullptr, String(), ResponseInit(), exceptionState);