Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement removeDataOfTypes to WKWebsiteDataStore #13605

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kshukuwa
Copy link
Contributor

@kshukuwa kshukuwa commented May 8, 2023

1300c28

Implement removeDataOfTypes to WKWebsiteDataStore
https://bugs.webkit.org/show_bug.cgi?id=256452

Reviewed by NOBODY (OOPS!).

Add a function to WKAPI similar to WKWebsiteDataStore::removeDataOfTypes
in the Cocoa port.

* Source/WebKit/Headers.cmake:
* Source/WebKit/Shared/API/c/WKBase.h:
* Source/WebKit/Sources.txt:
* Source/WebKit/SourcesCocoa.txt:
* Source/WebKit/UIProcess/API/C/WKAPICast.cpp: Added.
(WebKit::toWebsiteDataTypes):
(WebKit::toAPI):
* Source/WebKit/UIProcess/API/C/WKAPICast.h:
* Source/WebKit/UIProcess/API/C/WKWebsiteDataRecordRef.cpp: Copied from Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingTreeIOS.h.
(WKWebsiteDataRecordGetTypeID):
(WKWebsiteDataRecordGetDisplayName):
(WKWebsiteDataRecordGetDataTypes):
* Source/WebKit/UIProcess/API/C/WKWebsiteDataRecordRef.h: Added.
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:
(WKWebsiteDataStoreRemoveITPDataForDomain):
(WKWebsiteDataStoreStatisticsClearThroughWebsiteDataRemoval):
(WKWebsiteDataStoreRemoveAllFetchCaches):
(WKWebsiteDataStoreRemoveNetworkCache):
(WKWebsiteDataStoreRemoveMemoryCaches):
(WKWebsiteDataStoreRemoveFetchCacheForOrigin):
(WKWebsiteDataStoreRemoveAllIndexedDatabases):
(WKWebsiteDataStoreRemoveLocalStorage):
(WKWebsiteDataStoreRemoveAllServiceWorkerRegistrations):
(WKWebsiteDataStoreClearPrivateClickMeasurementsThroughWebsiteDataRemoval):
(WKWebsiteDataStoreClearStorage):
(WKWebsiteDataStoreAllWebsiteDataTypes):
(WKWebsiteDataStoreRemoveDataOfTypes):
(WKWebsiteDataStoreRemoveDataOfTypesWithValues):
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h:
* Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingTreeIOS.cpp:
* Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingTreeIOS.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.h:
* Source/WebKit/WebKit.xcodeproj/project.pbxproj:
* Tools/MiniBrowser/win/WebKitBrowserWindow.cpp:
(removeDataOfTypesCallback):
(WebKitBrowserWindow::clearWebsiteData):
(removeAllFetchCachesCallback): Deleted.
(removeNetworkCacheCallback): Deleted.
(removeMemoryCachesCallback): Deleted.
(removeAllServiceWorkerRegistrationsCallback): Deleted.
(removeAllIndexedDatabasesCallback): Deleted.
(removeLocalStorageCallback): Deleted.
* Tools/WebKitTestRunner/TestController.cpp:
(WTR::TestController::clearServiceWorkerRegistrations):
(WTR::TestController::clearDOMCache):
(WTR::TestController::clearDOMCaches):
(WTR::TestController::clearMemoryCache):
(WTR::TestController::clearIndexedDatabases):
(WTR::TestController::clearLocalStorage):
(WTR::TestController::clearStorage):
(WTR::TestController::clearStatisticsDataForDomain):
(WTR::TestController::statisticsClearThroughWebsiteDataRemoval):
(WTR::TestController::clearPrivateClickMeasurementsThroughWebsiteDataRemoval):
* Tools/WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):

1300c28

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 8, 2023
@Ahmad-S792 Ahmad-S792 added the WebKit2 Bugs relating to the WebKit2 API layer label May 8, 2023
Copy link
Contributor

@achristensen07 achristensen07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is mostly heading in a good direction, and hopefully my feedback can make it so I think it's completely heading in a good direction.

Source/WebKit/UIProcess/API/C/WKAPICast.h Outdated Show resolved Hide resolved
Source/WebKit/UIProcess/API/APIWebsiteDataRecord.h Outdated Show resolved Hide resolved
return WebKit::toAPI(API::WebsiteDataRecord::APIType);
}

WKWebsiteDataRecordRef WKWebsiteDataRecordCreate()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't exist. Same with the setters. Instead, I think we should make something like WKWebsiteDataStore.allWebsiteDataTypes, which will probably require making a WKSetRef type like WKDictionaryRef.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just kidding. I meant fetchDataRecordsOfTypes, which means we don't need a WKSetRef. We already have WKArrayRef.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean that WKWebsiteDataRecordRef is unnecessary?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It it probably necessary, but it should not be constructible or modifiable by API clients. It is a read-only object that is only constructed inside of WebKit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed WKWebsiteDataStoreRemoveDataOfTypesWithDataRecord as WKWebsiteDataRecord is not available and created WK APIs like WKWebsiteDataStoreRemoveDataOfTypesWithValues instead. What do you think?

Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp Outdated Show resolved Hide resolved
Source/WebKit/UIProcess/API/C/WKWebsiteDataRecordRef.h Outdated Show resolved Hide resolved
@kshukuwa
Copy link
Contributor Author

I will add new files to the xcode project.

@kshukuwa
Copy link
Contributor Author

The following files are not directly related to this fix. Adding WK???.cpp to the build target causes a build error related to Unified Sources, which has been fixed.

Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.h
Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingTreeIOS.h
Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingTreeIOS.cpp

https://bugs.webkit.org/show_bug.cgi?id=256452

Reviewed by NOBODY (OOPS!).

Add a function to WKAPI similar to WKWebsiteDataStore::removeDataOfTypes
in the Cocoa port.

* Source/WebKit/Headers.cmake:
* Source/WebKit/Shared/API/c/WKBase.h:
* Source/WebKit/Sources.txt:
* Source/WebKit/SourcesCocoa.txt:
* Source/WebKit/UIProcess/API/C/WKAPICast.cpp: Added.
(WebKit::toWebsiteDataTypes):
(WebKit::toAPI):
* Source/WebKit/UIProcess/API/C/WKAPICast.h:
* Source/WebKit/UIProcess/API/C/WKWebsiteDataRecordRef.cpp: Copied from Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingTreeIOS.h.
(WKWebsiteDataRecordGetTypeID):
(WKWebsiteDataRecordGetDisplayName):
(WKWebsiteDataRecordGetDataTypes):
* Source/WebKit/UIProcess/API/C/WKWebsiteDataRecordRef.h: Added.
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:
(WKWebsiteDataStoreRemoveITPDataForDomain):
(WKWebsiteDataStoreStatisticsClearThroughWebsiteDataRemoval):
(WKWebsiteDataStoreRemoveAllFetchCaches):
(WKWebsiteDataStoreRemoveNetworkCache):
(WKWebsiteDataStoreRemoveMemoryCaches):
(WKWebsiteDataStoreRemoveFetchCacheForOrigin):
(WKWebsiteDataStoreRemoveAllIndexedDatabases):
(WKWebsiteDataStoreRemoveLocalStorage):
(WKWebsiteDataStoreRemoveAllServiceWorkerRegistrations):
(WKWebsiteDataStoreClearPrivateClickMeasurementsThroughWebsiteDataRemoval):
(WKWebsiteDataStoreClearStorage):
(WKWebsiteDataStoreAllWebsiteDataTypes):
(WKWebsiteDataStoreRemoveDataOfTypes):
(WKWebsiteDataStoreRemoveDataOfTypesWithValues):
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h:
* Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingTreeIOS.cpp:
* Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingTreeIOS.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.h:
* Source/WebKit/WebKit.xcodeproj/project.pbxproj:
* Tools/MiniBrowser/win/WebKitBrowserWindow.cpp:
(removeDataOfTypesCallback):
(WebKitBrowserWindow::clearWebsiteData):
(removeAllFetchCachesCallback): Deleted.
(removeNetworkCacheCallback): Deleted.
(removeMemoryCachesCallback): Deleted.
(removeAllServiceWorkerRegistrationsCallback): Deleted.
(removeAllIndexedDatabasesCallback): Deleted.
(removeLocalStorageCallback): Deleted.
* Tools/WebKitTestRunner/TestController.cpp:
(WTR::TestController::clearServiceWorkerRegistrations):
(WTR::TestController::clearDOMCache):
(WTR::TestController::clearDOMCaches):
(WTR::TestController::clearMemoryCache):
(WTR::TestController::clearIndexedDatabases):
(WTR::TestController::clearLocalStorage):
(WTR::TestController::clearStorage):
(WTR::TestController::clearStatisticsDataForDomain):
(WTR::TestController::statisticsClearThroughWebsiteDataRemoval):
(WTR::TestController::clearPrivateClickMeasurementsThroughWebsiteDataRemoval):
* Tools/WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
@kshukuwa
Copy link
Contributor Author

kshukuwa commented Jun 6, 2023

@achristensen07
Could you please review it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merging-blocked Applied to prevent a change from being merged WebKit2 Bugs relating to the WebKit2 API layer
Projects
None yet
5 participants