Bug 196773 - Add a DiagnosticLogging method taking an arbitrary dictionary of values.
Summary: Add a DiagnosticLogging method taking an arbitrary dictionary of values.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-10 10:13 PDT by Jer Noble
Modified: 2019-04-15 16:50 PDT (History)
9 users (show)

See Also:


Attachments
Patch (29.51 KB, patch)
2019-04-10 11:32 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (26.74 KB, patch)
2019-04-10 13:44 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (27.54 KB, patch)
2019-04-15 16:02 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (27.50 KB, patch)
2019-04-15 16:08 PDT, Alex Christensen
jer.noble: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2019-04-10 10:13:54 PDT
Add a DiagnosticLogging method taking an arbitrary dictionary of values.
Comment 1 Jer Noble 2019-04-10 11:32:53 PDT
Created attachment 367143 [details]
Patch
Comment 2 Build Bot 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.
Comment 3 Alex Christensen 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.
Comment 4 Jer Noble 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).
Comment 5 Jer Noble 2019-04-10 13:44:12 PDT
Created attachment 367157 [details]
Patch
Comment 6 Alex Christensen 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.
Comment 7 Alex Christensen 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.
Comment 8 Alex Christensen 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 :(
Comment 9 Alex Christensen 2019-04-15 16:02:01 PDT
Created attachment 367469 [details]
Patch
Comment 10 Alex Christensen 2019-04-15 16:08:50 PDT
Created attachment 367471 [details]
Patch
Comment 11 Alex Christensen 2019-04-15 16:49:08 PDT
http://trac.webkit.org/r244307
Comment 12 Radar WebKit Bug Importer 2019-04-15 16:50:44 PDT
<rdar://problem/49923704>