Bug 195238 – Fix crashes when NetworkResourceLoader/NetworkLoadChecker are destroyed during HTTPS upgrade check

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Fix crashes when NetworkResourceLoader/NetworkLoadChecker are destroyed during HTTPS upgrade check

[Alex Christensen]

Created attachment 363402 [details]
Patch

[Chris Dumez]

Comment on attachment 363402 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:227
> +    applyHTTPSUpgradeIfNeeded(WTFMove(request), [this, weakThis = makeWeakPtr(*this), client, handler = WTFMove(handler), originalRequest = WTFMove(originalRequest)](auto request) mutable {

Seems suspicious that you'd fix this here but not in the processContentExtensionRulesForLoad() lambda below...

[Alex Christensen]

processContentExtensionRulesForLoad's implementation currently already has this protection, but there's no reason not to make this more robust.

[Alex Christensen]

Created attachment 363534 [details]
Patch

[Chris Dumez]

Comment on attachment 363534 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:244
> +            if (!weakThis)

Personally, I would have put this below the if (!result.has_value()) check so that we may return a more accurate exception when we already have one.

[Alex Christensen]

Created attachment 363539 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 363539 [details]
Patch

Clearing flags on attachment: 363539

Committed r242377: <https://trac.webkit.org/changeset/242377>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48573506>