Bug 196773

Summary: Add a DiagnosticLogging method taking an arbitrary dictionary of values.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, benjamin, cdumez, cmarcelo, dbates, eric.carlson, ews-watchlist, japhet, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch jer.noble: review+

Jer Noble
Reported 2019-04-10 10:13:54 PDT
Add a DiagnosticLogging method taking an arbitrary dictionary of values.
Attachments
Patch (29.51 KB, patch)
2019-04-10 11:32 PDT, Jer Noble
no flags
Patch (26.74 KB, patch)
2019-04-10 13:44 PDT, Jer Noble
no flags
Patch (27.54 KB, patch)
2019-04-15 16:02 PDT, Alex Christensen
no flags
Patch (27.50 KB, patch)
2019-04-15 16:08 PDT, Alex Christensen
jer.noble: review+
Jer Noble
Comment 1 2019-04-10 11:32:53 PDT
EWS Watchlist
Comment 2 2019-04-10 11:35:40 PDT
Attachment 367143 [details] did not pass style-queue: ERROR: Source/WTF/wtf/HashFunctions.h:288: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alex Christensen
Comment 3 2019-04-10 12:38:57 PDT
Comment on attachment 367143 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367143&action=review Let's add HashTraits<Variant<Types...>> and some WTF tests for this. > Source/WTF/wtf/HashFunctions.h:222 > + return std::hash<Variant<Types...>>(a) == std::hash<Variant<Types...>>(b); This should just use operator== without using std::hash.
Jer Noble
Comment 4 2019-04-10 13:43:07 PDT
Turns out, the VariantHash changes are totally unnecessary. I'll remove them (and separate out that work to another patch).
Jer Noble
Comment 5 2019-04-10 13:44:12 PDT
Alex Christensen
Comment 6 2019-04-15 13:47:20 PDT
Comment on attachment 367157 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367157&action=review > Source/WebKit/UIProcess/API/C/WKPageDiagnosticLoggingClient.h:68 > +typedef struct WKPageDiagnosticLoggingClientV2 { Let's not add to the C API for this. > Source/WebKit/UIProcess/API/Cocoa/_WKDiagnosticLoggingDelegate.h:43 > +- (void)_webView:(WKWebView *)webView logDiagnosticMessage:(NSString *)message description:(NSString *)description valueDictionary:(NSDictionary *)valueDictionary; Please add a test that this can be called in WKWebViewDiagnosticLogging.mm.
Alex Christensen
Comment 7 2019-04-15 15:14:48 PDT
Comment on attachment 367157 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367157&action=review >> Source/WebKit/UIProcess/API/Cocoa/_WKDiagnosticLoggingDelegate.h:43 >> +- (void)_webView:(WKWebView *)webView logDiagnosticMessage:(NSString *)message description:(NSString *)description valueDictionary:(NSDictionary *)valueDictionary; > > Please add a test that this can be called in WKWebViewDiagnosticLogging.mm. Also add availability macros.
Alex Christensen
Comment 8 2019-04-15 15:36:17 PDT
Comment on attachment 367157 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367157&action=review > Source/WebKit/UIProcess/Cocoa/DiagnosticLoggingClient.h:64 > + unsigned webviewLogDiagnosticMessageWithValueDictionary : 1; This is never set :(
Alex Christensen
Comment 9 2019-04-15 16:02:01 PDT
Alex Christensen
Comment 10 2019-04-15 16:08:50 PDT
Alex Christensen
Comment 11 2019-04-15 16:49:08 PDT
Radar WebKit Bug Importer
Comment 12 2019-04-15 16:50:44 PDT
Note You need to log in before you can comment on or make changes to this bug.