Bug 215519 - Use closeReason provided in didCloseWithCode delegate for the NSURLSessionWebSocket code path
Summary: Use closeReason provided in didCloseWithCode delegate for the NSURLSessionWeb...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-14 11:50 PDT by jmehta
Modified: 2020-08-21 12:16 PDT (History)
3 users (show)

See Also:


Attachments
Using the closeReason in the delegate (1023 bytes, patch)
2020-08-14 11:51 PDT, jmehta
no flags Details | Formatted Diff | Diff
Updating patch to add assertions and ChangeLog (2.19 KB, patch)
2020-08-20 17:43 PDT, jmehta
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jmehta 2020-08-14 11:50:19 PDT
Use closeReason provided in didCloseWithCode delegate.

Currently we're looking at the task's closeReason (which is not wrong), but making sure that we use the close code and reason provided in the delegate callback to ensure there is no weird bug that we hit in the future
Comment 1 jmehta 2020-08-14 11:51:09 PDT
Created attachment 406612 [details]
Using the closeReason in the delegate
Comment 2 Alex Christensen 2020-08-14 12:38:20 PDT
Comment on attachment 406612 [details]
Using the closeReason in the delegate

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

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1021
> +        auto closeReason = adoptNS([[NSString alloc] initWithData:reason encoding:NSUTF8StringEncoding]);

ASSERT([reason isEqualToData:task.closeReason]);
That'll catch these future bugs you mentioned.
Comment 3 jmehta 2020-08-14 13:02:03 PDT
(In reply to Alex Christensen from comment #2)
> Comment on attachment 406612 [details]
> Using the closeReason in the delegate
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=406612&action=review
> 
> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1021
> > +        auto closeReason = adoptNS([[NSString alloc] initWithData:reason encoding:NSUTF8StringEncoding]);
> 
> ASSERT([reason isEqualToData:task.closeReason]);
> That'll catch these future bugs you mentioned.

That sounds good
Comment 4 jmehta 2020-08-20 17:43:20 PDT
Created attachment 406984 [details]
Updating patch to add assertions and ChangeLog
Comment 5 Radar WebKit Bug Importer 2020-08-21 11:51:14 PDT
<rdar://problem/67570472>
Comment 6 EWS 2020-08-21 12:16:36 PDT
Committed r266011: <https://trac.webkit.org/changeset/266011>

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