Bug 210074 – Create a way to signal if the WKAppBoundDomains list is empty

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Kate Cheney]

We want different behavior for In-App Browser Privacy if WKAppBoundDomains is empty. We should signal this when reading from the plist.

[Radar WebKit Bug Importer]

<rdar://problem/61359228>

[Kate Cheney]

Created attachment 395619 [details]
Patch

[Chris Dumez]

Comment on attachment 395619 [details]
Patch

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

> Source/WebKit/Shared/NavigatingToAppBoundDomain.h:30
> +enum class NavigatingToAppBoundDomain : uint8_t { Yes, No, AppBoundDomainsEmpty };

What does AppBoundDomainsEmpty mean here? Why is it useful to differentiate? This seems like a weird name in this enum. Maybe it should be something like "Maybe" or "Unknown", which would fit better with "No", "Yes".

[Kate Cheney]

(In reply to Chris Dumez from comment #3)
> Comment on attachment 395619 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=395619&action=review
> 
> > Source/WebKit/Shared/NavigatingToAppBoundDomain.h:30
> > +enum class NavigatingToAppBoundDomain : uint8_t { Yes, No, AppBoundDomainsEmpty };
> 
> What does AppBoundDomainsEmpty mean here? Why is it useful to differentiate?
> This seems like a weird name in this enum. Maybe it should be something like
> "Maybe" or "Unknown", which would fit better with "No", "Yes".

It means the WKAppBoundDomains plist entry was empty. I could change it to something like 'Unknown' but I feel like that is less clear than just signaling that the list was empty when asking if something is an app-bound domain.

[Chris Dumez]

(In reply to katherine_cheney from comment #4)
> (In reply to Chris Dumez from comment #3)
> > Comment on attachment 395619 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=395619&action=review
> > 
> > > Source/WebKit/Shared/NavigatingToAppBoundDomain.h:30
> > > +enum class NavigatingToAppBoundDomain : uint8_t { Yes, No, AppBoundDomainsEmpty };
> > 
> > What does AppBoundDomainsEmpty mean here? Why is it useful to differentiate?
> > This seems like a weird name in this enum. Maybe it should be something like
> > "Maybe" or "Unknown", which would fit better with "No", "Yes".
> 
> It means the WKAppBoundDomains plist entry was empty. I could change it to
> something like 'Unknown' but I feel like that is less clear than just
> signaling that the list was empty when asking if something is an app-bound
> domain.

Why is it important to differentiate "No" and "Empty" ?

[Chris Dumez]

(In reply to Chris Dumez from comment #5)
> (In reply to katherine_cheney from comment #4)
> > (In reply to Chris Dumez from comment #3)
> > > Comment on attachment 395619 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=395619&action=review
> > > 
> > > > Source/WebKit/Shared/NavigatingToAppBoundDomain.h:30
> > > > +enum class NavigatingToAppBoundDomain : uint8_t { Yes, No, AppBoundDomainsEmpty };
> > > 
> > > What does AppBoundDomainsEmpty mean here? Why is it useful to differentiate?
> > > This seems like a weird name in this enum. Maybe it should be something like
> > > "Maybe" or "Unknown", which would fit better with "No", "Yes".
> > 
> > It means the WKAppBoundDomains plist entry was empty. I could change it to
> > something like 'Unknown' but I feel like that is less clear than just
> > signaling that the list was empty when asking if something is an app-bound
> > domain.
> 
> Why is it important to differentiate "No" and "Empty" ?

Another alternative would also be Optional<NavigatingToAppBoundDomain>

[Kate Cheney]

(In reply to Chris Dumez from comment #5)
> (In reply to katherine_cheney from comment #4)
> > (In reply to Chris Dumez from comment #3)
> > > Comment on attachment 395619 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=395619&action=review
> > > 
> > > > Source/WebKit/Shared/NavigatingToAppBoundDomain.h:30
> > > > +enum class NavigatingToAppBoundDomain : uint8_t { Yes, No, AppBoundDomainsEmpty };
> > > 
> > > What does AppBoundDomainsEmpty mean here? Why is it useful to differentiate?
> > > This seems like a weird name in this enum. Maybe it should be something like
> > > "Maybe" or "Unknown", which would fit better with "No", "Yes".
> > 
> > It means the WKAppBoundDomains plist entry was empty. I could change it to
> > something like 'Unknown' but I feel like that is less clear than just
> > signaling that the list was empty when asking if something is an app-bound
> > domain.
> 
> Why is it important to differentiate "No" and "Empty" ?

For simplicity, I split this patch out from a bigger one. The behavior of In-App Browser privacy should be different if the list has no entries. Optional<NavigatingToAppBoundDomain> would probably work.

[Kate Cheney]

Created attachment 395626 [details]
Patch

[Brent Fulgham]

Comment on attachment 395626 [details]
Patch

r=me

[EWS]

Committed r259615: <https://trac.webkit.org/changeset/259615>

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