Bug 237735

Summary:

Adjust when _setPrivacyProxyFailClosedForUnreachableNonMainHosts is called

Product:

WebKit

Reporter:

Alex Christensen <achristensen>

Component:

New Bugs

Assignee:

Alex Christensen <achristensen>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, commit-queue, ggaren, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

237829

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Alex Christensen

Reported 2022-03-10 14:55:10 PST

Adjust when _setPrivacyProxyFailClosedForUnreachableNonMainHosts is called

Attachments
Patch (3.40 KB, patch) 2022-03-10 14:57 PST, Alex Christensen	no flags	Details Formatted Diff Diff
Patch (4.50 KB, patch) 2022-03-11 19:37 PST, Alex Christensen	no flags	Details Formatted Diff Diff
Patch (4.52 KB, patch) 2022-03-17 11:30 PDT, Alex Christensen	no flags	Details Formatted Diff Diff
Patch (4.45 KB, patch) 2022-04-13 17:17 PDT, Alex Christensen	no flags	Details Formatted Diff Diff
Patch (4.39 KB, patch) 2022-04-13 17:18 PDT, Alex Christensen	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Alex Christensen

Comment 1 2022-03-10 14:57:30 PST

Created attachment 454406 [details] Patch

Alex Christensen

Comment 2 2022-03-10 14:57:34 PST

<rdar://problem/89972004>

Geoffrey Garen

Comment 3 2022-03-11 10:09:01 PST

Comment on attachment 454406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454406&action=review r=me > Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:346 > + || request.url().host() == parameters.topOrigin->host()) Is there a condition where (isMainFrameNavigation && parameters.topOrigin && request.url().host() != parameters.topOrigin->host())? If not, maybe we don't need to check isMainFrameNavigation.

Alex Christensen

Comment 4 2022-03-11 18:31:58 PST

Good point. I also need to modify web sockets.

Alex Christensen

Comment 5 2022-03-11 18:54:32 PST

parameters.topOrigin is sometimes null and sometimes contains an origin with an empty host. I'm going to stay on the safe side and keep the main frame main resource check.

Alex Christensen

Comment 6 2022-03-11 19:37:15 PST

Created attachment 454526 [details] Patch

Alex Christensen

Comment 7 2022-03-11 19:38:05 PST

Comment on attachment 454526 [details] Patch I verified that this does not re-introduce the original issue. Intuitively this should fix the perf regression, and I'll check it in and see if the bots agree.

EWS

Comment 8 2022-03-11 21:02:35 PST

Committed r291206 (248361@main): <https://commits.webkit.org/248361@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454526 [details].

WebKit Commit Bot

Comment 9 2022-03-14 08:44:11 PDT

Re-opened since this is blocked by bug 237829

Alex Christensen

Comment 10 2022-03-17 11:30:00 PDT

Created attachment 454998 [details] Patch

Geoffrey Garen

Comment 11 2022-03-17 11:48:19 PDT

Comment on attachment 454998 [details] Patch r=me Not sure what was wrong with the previous patch, or how this fixes it. Can you add that note in the ChangeLog?

Alex Christensen

Comment 12 2022-03-17 14:22:36 PDT

I'm under the impression that there was nothing wrong with the previous patch, but it was rolled out before the bots caught up to the fact that it fixed a performance regression.

Alex Christensen

Comment 13 2022-03-21 20:28:21 PDT

http://trac.webkit.org/r291598

Alex Christensen

Comment 14 2022-03-21 20:30:44 PDT

This was hopefully the final fix for rdar://88965550

Alex Christensen

Comment 15 2022-04-05 15:03:35 PDT

Reverting yet again in https://bugs.webkit.org/show_bug.cgi?id=238842

Alex Christensen

Comment 16 2022-04-13 17:17:41 PDT

Reopening to attach new patch.