Summary: | Allow logging minimal info about sending media files through XMLHttpRequest | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Said Abou-Hallawa <sabouhallawa> | ||||||||||
Component: | Images | Assignee: | Said Abou-Hallawa <sabouhallawa> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, cdumez, changseok, esprehn+autocc, ews-watchlist, gyuyoung.kim, mifenton, simon.fraser, thorton, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=223706 | ||||||||||||
Attachments: |
|
Description
Said Abou-Hallawa
2021-05-10 22:26:57 PDT
Created attachment 428252 [details]
Patch
Created attachment 428254 [details]
Patch
Created attachment 428302 [details]
Patch
Comment on attachment 428302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428302&action=review > Source/WebCore/html/HTMLFormElement.cpp:408 > + document().page()->logSubmittedImageOrMediaFiles(imageOrMediaFilesCount); Let's null check page: if (auto* page = document().page()) 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) auto* 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. Created attachment 428703 [details]
Patch
Comment on attachment 428302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428302&action=review >> Source/WebCore/xml/XMLHttpRequest.cpp:549 >> + if (auto page = scriptExecutionContext()->isDocument() ? document()->page() : nullptr) > > auto* > > 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. This is a great idea. I was looking for a common place for all file uploading but I could not find one because I was looking for it in WebCore. I added the logging in addParametersShared(). Committed r277666 (237868@main): <https://commits.webkit.org/237868@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428703 [details]. |