We want different behavior for In-App Browser Privacy if WKAppBoundDomains is empty. We should signal this when reading from the plist.
<rdar://problem/61359228>
Created attachment 395619 [details] Patch
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".
(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.
(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" ?
(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>
(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.
Created attachment 395626 [details] Patch
Comment on attachment 395626 [details] Patch r=me
Committed r259615: <https://trac.webkit.org/changeset/259615> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395626 [details].