Bug 212551 - [Cocoa] Improve logging quality for non-ephemeral sessions
Summary: [Cocoa] Improve logging quality for non-ephemeral sessions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-29 16:00 PDT by Brent Fulgham
Modified: 2020-05-30 10:39 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.23 KB, patch)
2020-05-29 16:44 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (6.28 KB, patch)
2020-05-29 16:56 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (6.25 KB, patch)
2020-05-29 17:04 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch for landing (6.32 KB, patch)
2020-05-29 17:37 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2020-05-29 16:00:34 PDT
In Bug 209522 I switched normal mode logging to use the same privacy-protecting mode we use for ephemeral sessions. Unfortunately, this had the side-effect of removing network load data used to triage page load issues.

Instead, adopt the more fine-grained logging provided by the low-level 'nw_context_privacy_level_sensitive' infrastructure.
Comment 1 Brent Fulgham 2020-05-29 16:01:05 PDT
<rdar://problem/62461099>
Comment 2 Brent Fulgham 2020-05-29 16:01:30 PDT
This will give us better ability to investigate page load problems without capturing sensitive user-identifiable logging.
Comment 3 Brent Fulgham 2020-05-29 16:44:23 PDT
Created attachment 400633 [details]
Patch
Comment 4 Brent Fulgham 2020-05-29 16:56:06 PDT
Created attachment 400635 [details]
Patch
Comment 5 David Kilzer (:ddkilzer) 2020-05-29 17:00:40 PDT
Comment on attachment 400635 [details]
Patch

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

r- due to build failures.

> Source/WebCore/PAL/pal/spi/cf/CFNetworkSPI.h:80
> +#ifndef NW_CONTEXT_HAS_PRIVACY_LEVEL_SILENT
> +#define NW_CONTEXT_HAS_PRIVACY_LEVEL_SILENT    1
> +#endif

Why is this needed separately from HAVE(LOGGING_PRIVACY_LEVEL)?

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1033
> +    auto loggingPrivacyLevel = nw_context_privacy_level_sensitive;

Need to protect this with #if HAVE(LOGGING_PRIVACY_LEVEL)/#endif.

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1041
> +#if defined(NW_CONTEXT_HAS_PRIVACY_LEVEL_SILENT)
> +        loggingPrivacyLevel = nw_context_privacy_level_silent;
> +#endif

Why not protect this with #if HAVE(LOGGING_PRIVACY_LEVEL)/#endif?
Comment 6 Brent Fulgham 2020-05-29 17:04:22 PDT
Created attachment 400637 [details]
Patch
Comment 7 Brent Fulgham 2020-05-29 17:07:54 PDT
Comment on attachment 400635 [details]
Patch

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

>> Source/WebCore/PAL/pal/spi/cf/CFNetworkSPI.h:80
>> +#endif
> 
> Why is this needed separately from HAVE(LOGGING_PRIVACY_LEVEL)?

nw_context_privacy_level_silent is new. The other three are existing values. This new #define is available when we build for an Internal SDK, and we need to use it so our builders don't break.

>> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1033
>> +    auto loggingPrivacyLevel = nw_context_privacy_level_sensitive;
> 
> Need to protect this with #if HAVE(LOGGING_PRIVACY_LEVEL)/#endif.

No -- this one is fine. It's an existing value.

>> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1041
>> +#endif
> 
> Why not protect this with #if HAVE(LOGGING_PRIVACY_LEVEL)/#endif?

This one is the special new case that might not exist depending on which SDK/platform you are building on. So it needs its own define.
Comment 8 David Kilzer (:ddkilzer) 2020-05-29 17:13:20 PDT
Comment on attachment 400637 [details]
Patch

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

r=me if you add the #if HAVE(LOGGING_PRIVACY_LEVEL)/#endif guard.

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1033
> +    auto loggingPrivacyLevel = nw_context_privacy_level_sensitive;

This still needs #if HAVE(LOGGING_PRIVACY_LEVEL)/#endif guard otherwise it will be an unused variable.

It should result in a build failure, except I think we have that warning disabled on the WebKit project.
Comment 9 Brent Fulgham 2020-05-29 17:37:35 PDT
Created attachment 400641 [details]
Patch for landing
Comment 10 EWS 2020-05-29 18:19:30 PDT
Committed r262332: <https://trac.webkit.org/changeset/262332>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400641 [details].
Comment 11 David Kilzer (:ddkilzer) 2020-05-30 10:39:37 PDT
(In reply to EWS from comment #10)
> Committed r262332: <https://trac.webkit.org/changeset/262332>

Follow-up build fix for Windows:

Committed r262347: <https://trac.webkit.org/changeset/262347>