[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..a1290315098a1d16742481a132c73d37653e0b01 100644
--- a/third_party/WebKit/Source/modules/fetch/Response.cpp
+++ b/third_party/WebKit/Source/modules/fetch/Response.cpp
@@ -1,3 +1,4 @@
+

[yhirano, 2017/04/05 03:48:03]

-

[Mircea Trofin, 2017/04/05 05:12:21]

Acknowledged.

 // 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,130 @@ 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.
+// See https://crbug.com/708238 for tracking avoiding the hand-generated code.
+void compileFromResponseCallback(
+    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());
+}
+
+// See https://crbug.com/708238 for tracking avoiding the hand-generated code.
+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);

[yhirano, 2017/04/05 03:48:03]

I think this should work as 
  wasmCompileOverLoad(Promise.resolve(args[0]))

instead of 
  compileFromResponseCallback(args)

.

[Mircea Trofin, 2017/04/05 05:12:22]

So I understand - what would be the motivation - ensuring one path into
compileFromResponseCallback? (I wanted to avoid the extra prep work, that would
be my motivation for the current state of things).

[yhirano, 2017/04/05 12:02:50]

The current implementation calls compileFromResponseCallback synchronously and
my suggestion doesn't. The latter is correct based on
https://www.w3.org/2001/tag/doc/promises-guide#resolve-arguments.

[Mircea Trofin, 2017/04/05 17:17:49]

Thanks for the clarification and the pointer. I made the change

Still not sure though it buys us much: compileFromResponseCallback just starts
the asynchronous fetch and returns a promise; and we still need to do
typechecking (as the CL demonstrates), to determine if the API needs to call
back to v8's original behavior: if the parameter passed to the blink-side
overload isn't a promise nor a Response, we still need to do the v8 behavior.

Am I missing something?

[yhirano, 2017/04/07 02:39:36]

Ah, I see, thank you. Returning false means redirecting to the original
function. I have another question about the interface, but will ask on anther
thread.

[yhirano, 2017/04/07 02:52:29]

Sorry, I was wrong. Please replace the last response with below.


Ah, I see, thank you. Returning false means redirecting to the original
function. We need the type checking code.

But I still think compile(response) should behave exactly same as
compile(Promise.resolve(response)), and that should be clear without closer look
at compileFromResponseCallback. In the current implmentation, there is a
microtask timing difference, right?

[Mircea Trofin, 2017/04/07 06:11:16]

Ah! I think I see what you mean. The example at that link, basically, says it
all. I think I got it now.

+    return true;
+  }
+  return false;
+}
+
 }  // namespace
 
+v8::Local<v8::FunctionTemplate> Response::installerOverride(
+    v8::Isolate* isolate,
+    const DOMWrapperWorld& world) {
+  isolate->SetWasmCompileCallback(wasmCompileOverload);
+  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);