Add WKWebsiteDataTypeHashSalt to allow fetching/removing DeviceIDHashSalt data #10884
Conversation
szewai
added
the
New Bugs
|
EWS run on previous version of this PR (hash c79c913)
|
szewai
force-pushed
the
eng/Add-WKWebsiteDataTypeHashSalt-to-allow-fetchingremoving-DeviceIDHashSalt-data
branch
from
c79c913
to
fbbc05f
Compare
|
EWS run on previous version of this PR (hash fbbc05f)
|
| @@ -140,6 +140,7 @@ RegistrationDatabase::RegistrationDatabase(RegistrationStore& store, String&& da | |||
| , m_databaseDirectory(WTFMove(databaseDirectory)) | |||
| , m_databaseFilePath(FileSystem::pathByAppendingComponent(m_databaseDirectory, databaseFilename())) | |||
| { | |||
| WTFLogAlways("sihuil:[%p]RegistrationDatabase::RegistrationDatabase m_databaseFilePath[%s]", this, m_databaseFilePath.utf8().data()); | |||
| @@ -687,7 +687,7 @@ void WebsiteDataStore::removeData(OptionSet<WebsiteDataType> dataTypes, WallTime | |||
| } | |||
| } | |||
|
|
|||
| if (dataTypes.contains(WebsiteDataType::DeviceIdHashSalt) || (dataTypes.contains(WebsiteDataType::Cookies))) | |||
| if (dataTypes.contains(WebsiteDataType::DeviceIdHashSalt)) | |||
There was a problem hiding this comment.
I am not sure we should do this change.
Spec states: [User Agents](https://w3c.github.io/mediacapture-main/#dfn-user-agent) MUST rotate per-origin device identifiers when other persistent storage are cleared.
There was a problem hiding this comment.
ah so this suggests device id should have the same lifetime as cookies. In this case do we think HashSalt is cookies?
If it is:
Then we should return WKWebsiteDataTypeCookies for website data fetch when clients have DeviceIdHashSalt data, and remove both cookies and hash salt when clients remove WKWebsiteDataTypeCookies data.
If it is not:
Then we should return WKWebsiteDataTypeHashSalt for website data fetch, and let client remove data for WKWebsiteDataTypeHashSalt. (We may keep the existing behavior that removes WKWebsiteDataTypeHashSalt data when clients remove WKWebsiteDataTypeCookies)
Which one do you think it should be?
There was a problem hiding this comment.
I would do the latter, expose it but continue removing it when cookies are cleared.
There was a problem hiding this comment.
Ok, the patch is updated as suggested
szewai
force-pushed
the
eng/Add-WKWebsiteDataTypeHashSalt-to-allow-fetchingremoving-DeviceIDHashSalt-data
branch
from
fbbc05f
to
9ae8f5d
Compare
|
EWS run on current version of this PR (hash 9ae8f5d)
|
| @@ -68,6 +68,9 @@ WK_EXTERN NSString * const WKWebsiteDataTypeSearchFieldRecentSearches WK_API_AVA | |||
| /*! @constant WKWebsiteDataTypeMediaKeys MediaKeys storage */ | |||
| WK_EXTERN NSString * const WKWebsiteDataTypeMediaKeys WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); | |||
|
|
|||
| /*! @constant WKWebsiteDataTypeHashSalt Hash salt for deviceId */ | |||
There was a problem hiding this comment.
Ok, though after reading this comment I still have no idea what this clears.
There was a problem hiding this comment.
It's storage used for this type: https://developer.mozilla.org/en-US/docs/Web/API/MediaDeviceInfo/deviceId...
…Salt data https://bugs.webkit.org/show_bug.cgi?id=253174 rdar://106096684 Reviewed by Chris Dumez. Otherwise the data type cannot be viewed by WebKit client. * Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataRecord.h: * Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataRecord.mm: (dataTypesToString): * Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataRecordInternal.h: (WebKit::toWebsiteDataType): (WebKit::toWKWebsiteDataTypes): * Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm: (+[WKWebsiteDataStore allWebsiteDataTypes]): Canonical link: https://commits.webkit.org/261194@main
webkit-early-warning-system
force-pushed
the
eng/Add-WKWebsiteDataTypeHashSalt-to-allow-fetchingremoving-DeviceIDHashSalt-data
branch
from
9ae8f5d
to
2630f20
Compare
|
Committed 261194@main (2630f20): https://commits.webkit.org/261194@main Reviewed commits have been landed. Closing PR #10884 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
2630f20
9ae8f5d