Commit 6f42d0ce authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

[wasm] Handle FetchDataLoader::Client::Abort for streaming compilation

A wpt test showed that for WebAssembly streaming compilation, we did not
handle the abort of a {fetch} correctly. With this CL we throw a
DOMException in that case.

I am not an expert of the FetchDataLoader API, and of the blink GC.
Please take a closer look at these aspects.

R=ricea@chromium.org

Bug: chromium:943487
Change-Id: I3038415265928947233664653a6e64316d240405
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1535877
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: default avatarAdam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644742}
parent f9355806
...@@ -104,7 +104,7 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader, ...@@ -104,7 +104,7 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader,
return AbortCompilation(); return AbortCompilation();
} }
void Trace(blink::Visitor* visitor) override { void Trace(Visitor* visitor) override {
visitor->Trace(consumer_); visitor->Trace(consumer_);
visitor->Trace(client_); visitor->Trace(client_);
visitor->Trace(script_state_); visitor->Trace(script_state_);
...@@ -112,6 +112,25 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader, ...@@ -112,6 +112,25 @@ class FetchDataLoaderForWasmStreaming final : public FetchDataLoader,
BytesConsumer::Client::Trace(visitor); BytesConsumer::Client::Trace(visitor);
} }
void AbortFromClient() {
auto* exception = DOMException::Create(DOMExceptionCode::kAbortError);
ScriptState::Scope scope(script_state_);
// Calling ToV8 in a ScriptForbiddenScope will trigger a CHECK and
// cause a crash. ToV8 just invokes a constructor for wrapper creation,
// which is safe (no author script can be run). Adding AllowUserAgentScript
// directly inside createWrapper could cause a perf impact (calling
// isMainThread() every time a wrapper is created is expensive). Ideally,
// resolveOrReject shouldn't be called inside a ScriptForbiddenScope.
{
ScriptForbiddenScope::AllowUserAgentScript allow_script;
v8::Local<v8::Value> v8_exception =
ToV8(exception, script_state_->GetContext()->Global(),
script_state_->GetIsolate());
streaming_->Abort(v8_exception);
}
}
private: private:
// TODO(ahaas): replace with spec-ed error types, once spec clarifies // TODO(ahaas): replace with spec-ed error types, once spec clarifies
// what they are. // what they are.
...@@ -144,17 +163,19 @@ class WasmDataLoaderClient final ...@@ -144,17 +163,19 @@ class WasmDataLoaderClient final
USING_GARBAGE_COLLECTED_MIXIN(WasmDataLoaderClient); USING_GARBAGE_COLLECTED_MIXIN(WasmDataLoaderClient);
public: public:
WasmDataLoaderClient() = default; explicit WasmDataLoaderClient(FetchDataLoaderForWasmStreaming* loader)
: loader_(loader) {}
void DidFetchDataLoadedCustomFormat() override {} void DidFetchDataLoadedCustomFormat() override {}
void DidFetchDataLoadFailed() override { NOTREACHED(); } void DidFetchDataLoadFailed() override { NOTREACHED(); }
void Abort() override { void Abort() override { loader_->AbortFromClient(); }
// TODO(ricea): This should probably cause the promise owned by
// v8::WasmModuleObjectBuilderStreaming to reject with an AbortError void Trace(Visitor* visitor) override {
// DOMException. As it is, the cancellation will cause it to reject with a visitor->Trace(loader_);
// TypeError later. FetchDataLoader::Client::Trace(visitor);
} }
private: private:
Member<FetchDataLoaderForWasmStreaming> loader_;
DISALLOW_COPY_AND_ASSIGN(WasmDataLoaderClient); DISALLOW_COPY_AND_ASSIGN(WasmDataLoaderClient);
}; };
...@@ -256,7 +277,7 @@ class WasmStreamingClient : public v8::WasmStreaming::Client { ...@@ -256,7 +277,7 @@ class WasmStreamingClient : public v8::WasmStreaming::Client {
return; return;
Platform::Current()->CacheMetadata( Platform::Current()->CacheMetadata(
blink::mojom::CodeCacheType::kWebAssembly, response_url_, mojom::CodeCacheType::kWebAssembly, response_url_,
response_time_, serialized_data.data(), serialized_data.size()); response_time_, serialized_data.data(), serialized_data.size());
} }
...@@ -362,7 +383,8 @@ void StreamFromResponseCallback( ...@@ -362,7 +383,8 @@ void StreamFromResponseCallback(
MakeGarbageCollected<FetchDataLoaderForWasmStreaming>(streaming, MakeGarbageCollected<FetchDataLoaderForWasmStreaming>(streaming,
script_state); script_state);
response->BodyBuffer()->StartLoading( response->BodyBuffer()->StartLoading(
loader, MakeGarbageCollected<WasmDataLoaderClient>(), exception_state); loader, MakeGarbageCollected<WasmDataLoaderClient>(loader),
exception_state);
} }
} // namespace } // namespace
......
...@@ -4174,8 +4174,6 @@ crbug.com/943487 external/wpt/wasm/webapi/rejected-arg.any.serviceworker.html [ ...@@ -4174,8 +4174,6 @@ crbug.com/943487 external/wpt/wasm/webapi/rejected-arg.any.serviceworker.html [
crbug.com/943487 external/wpt/wasm/webapi/rejected-arg.any.worker.html [ Timeout Pass ] crbug.com/943487 external/wpt/wasm/webapi/rejected-arg.any.worker.html [ Timeout Pass ]
crbug.com/943487 external/wpt/wasm/webapi/origin.sub.any.serviceworker.html [ Timeout Pass ] crbug.com/943487 external/wpt/wasm/webapi/origin.sub.any.serviceworker.html [ Timeout Pass ]
crbug.com/943487 external/wpt/wasm/webapi/rejected-arg.any.sharedworker.html [ Timeout Pass ] crbug.com/943487 external/wpt/wasm/webapi/rejected-arg.any.sharedworker.html [ Timeout Pass ]
crbug.com/943487 external/wpt/wasm/webapi/abort.any.worker.html [ Timeout Pass ]
crbug.com/943487 external/wpt/wasm/webapi/abort.any.html [ Timeout Pass ]
crbug.com/792435 external/wpt/css/css-multicol/multicol-rule-004.xht [ Failure ] crbug.com/792435 external/wpt/css/css-multicol/multicol-rule-004.xht [ Failure ]
crbug.com/792437 external/wpt/css/css-multicol/multicol-rule-inset-000.xht [ Failure ] crbug.com/792437 external/wpt/css/css-multicol/multicol-rule-inset-000.xht [ Failure ]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment