Bug 212551 – [Cocoa] Improve logging quality for non-ephemeral sessions

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brent Fulgham]

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.

[Brent Fulgham]

<rdar://problem/62461099>

[Brent Fulgham]

This will give us better ability to investigate page load problems without capturing sensitive user-identifiable logging.

[Brent Fulgham]

Created attachment 400633 [details]
Patch

[Brent Fulgham]

Created attachment 400635 [details]
Patch

[David Kilzer (:ddkilzer)]

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?

[Brent Fulgham]

Created attachment 400637 [details]
Patch

[Brent Fulgham]

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.

[David Kilzer (:ddkilzer)]

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.

[Brent Fulgham]

Created attachment 400641 [details]
Patch for landing

[EWS]

Committed r262332: <https://trac.webkit.org/changeset/262332>

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

[David Kilzer (:ddkilzer)]

(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>