[webkit-reviews] review denied: [Bug 225636] Allow logging minimal info about sending media files through XMLHttpRequest : [Attachment 428302] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 12 11:24:34 PDT 2021

Alex Christensen <achristensen at apple.com> has denied Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 225636: Allow logging minimal info about sending media files through

Attachment 428302: Patch


--- Comment #5 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 428302
  --> https://bugs.webkit.org/attachment.cgi?id=428302

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

> Source/WebCore/html/HTMLFormElement.cpp:408
> +   

Let's null check page:
if (auto* page = document().page())

> Source/WebCore/page/Page.cpp:1056
> +void Page::logSubmittedImageOrMediaFiles(unsigned imageOrMediaFilesCount)

Why don't we just make one function and pass a FormData& as a parameter instead
of one function to count then another function to log?

> Source/WebCore/xml/XMLHttpRequest.cpp:549
> +	   if (auto page = scriptExecutionContext()->isDocument() ?
document()->page() : nullptr)


It's also not great to introduce code in XHR that only works in document
context.  This fixes another case of unreported FormData uploading, but workers
are the next case and fetch is probably the one after that.  This might be
nicer in a more central location, like if we added a request parameter to
addParametersShared and checked its HTTP body there, instead of one in
HTMLFormElement and one in xhr.

More information about the webkit-reviews mailing list