Bug 210363 – Turn all instances of NavigatingToAppBoundDomain into Optionals.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Kate Cheney]

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).

[Radar WebKit Bug Importer]

<rdar://problem/61612861>

[Kate Cheney]

Created attachment 396138 [details]
Patch

[Brent Fulgham]

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;

[Kate Cheney]

(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!

[Kate Cheney]

Created attachment 396305 [details]
Patch for landing

[EWS]

Committed r260031: <https://trac.webkit.org/changeset/260031>

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