Bug 233900

Summary: Reload web views when toggling the captive portal mode at system level
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, ggaren, kkinnunen, pvollan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=233954
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch none

Description Chris Dumez 2021-12-06 16:14:08 PST
Reload web views when toggling the captive portal mode at system level, so that the views end up being backed by WebProcesses with the proper captive portal mode.
Comment 1 Chris Dumez 2021-12-06 16:15:46 PST
Created attachment 446091 [details]
Patch
Comment 2 Chris Dumez 2021-12-06 16:37:40 PST
Created attachment 446096 [details]
Patch
Comment 3 Brent Fulgham 2021-12-06 16:42:01 PST
Comment on attachment 446096 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:979
> +            page->reload({ });

Is the 'reload' process smart enough to handle cases where something is opted-out?
Comment 4 Chris Dumez 2021-12-06 17:16:29 PST
(In reply to Brent Fulgham from comment #3)
> Comment on attachment 446096 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=446096&action=review
> 
> r=me
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:979
> > +            page->reload({ });
> 
> Is the 'reload' process smart enough to handle cases where something is
> opted-out?

The reload process will trigger a decidePolicyForNavigationAction and it is up to the client app to override via WKWebpagePreferences then. I guess it is not ideal in this case since we're going to reload unnecessarily in such case.

Not sure how to best deal with this.
Comment 5 Chris Dumez 2021-12-07 08:07:25 PST
(In reply to Chris Dumez from comment #4)
> (In reply to Brent Fulgham from comment #3)
> > Comment on attachment 446096 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=446096&action=review
> > 
> > r=me
> > 
> > > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:979
> > > +            page->reload({ });
> > 
> > Is the 'reload' process smart enough to handle cases where something is
> > opted-out?
> 
> The reload process will trigger a decidePolicyForNavigationAction and it is
> up to the client app to override via WKWebpagePreferences then. I guess it
> is not ideal in this case since we're going to reload unnecessarily in such
> case.
> 
> Not sure how to best deal with this.

I *think* I can keep track of which views relied on the system setting and which views were explicitly opted in or out via WKWebpagePreferences. Then I would only reload the ones that relied on the system setting. I am looking into this now.
Comment 6 Chris Dumez 2021-12-07 08:28:50 PST
Created attachment 446175 [details]
Patch
Comment 7 Chris Dumez 2021-12-07 09:10:59 PST
Created attachment 446183 [details]
Patch
Comment 8 EWS 2021-12-07 11:36:07 PST
Committed r286602 (244927@trunk): <https://commits.webkit.org/244927@trunk>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 446183 [details].
Comment 9 Radar WebKit Bug Importer 2021-12-07 11:37:26 PST
<rdar://problem/86168069>