[wasm] response-based compile APIs

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

Index: third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp
diff --git a/third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp b/third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp
index e5b6ce557785ad9816bf866ffd45be9bc671477d..06413936f16919f388151d22050864cf399a0bfe 100644
--- a/third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp
+++ b/third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp
@@ -21,8 +21,7 @@ class FetchDataLoaderAsBlobHandle final : public FetchDataLoader,
   USING_GARBAGE_COLLECTED_MIXIN(FetchDataLoaderAsBlobHandle);
 
  public:
-  explicit FetchDataLoaderAsBlobHandle(const String& mimeType)
-      : m_mimeType(mimeType) {}
+  FetchDataLoaderAsBlobHandle(const String& mimeType) : m_mimeType(mimeType) {}

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

Why did you change this line?

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

Hmm... weird. I think I misread a previous comment. Changing back.

 
   void start(BytesConsumer* consumer,
              FetchDataLoader::Client* client) override {
@@ -305,6 +304,96 @@ class FetchDataLoaderAsStream final : public FetchDataLoader,
   Member<Stream> m_outStream;
 };
 
+class FetchDataLoaderAsWasmModule final : public FetchDataLoader,

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

This FechDataLoader calls m_resolver->reject with an exception sometimes,
m_resolver->reject with undefined sometimes and m_client->didFetchDataLoadFailed
in other cases. Error handling should be consistent, so I think it's a good idea
to state that this FetchDataLoader doesn't call didFetchDataLoadFailed but uses
the given resolver to notify errors.

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

Makes sense. Also noticed the wasm spec doesn't seem to cover faulty
channel/cancellation. Throwing TypeError for now (consistent to passing an
unsupported parameter). Will follow up with a subsequent CL when the spec
clarifies.

+                                          public BytesConsumer::Client {
+  USING_GARBAGE_COLLECTED_MIXIN(FetchDataLoaderAsWasmModule);
+
+ public:
+  FetchDataLoaderAsWasmModule(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) {
+      // buffer is allocated by beginRead and de-allocated by endRead.
+      const char* buffer = nullptr;
+      size_t available = 0;
+      BytesConsumer::Result result = m_consumer->beginRead(&buffer, &available);
+
+      switch (result) {
+        case BytesConsumer::Result::ShouldWait:
+          DCHECK_EQ(buffer, nullptr);
+          DCHECK_EQ(available, 0U);
+          return;
+        case BytesConsumer::Result::Ok: {
+          DCHECK_NE(buffer, nullptr);
+          DCHECK_GT(available, 0U);

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

|available| can be zero. See comments in BytesConsumer.h.

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

Hmm... I'm looking at these 2 lines:

// Returns Ok when readable.
[...]
// |*buffer| will be set to null and |*available| will be set to 0 if not
// readable.

I assumed that meant the only state buffer and available are non-null/zero is
Result::OK. I realize that's not strictly implied. Perhaps I can add a
clarifying comment to avoid confusion?

Changed the code here, meanwhile, to match the rest of this file.

+          m_builder.OnBytesReceived(reinterpret_cast<const uint8_t*>(buffer),
+                                    available);
+          result = m_consumer->endRead(available);

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

|result| can be |Done|. See comments in BytesConsumer.h.

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

Acknowledged.

+          if (result == BytesConsumer::Result::Error) {
+            m_resolver->reject();
+          }
+          break;
+        }
+        case BytesConsumer::Result::Done: {
+          DCHECK_EQ(buffer, nullptr);
+          DCHECK_EQ(available, 0U);
+          v8::Isolate* isolate = m_resolver->getScriptState()->isolate();
+          ScriptState::Scope scope(m_scriptState.get());
+          v8::TryCatch trycatch(isolate);
+          v8::Local<v8::WasmCompiledModule> module;
+          if (m_builder.Finish().ToLocal(&module)) {
+            DCHECK(!trycatch.HasCaught());
+            ScriptValue scriptValue(m_scriptState.get(), module);
+            m_resolver->resolve(scriptValue);
+          } else {
+            DCHECK(trycatch.HasCaught());
+            m_resolver->reject(trycatch.Exception());
+            trycatch.Reset();
+          }
+          m_client->didFetchDataLoadedStream();
+          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(); }

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

I think you need to do something with m_resolver.

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

Acknowledged.

+
+  DEFINE_INLINE_TRACE() {
+    visitor->trace(m_consumer);
+    visitor->trace(m_resolver);
+    visitor->trace(m_client);
+    FetchDataLoader::trace(visitor);
+    BytesConsumer::Client::trace(visitor);
+  }
+
+ private:
+  Member<BytesConsumer> m_consumer;
+  Member<ScriptPromiseResolver> m_resolver;
+  Member<FetchDataLoader::Client> m_client;
+  v8::WasmModuleObjectBuilder m_builder;
+  const RefPtr<ScriptState> m_scriptState;
+};
+
 }  // namespace
 
 FetchDataLoader* FetchDataLoader::createLoaderAsBlobHandle(
@@ -324,4 +413,11 @@ FetchDataLoader* FetchDataLoader::createLoaderAsStream(Stream* outStream) {
   return new FetchDataLoaderAsStream(outStream);
 }
 
+FetchDataLoader* FetchDataLoader::createLoaderAsWasmModule(
+    v8::Isolate* isolate,
+    ScriptPromiseResolver* resolver,
+    ScriptState* scriptState) {
+  return new FetchDataLoaderAsWasmModule(isolate, resolver, scriptState);
+}
+
 }  // namespace blink