Bug 155111 – [Fetch API] Implement fetch skeleton

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

We can start implementing fetch API for window and worker by enabling a skeleton for it and rejecting the promise.

[youenn fablet]

Created attachment 273181 [details]
Patch

[WebKit Commit Bot]

Attachment 273181 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/fetch/DOMWindowFetch.h:46:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 72 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 273181 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=273181&action=review

I’m not sure this is better as a module. I know the theory is that it makes the code better to not have everything in DOMWindow.idl but I am not sure it’s great in practice.

On the questions about using references, I assume there is some reason, but I want to know what it is.

All the same comments apply to the worker global scope version that apply to the DOMWindow version.

> Source/WebCore/Modules/fetch/DOMWindowFetch.h:46
> +    explicit DOMWindowFetch(DOMWindow* window) : DOMWindowProperty(window->frame()) { }

This always dereferences the DOMWindow, so what prevents us from making this argument a reference rather than a pointer?

> Source/WebCore/Modules/fetch/DOMWindowFetch.h:47
> +    static DOMWindowFetch* from(DOMWindow*);

This always returns non-null and always requires a non-null argument. What prevents us from writing this?

    static DOMWindowFetch& from(DOMWindow&);

> Source/WebCore/Modules/fetch/DOMWindowFetch.h:50
> +    static void fetch(DOMWindow&, FetchRequest*, const Dictionary&, FetchPromise&&);

Can the FetchRequest be null? If not, then why is it a pointer rather than a reference?

> Source/WebCore/Modules/fetch/DOMWindowFetch.idl:33
> +    Promise fetch(DOMString input, [Default=Undefined] optional Dictionary init);
> +    Promise fetch(FetchRequest input, [Default=Undefined] optional Dictionary init);

Why do we need both “Default=Undefined” and “optional”? Flaw in the bindings machinery?

> Source/WebCore/Modules/fetch/WorkerGlobalScopeFetch.idl:33
> +    Promise fetch(DOMString input, [Default=Undefined] optional Dictionary init);
> +    Promise fetch(FetchRequest input, [Default=Undefined] optional Dictionary init);

Really strange that we do all the extra work to make this a module but that does nothing to eliminate the duplication between DOMWindow and WorkerGlobalScope.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2899
> +                    push(@implContent, "static inline EncodedJSValue ${functionName}Promise(ExecState*, JSPromiseDeferred*);\n");

There is no reason to specify “inline” here. It has no effect because this is just a declaration and inline only matters in the definition.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2903
> +                    push(@implContent, "inline EncodedJSValue ${functionName}Promise(ExecState*, JSPromiseDeferred*);\n");

There is no reason to specify “inline” here. It has no effect because this is just a declaration and inline only matters in the definition.

[youenn fablet]

Created attachment 273282 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 273282 [details]
Patch for landing

Clearing flags on attachment: 273282

Committed r197748: <http://trac.webkit.org/changeset/197748>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.