In order to support the tracking of the efficacy of Intelligent Tracking Protection, add some logging of the cookie information associated with each loaded resource. This logging is off by default, and is enabled with `sudo defaults write -g WebKitLogCookieInformation -bool true`.
Created attachment 329520 [details] Patch
Attachment 329520 [details] did not pass style-queue: ERROR: Source/WebCore/platform/network/cocoa/CookieCocoa.mm:76: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 329521 [details] Fix GTK build.
Comment on attachment 329520 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329520&action=review Do we care to measure the creation of cookies that are made from a Set-Cookie header field and never leave CFNetwork? > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:759 > + "{ \"name\"" ": \"%{public}s\"" > + ", \"value\"" ": \"%{public}s\"" > + ", \"domain\"" ": \"%{public}s\"" What are these logs going to? They contain privacy sensitive information.
Attachment 329521 [details] did not pass style-queue: ERROR: Source/WebCore/platform/network/cocoa/CookieCocoa.mm:76: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 329710 [details] Patch
Attachment 329710 [details] did not pass style-queue: ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:733: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:734: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:735: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:745: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:746: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:747: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:748: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:754: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:755: Missing space before { [whitespace/braces] [5] ERROR: Source/WebCore/platform/network/cocoa/CookieCocoa.mm:76: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 10 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 329743 [details] Patch
Attachment 329743 [details] did not pass style-queue: ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:740: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:741: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:742: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:752: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:753: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:754: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:755: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:761: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:762: Missing space before { [whitespace/braces] [5] ERROR: Source/WebCore/platform/network/cocoa/CookieCocoa.mm:76: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 10 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 329744 [details] Patch
Attachment 329744 [details] did not pass style-queue: ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:740: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:741: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:742: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:752: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:753: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:754: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:755: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:761: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:762: Missing space before { [whitespace/braces] [5] ERROR: Source/WebCore/platform/network/cocoa/CookieCocoa.mm:76: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 10 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 329774 [details] Patch
Attachment 329774 [details] did not pass style-queue: ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:740: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:741: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:742: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:752: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:753: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:754: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:755: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:761: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:762: Missing space before { [whitespace/braces] [5] ERROR: Source/WebCore/platform/network/cocoa/CookieCocoa.mm:76: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 10 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 329825 [details] Patch
Attachment 329825 [details] did not pass style-queue: ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:740: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:741: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:742: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:752: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:753: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:754: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:755: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:761: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:762: Missing space before { [whitespace/braces] [5] ERROR: Source/WebCore/platform/network/cocoa/CookieCocoa.mm:76: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 10 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 329865 [details] Patch
Attachment 329865 [details] did not pass style-queue: ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:740: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:741: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:742: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:752: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:753: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:754: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:755: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:761: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:762: Missing space before { [whitespace/braces] [5] ERROR: Source/WebCore/platform/network/cocoa/CookieCocoa.mm:76: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 10 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
<rdar://problem/35802295>
(In reply to Keith Rollin from comment #0) > In order to support the tracking of the efficacy of Intelligent Tracking > Protection, add some logging of the cookie information associated with each > loaded resource. This logging is off by default, and is enabled with `sudo > defaults write -g WebKitLogCookieInformation -bool true`. Unless you are running Safari as root, you probably don't need the 'sudo'. :-)
Comment on attachment 329865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329865&action=review Looks (and works!) well. r=me, but please consider not dumping cookie logging when there are no cookies. > Source/WebCore/platform/network/cocoa/CookieCocoa.mm:60 > + id value = props[@"Created"]; Maybe: id value = cookie.props[@"Created"]; > Source/WebCore/platform/network/cocoa/CookieCocoa.mm:67 > + return toCanonicalFormat(((NSNumber*) value).doubleValue); Please use ugly Objective-C syntax: "((NSNumber *)value)" and elsewhere. > Source/WebCore/platform/network/cocoa/CookieCocoa.mm:108 > + // FIXME: Set Created property Do we need to do this in this patch? > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:743 > + LOCAL_LOG(R"( "cookies": [)"); Do we want to output this information when the cookie.size() == 0? Maybe that's just noise in the log?
(In reply to Brent Fulgham from comment #20) > Comment on attachment 329865 [details] > > Source/WebCore/platform/network/cocoa/CookieCocoa.mm:60 > > + id value = props[@"Created"]; > > Maybe: > > id value = cookie.props[@"Created"]; OK. > > Source/WebCore/platform/network/cocoa/CookieCocoa.mm:67 > > + return toCanonicalFormat(((NSNumber*) value).doubleValue); > > Please use ugly Objective-C syntax: "((NSNumber *)value)" and elsewhere. I don't understand this comment. I don't know what you're suggesting I do. > > Source/WebCore/platform/network/cocoa/CookieCocoa.mm:108 > > + // FIXME: Set Created property > > Do we need to do this in this patch? It matched a similar comment already in the function. I could take it out. > > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:743 > > + LOCAL_LOG(R"( "cookies": [)"); > > Do we want to output this information when the cookie.size() == 0? Maybe > that's just noise in the log? OK.
(In reply to Keith Rollin from comment #21) > (In reply to Brent Fulgham from comment #20) > > > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:743 > > > + LOCAL_LOG(R"( "cookies": [)"); > > > > Do we want to output this information when the cookie.size() == 0? Maybe > > that's just noise in the log? > > OK. Actually, I think I'd like to leave it in. By leaving it in, we make the client parsing easier (it doesn't have to test to see if 'cookies' exists). Also, while making the suggested change, I found that the code because more complex. If we make 'cookies' optional, then the preceding comma needs to be elided, and the line that closes the array and the object ("]}") at the same time needs to be split up. Because of the os_log works, it's problematic to handle these variations by building up text lines, and instead we need to handle all of the variations with separate condition logic that outputs crafted constant lines. I could do it, but the code is a lot uglier, harder to follow, and harder to extend.
(In reply to Keith Rollin from comment #22) > (In reply to Keith Rollin from comment #21) > > (In reply to Brent Fulgham from comment #20) > > > > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:743 > > > > + LOCAL_LOG(R"( "cookies": [)"); > > > > > > Do we want to output this information when the cookie.size() == 0? Maybe > > > that's just noise in the log? > > > > OK. > > Actually, I think I'd like to leave it in. By leaving it in, we make the > client parsing easier (it doesn't have to test to see if 'cookies' exists). > Also, while making the suggested change, I found that the code because more > complex. If we make 'cookies' optional, then the preceding comma needs to be > elided, and the line that closes the array and the object ("]}") at the same > time needs to be split up. Because of the os_log works, it's problematic to > handle these variations by building up text lines, and instead we need to > handle all of the variations with separate condition logic that outputs > crafted constant lines. I could do it, but the code is a lot uglier, harder > to follow, and harder to extend. Fair enough!
(In reply to Keith Rollin from comment #21) > (In reply to Brent Fulgham from comment #20) > > Comment on attachment 329865 [details] > > > > Source/WebCore/platform/network/cocoa/CookieCocoa.mm:67 > > > + return toCanonicalFormat(((NSNumber*) value).doubleValue); > > > > Please use ugly Objective-C syntax: "((NSNumber *)value)" and elsewhere. > > I don't understand this comment. I don't know what you're suggesting I do. Instead of this: ((NSNumber*) value).doubleValue Do: ((NSNumber *)value).doubleValue ... and similar changes elsewhere,
(In reply to Brent Fulgham from comment #19) > (In reply to Keith Rollin from comment #0) > > .. and is enabled with `sudo > > defaults write -g WebKitLogCookieInformation -bool true`. > > Unless you are running Safari as root, you probably don't need the 'sudo'. > :-) Huh. I thought I determined that it was needed. But you're right -- it's not.
Created attachment 330017 [details] Patch
Attachment 330017 [details] did not pass style-queue: ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:740: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:741: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:742: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:752: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:753: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:754: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:755: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:761: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:762: Missing space before { [whitespace/braces] [5] ERROR: Source/WebCore/platform/network/cocoa/CookieCocoa.mm:75: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 10 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 330017 [details] Patch Attachment 330017 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5786734 New failing tests: fast/mediastream/MediaStream-MediaElement-setObject-null.html
Created attachment 330023 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 330017 [details] Patch Clearing flags on attachment: 330017 Committed r226226: <https://trac.webkit.org/changeset/226226>
All reviewed patches have been landed. Closing bug.