Bug 228708 – [App Privacy Report] Domains are deleted for ephemeral website data stores

• Component: New Bugs
• Status: RESOLVED FIXED

[Kate Cheney]

[App Privacy Report] Domains are deleted for ephemeral website data stores

[Kate Cheney]

Created attachment 434781 [details]
Patch

[Kate Cheney]

Comment on attachment 434781 [details]
Patch

Removing r flag, this should probably also fix the Safari Privacy Report case, which is basically an identical fix.

[Alex Christensen]

Comment on attachment 434781 [details]
Patch

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

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1830
> +    if (!dataStoreIsPersistent)

m_sessionID.isEphemeral()
Then you don't need to pass the redundant bool around.

[Kate Cheney]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 434781 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=434781&action=review
> 
> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1830
> > +    if (!dataStoreIsPersistent)
> 
> m_sessionID.isEphemeral()
> Then you don't need to pass the redundant bool around.

Will an ephemeral website data store automatically use an ephemeral session?

[Alex Christensen]

Comment on attachment 434781 [details]
Patch

Yes.  WebsiteDataStore::isPersistent() and NetworkSessionCocoa::m_sessionID.isEphemeral() are opposites, getting their information from the same place.

[Kate Cheney]

Created attachment 434797 [details]
Patch

[Kate Cheney]

(In reply to Alex Christensen from comment #5)
> Comment on attachment 434781 [details]
> Patch
> 
> Yes.  WebsiteDataStore::isPersistent() and
> NetworkSessionCocoa::m_sessionID.isEphemeral() are opposites, getting their
> information from the same place.

TIL.

[Alex Christensen]

Comment on attachment 434797 [details]
Patch

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

r=me

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1830
> +    if (sessionID().isEphemeral()) {

Could you add a radar link where you requested SPI to add an automated test for this?
// FIXME: Add automated test for this once rdar://???????? is resolved.

[Kate Cheney]

Created attachment 434802 [details]
Patch for landing

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 434802 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Kate Cheney]

Created attachment 434803 [details]
Patch for landing

[Maciej Stachowiak]

Comment on attachment 434781 [details]
Patch

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

> Source/WebKit/ChangeLog:11
> +        No new tests. Since this data is not stored in WebKit, we have no
> +        way to test when it gets deleted or not. I tested manually and requested
> +        an automated test at the correct networking layer.

Should we consider making a testing hook for this so that a layout test or API test is possible?

> Source/WebKit/ChangeLog:17
> +        Only clear App Privacy Report data if the website data store is
> +        persistent. Safari calls the SPI to remove website data for ephemeral
> +        data stores in some cases that are unrelated to a user clearing
> +        website data or history and thus should not clear App Privacy Report
> +        data.

Private browsing is also an ephemeral data store. With this change, would private browsing leave traces in the Privacy Report data store, or do we have some other way of avoiding that?

[Maciej Stachowiak]

<rdar://81282432>

[Radar WebKit Bug Importer]

<rdar://problem/81436158>

[Kate Cheney]

(In reply to Maciej Stachowiak from comment #12)
> Comment on attachment 434781 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=434781&action=review
> 
> > Source/WebKit/ChangeLog:11
> > +        No new tests. Since this data is not stored in WebKit, we have no
> > +        way to test when it gets deleted or not. I tested manually and requested
> > +        an automated test at the correct networking layer.
> 
> Should we consider making a testing hook for this so that a layout test or
> API test is possible?

Yes, I requested this in the radar mentioned in the FIXME. The ideal would be SPI that let us test this directly.

> 
> > Source/WebKit/ChangeLog:17
> > +        Only clear App Privacy Report data if the website data store is
> > +        persistent. Safari calls the SPI to remove website data for ephemeral
> > +        data stores in some cases that are unrelated to a user clearing
> > +        website data or history and thus should not clear App Privacy Report
> > +        data.
> 
> Private browsing is also an ephemeral data store. With this change, would
> private browsing leave traces in the Privacy Report data store, or do we
> have some other way of avoiding that?

Ephemeral sessions in full browsers (aka private browsing) do not log any information in the App Privacy Report. However, ephemeral sessions for non-browsers still log domains to prevent sidesteps of APR by using ephemeral sessions.

[Radar WebKit Bug Importer]

<rdar://problem/81436167>

[EWS]

Committed r280575 (240197@main): <https://commits.webkit.org/240197@main>

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