[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..efbe521c65ae6a839c5bbce472b8ab91a2f4b955 100644
--- a/third_party/WebKit/Source/modules/fetch/Response.cpp
+++ b/third_party/WebKit/Source/modules/fetch/Response.cpp
@@ -16,6 +16,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 +121,220 @@ bool isValidReasonPhrase(const String& statusText) {
   return true;
 }
 
+class FetchDataLoaderAsWasm final : public FetchDataLoader,

[yhirano, 2017/04/04 00:44:35]

Please define this (and related classes) in FetchDataLoader.cpp.

[Mircea Trofin, 2017/04/04 04:08:12]

Done. 

Except that we need the WasmDataLoaderClient (was: WasmConsumer) here, so moved
just FetchDataLoaderAsWasmModule (was: FetchDataLoaderAsWasm), and added the
corresponding create*** API.

Alternatively, WasmDataLoaderClient could have its own .h file, but it's
ultimately and implementation detail of this pipeline here, and adding a
separate file seems a bit overkill.

+                                    public BytesConsumer::Client {
+  USING_GARBAGE_COLLECTED_MIXIN(FetchDataLoaderAsWasm);
+
+ public:
+  explicit FetchDataLoaderAsWasm(v8::Isolate* isolate,
+                                 ScriptPromiseResolver* resolver,
+                                 ScriptState* scriptState)
+      : m_resolver(resolver), m_builder(isolate), m_scriptState(scriptState) {}
+
+  void start(BytesConsumer* consumer,
+             FetchDataLoader::Client* client) override {
+    DCHECK(!m_consumer);
+    DCHECK(!m_client);
+    m_client = client;
+    m_consumer = consumer;
+    m_consumer->setClient(this);
+    onStateChange();
+  }
+
+  void onStateChange() override {
+    while (true) {
+      const char* buffer;
+      size_t available;
+      BytesConsumer::Result result = m_consumer->beginRead(&buffer, &available);
+
+      switch (result) {
+        case BytesConsumer::Result::ShouldWait:
+          return;
+        case BytesConsumer::Result::Ok: {

[yhirano, 2017/04/04 00:44:35]

Why do we need this bracket?

[Mircea Trofin, 2017/04/04 04:08:12]

I prefer adding braces for more than 1 line + the break. 

+          SendBytes(buffer, available);
+          result = m_consumer->endRead(available);

[yhirano, 2017/04/04 00:44:35]

What happens when this returns an error?

[Mircea Trofin, 2017/04/04 04:08:12]

Good catch - we can reject at that point. Done.

+          break;
+        }
+        case BytesConsumer::Result::Done: {
+          if (available > 0)
+            SendBytes(buffer, available);
+          v8::Isolate* isolate = m_resolver->getScriptState()->isolate();
+          ScriptState::Scope scope(m_scriptState.get());
+          v8::TryCatch trycatch(isolate);
+          v8::MaybeLocal<v8::WasmCompiledModule> maybeModule =
+              m_builder.Finish();
+          v8::Local<v8::WasmCompiledModule> module;
+          if (maybeModule.ToLocal(&module)) {
+            DCHECK(!trycatch.HasCaught());
+            ScriptValue sv(m_scriptState.get(), module);
+            m_resolver->resolve(sv);
+          } else {
+            DCHECK(trycatch.HasCaught());
+            m_resolver->reject(trycatch.Exception());
+            trycatch.Reset();
+          }
+          m_client->didFetchDataLoadedStream();

[yhirano, 2017/04/04 00:44:35]

This function is not for loading a WASM. It's for streams, so please don't call
it here. The right way is to define a new function in FetchDataLoader::Client.

[Mircea Trofin, 2017/04/04 04:08:12]

Done.

+          return;
+        }
+        case BytesConsumer::Result::Error: {
+          // TODO(mtrofin): do we need an abort on the wasm side?
+          // m_outStream->abort();
+          m_client->didFetchDataLoadFailed();
+          return;
+        }
+      }
+    }
+  }
+
+  void cancel() override { m_consumer->cancel(); }
+
+  DEFINE_INLINE_TRACE() {
+    visitor->trace(m_consumer);
+    visitor->trace(m_resolver);
+    visitor->trace(m_client);
+    FetchDataLoader::trace(visitor);
+    BytesConsumer::Client::trace(visitor);
+  }
+
+ private:
+  void SendBytes(const char* bytes, size_t size) {
+    char* bufferCopy = new char[size];
+    memcpy(bufferCopy, bytes, size);
+    // TODO(mtrofin): we want to extend OnBytesReceived to test
+    // for decoding errors or for compilation errors that
+    // happened meanwhile.
+    m_builder.OnBytesReceived(std::unique_ptr<const uint8_t[]>(
+                                  reinterpret_cast<const uint8_t*>(bufferCopy)),

[yhirano, 2017/04/04 00:44:35]

Is this safe? I think this is confusing; can you remove this reinterpret_cast
and declare a unique pointer from the beginning?

[Mircea Trofin, 2017/04/04 04:08:12]

It is safe because bufferCopy doesn't get aliased out of this scope, but you're
right, it's just cleaner to start and end with std::unique_ptr. Done.

+                              size);
+  }
+
+  Member<BytesConsumer> m_consumer;
+  Member<ScriptPromiseResolver> m_resolver;
+  Member<FetchDataLoader::Client> m_client;
+  v8::WasmModuleObjectBuilder m_builder;
+  const RefPtr<ScriptState> m_scriptState;
+};
+
+// TODO(mtrofin): WasmConsumer is necessary so we may provide an
+// argument to BodyStreamBuffer::startLoading, however, it fulfills
+// a very small role. Consider refactoring to avoid it.
+class WasmConsumer final : public GarbageCollectedFinalized<WasmConsumer>,
+                           public FetchDataLoader::Client {
+  WTF_MAKE_NONCOPYABLE(WasmConsumer);
+  USING_GARBAGE_COLLECTED_MIXIN(WasmConsumer);
+
+ public:
+  explicit WasmConsumer(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(

[yhirano, 2017/04/04 00:44:35]

Please use camelCase instead of CamelCase.

[Mircea Trofin, 2017/04/04 04:08:12]

Done.

+    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(
+          new FetchDataLoaderAsWasm(args.GetIsolate(), resolver, scriptState),
+          new WasmConsumer(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) {

[yhirano, 2017/04/04 00:44:35]

ditto

[Mircea Trofin, 2017/04/04 04:08:12]

Done.

+  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);
+  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);