Bug 210363 - Turn all instances of NavigatingToAppBoundDomain into Optionals.
Summary: Turn all instances of NavigatingToAppBoundDomain into Optionals.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kate Cheney
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-10 16:54 PDT by Kate Cheney
Modified: 2020-04-13 11:15 PDT (History)
5 users (show)

See Also:

Attachments
Patch (65.59 KB, patch)
2020-04-10 17:34 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Patch for landing (65.66 KB, patch)
2020-04-13 10:36 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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].