Bug 210363

Summary: Turn all instances of NavigatingToAppBoundDomain into Optionals.
Product: WebKit Reporter: Kate Cheney <katherine_cheney>
Component: WebKit Misc.Assignee: Kate Cheney <katherine_cheney>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, cgarcia, ews-watchlist, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Kate Cheney 2020-04-10 16:54:23 PDT 
We should distinguish between the case of an empty app-bound domains list everywhere for future behavior changes (currently it is only an Optional until it reaches WebPageProxy).
Comment 1 Radar WebKit Bug Importer 2020-04-10 16:55:04 PDT 
<rdar://problem/61612861>
Comment 2 Kate Cheney 2020-04-10 17:34:55 PDT 
Created attachment 396138 [details]
Patch
Comment 3 Brent Fulgham 2020-04-13 10:00:28 PDT 
Comment on attachment 396138 [details]
Patch

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

r=me

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1203
> +    auto shouldBeConsideredAppBound = !isNavigatingToAppBoundDomain ? NavigatingToAppBoundDomain::Yes : *isNavigatingToAppBoundDomain;

Nit: I had to think about this for a minute before I understood it. I think this *might* be clearer:

auto shouldBeConsideredAppBound = isNavigatingToAppBoundDomain ? *isNavigatingToAppBoundDomain : NavigatingToAppBoundDomain::Yes;
Comment 4 Kate Cheney 2020-04-13 10:09:24 PDT 
(In reply to Brent Fulgham from comment #3)
> Comment on attachment 396138 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=396138&action=review
> 
> r=me
> 
> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1203
> > +    auto shouldBeConsideredAppBound = !isNavigatingToAppBoundDomain ? NavigatingToAppBoundDomain::Yes : *isNavigatingToAppBoundDomain;
> 
> Nit: I had to think about this for a minute before I understood it. I think
> this *might* be clearer:
> 
> auto shouldBeConsideredAppBound = isNavigatingToAppBoundDomain ?
> *isNavigatingToAppBoundDomain : NavigatingToAppBoundDomain::Yes;

Good call. I'll make this change before landing. Thanks!
Comment 5 Kate Cheney 2020-04-13 10:36:35 PDT 
Created attachment 396305 [details]
Patch for landing
Comment 6 EWS 2020-04-13 11:15:48 PDT 
Committed r260031: <https://trac.webkit.org/changeset/260031>

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