...
Created attachment 447390 [details] Patch
Created attachment 447392 [details] Patch
What's the use case where the directory already exists but is not excluded? Can we add a test for that case?
(In reply to Geoffrey Garen from comment #3) > What's the use case where the directory already exists but is not excluded? > Can we add a test for that case? We currently create storage directories when we decide the location, for example see WebsiteDataStore::defaultLocalStorageDirectory() and WebsiteDataStore::websiteDataDirectoryFileSystemRepresentation in WebsiteDataStoreCocoa.mm. That means when we call excludeDirectoryFromBackup, the directory should already exist (WebsiteDataStore parameters should be decided before that). I can try creating an API test.
Created attachment 447459 [details] Patch
Created attachment 447618 [details] Patch
Comment on attachment 447618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447618&action=review > Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:649 > + if (![url setResourceValue:@YES forKey:NSURLIsExcludedFromBackupKey error:nil]) It would be good to log error. Maybe write something like [url setResourceValue:@YES forKey:NSURLIsExcludedFromBackupKey error:error]; RELEASE_LOG_ERROR_IF(error, Storage, ... +log error). > Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm:463 > + [[NSFileManager defaultManager] removeItemAtPath:url.absoluteString error:nil]; Should we create the directory without the key to make sure the new code path makes it working?
Comment on attachment 447618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447618&action=review >> Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:649 >> + if (![url setResourceValue:@YES forKey:NSURLIsExcludedFromBackupKey error:nil]) > > It would be good to log error. > Maybe write something like > [url setResourceValue:@YES forKey:NSURLIsExcludedFromBackupKey error:error]; > RELEASE_LOG_ERROR_IF(error, Storage, ... +log error). Sure, will add. >> Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm:463 >> + [[NSFileManager defaultManager] removeItemAtPath:url.absoluteString error:nil]; > > Should we create the directory without the key to make sure the new code path makes it working? Sure. Will add the test.
Created attachment 447825 [details] Patch
Committed r287377 (245517@main): <https://commits.webkit.org/245517@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447825 [details].
<rdar://problem/86829012>