Created attachment 273181 [details] Patch

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.

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.

Created attachment 273282 [details] Patch for landing

Comment on attachment 273282 [details] Patch for landing Clearing flags on attachment: 273282 Committed r197748: <http://trac.webkit.org/changeset/197748>

All reviewed patches have been landed. Closing bug.