Bug 207067 - Add KVO SPI WKWebView._negotiatedLegacyTLS
Summary: Add KVO SPI WKWebView._negotiatedLegacyTLS
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: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-31 13:30 PST by Alex Christensen
Modified: 2020-02-03 14:05 PST (History)
4 users (show)

See Also:


Attachments
patch (23.30 KB, patch)
2020-01-31 13:33 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
patch (23.30 KB, patch)
2020-01-31 14:01 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
patch with SPI documentation. (23.54 KB, patch)
2020-01-31 14:12 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (24.19 KB, patch)
2020-01-31 14:32 PST, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2020-01-31 13:30:45 PST
Add KVO SPI WKWebView._negotiatedLegacyTLS
Comment 1 Alex Christensen 2020-01-31 13:33:31 PST
Created attachment 389403 [details]
patch
Comment 2 Radar WebKit Bug Importer 2020-01-31 13:37:12 PST
<rdar://problem/59072968>
Comment 3 Alex Christensen 2020-01-31 14:01:16 PST
Created attachment 389412 [details]
patch
Comment 4 Alex Christensen 2020-01-31 14:12:49 PST
Created attachment 389413 [details]
patch with SPI documentation.
Comment 5 Andy Estes 2020-01-31 14:28:00 PST
Comment on attachment 389413 [details]
patch with SPI documentation.

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

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:820
> +        auto tlsVersion = (tls_protocol_version_t)metrics.negotiatedTLSProtocolVersion.unsignedShortValue;

reinterpret_cast?

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:821
> +        if (tlsVersion == tls_protocol_version_TLSv10 || tlsVersion == tls_protocol_version_TLSv11)

`tlsVersion < tls_protocol_version_TLSv12`?

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:828
> +            if (tlsVersion == kTLSProtocol11 || tlsVersion == kTLSProtocol1)

Ditto?

> Source/WebKit/UIProcess/PageLoadState.h:226
> +        double estimatedProgress { 0.0 };

Our style is to just use 0 here.
Comment 6 Alex Christensen 2020-01-31 14:32:04 PST
(In reply to Andy Estes from comment #5)
> Comment on attachment 389413 [details]
> patch with SPI documentation.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389413&action=review
> 
> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:820
> > +        auto tlsVersion = (tls_protocol_version_t)metrics.negotiatedTLSProtocolVersion.unsignedShortValue;
> 
> reinterpret_cast?
Sounds good.

> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:821
> > +        if (tlsVersion == tls_protocol_version_TLSv10 || tlsVersion == tls_protocol_version_TLSv11)
> 
> `tlsVersion < tls_protocol_version_TLSv12`?
> 
> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:828
> > +            if (tlsVersion == kTLSProtocol11 || tlsVersion == kTLSProtocol1)
> 
> Ditto?
Strangely enough, we only want to do that with these two particular protocols here.

> > Source/WebKit/UIProcess/PageLoadState.h:226
> > +        double estimatedProgress { 0.0 };
> 
> Our style is to just use 0 here.
I think you're right, even though I think we should change our style in this wonderfully statically-typed language with typed literals.  I'll change to 0.
Comment 7 Alex Christensen 2020-01-31 14:32:58 PST
Created attachment 389416 [details]
Patch
Comment 8 Alex Christensen 2020-01-31 14:34:09 PST
Also of note: we use c-style casts heavily in WebKit's ObjC++ code.  Maybe we should go through and change them all to c++-style casts.
Comment 9 WebKit Commit Bot 2020-01-31 15:16:52 PST
Comment on attachment 389416 [details]
Patch

Clearing flags on attachment: 389416

Committed r255522: <https://trac.webkit.org/changeset/255522>
Comment 10 WebKit Commit Bot 2020-01-31 15:16:54 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Andy Estes 2020-01-31 15:36:13 PST
(In reply to Alex Christensen from comment #8)
> Also of note: we use c-style casts heavily in WebKit's ObjC++ code.  Maybe
> we should go through and change them all to c++-style casts.

I think our rule (or at least the rule I follow) is to use a C-style cast for Objective-C and CF types (e.g., (__bridge CFSStringRef)@"hello") and C++-style casts otherwise. Or perhaps more generally, use C++ casts for the code that caused you to create the .mm file and C-style casts otherwise.
Comment 12 Alex Christensen 2020-02-03 14:05:16 PST
https://trac.webkit.org/changeset/255527/webkit was a build fix after this.