Bug 223706

Summary: Allow logging minimal info about uploading media files in the system diagnose
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: ImagesAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, cdumez, changseok, esprehn+autocc, ews-watchlist, gyuyoung.kim, japhet, mifenton, ryuan.choi, sergio, 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=225636
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Said Abou-Hallawa 2021-03-24 12:09:12 PDT
Give the WebKit client the ability to log info about the media files which was submitted to a web server through an HTMLFormElement.
Comment 1 Said Abou-Hallawa 2021-03-24 12:45:30 PDT
Created attachment 424169 [details]
Patch
Comment 2 Said Abou-Hallawa 2021-03-24 12:55:41 PDT
Created attachment 424171 [details]
Patch
Comment 3 Simon Fraser (smfr) 2021-03-24 13:29:04 PDT
Comment on attachment 424171 [details]
Patch

Is doing this by extending the C API the right way?
Comment 4 Alex Christensen 2021-03-24 13:46:43 PDT
Comment on attachment 424171 [details]
Patch

Let's not add to the C API.  If someone is using the C API that needs to get this new message, they need to migrate to the ObjC API.
I'm also not sure if we need to add LogDiagnosticMessageWithDomain.  Can one of the other logging methods do?
Comment 5 Said Abou-Hallawa 2021-03-24 14:07:53 PDT
(In reply to Alex Christensen from comment #4)
> Comment on attachment 424171 [details]
> Patch
> 
> Let's not add to the C API.  If someone is using the C API that needs to get
> this new message, they need to migrate to the ObjC API.
> I'm also not sure if we need to add LogDiagnosticMessageWithDomain.  Can one
> of the other logging methods do?

Can you give an example which I can follow?
Comment 6 Radar WebKit Bug Importer 2021-03-24 14:17:49 PDT
<rdar://problem/75804136>
Comment 7 Said Abou-Hallawa 2021-03-24 16:02:57 PDT
Created attachment 424196 [details]
Patch
Comment 8 Tim Horton 2021-03-24 16:49:21 PDT
Comment on attachment 424196 [details]
Patch

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

> Source/WebCore/html/HTMLFormElement.cpp:394
> +        diagnosticLoggingClient.logDiagnosticMessageWithDomain(message, "WebKit.media"_s);

Where did the domain come from? The capitalization is sort of vaguely weird.
Comment 9 Alex Christensen 2021-03-24 17:05:23 PDT
Comment on attachment 424196 [details]
Patch

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

>> Source/WebCore/html/HTMLFormElement.cpp:394
>> +        diagnosticLoggingClient.logDiagnosticMessageWithDomain(message, "WebKit.media"_s);
> 
> Where did the domain come from? The capitalization is sort of vaguely weird.

It seems like you want to log an integer.  Would logDiagnosticMessageWithValue work?
Comment 10 Alex Christensen 2021-03-24 17:16:15 PDT
Comment on attachment 424196 [details]
Patch

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

>>> Source/WebCore/html/HTMLFormElement.cpp:394
>>> +        diagnosticLoggingClient.logDiagnosticMessageWithDomain(message, "WebKit.media"_s);
>> 
>> Where did the domain come from? The capitalization is sort of vaguely weird.
> 
> It seems like you want to log an integer.  Would logDiagnosticMessageWithValue work?

Could the domain be an enum instead, and an NS_ENUM at the API layer?  It would start out with just one thing in it, but we could add more later.
Comment 11 Said Abou-Hallawa 2021-03-24 17:27:54 PDT
Comment on attachment 424196 [details]
Patch

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

>>>> Source/WebCore/html/HTMLFormElement.cpp:394
>>>> +        diagnosticLoggingClient.logDiagnosticMessageWithDomain(message, "WebKit.media"_s);
>>> 
>>> Where did the domain come from? The capitalization is sort of vaguely weird.
>> 
>> It seems like you want to log an integer.  Would logDiagnosticMessageWithValue work?
> 
> Could the domain be an enum instead, and an NS_ENUM at the API layer?  It would start out with just one thing in it, but we could add more later.

I think the enum is a better approach.
Comment 12 Said Abou-Hallawa 2021-03-24 21:48:35 PDT
Created attachment 424216 [details]
Patch
Comment 13 EWS 2021-03-26 10:08:13 PDT
Committed r275103: <https://commits.webkit.org/r275103>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424216 [details].