[webkit-reviews] review denied: [Bug 209016] [ iOS and Mac wk2 ] http/tests/in-app-browser-privacy/ tests failing. : [Attachment 393519] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Mar 13 13:05:19 PDT 2020
Chris Dumez <cdumez at apple.com> has denied katherine_cheney at apple.com's request
for review:
Bug 209016: [ iOS and Mac wk2 ] http/tests/in-app-browser-privacy/ tests
failing.
https://bugs.webkit.org/show_bug.cgi?id=209016
Attachment 393519: Patch
https://bugs.webkit.org/attachment.cgi?id=393519&action=review
--- Comment #7 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 393519
--> https://bugs.webkit.org/attachment.cgi?id=393519
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=393519&action=review
> Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:527
> + hasInitializedAppBoundDomains = false;
I think that for this code to actually be correct, you'd have to reset
hasInitializedAppBoundDomains to false in the lambda inside
clearAppBoundDomains(), after clearing the domains. That said, I think we can
simplify this code a lot like by tweaking existing code like so:
enum class ForceReinitialization : bool { No, Yes };
void WebsiteDataStore::initializeAppBoundDomains(ForceReinitialization
forceReinitialization)
{
ASSERT(RunLoop::isMain());
if (hasInitializedAppBoundDomains && forceReinitialization !=
ForceReinitialization::Yes) // Updated
return;
static const auto maxAppBoundDomainCount = 10;
appBoundDomainQueue().dispatch([forceReinitialization] () mutable {
if (hasInitializedAppBoundDomains && forceReinitialization !=
ForceReinitialization::Yes) // Updated
return;
NSArray<NSString *> *domains = [[NSBundle mainBundle]
objectForInfoDictionaryKey:@"WKAppBoundDomains"];
RunLoop::main().dispatch([forceReinitialization , domains =
retainPtr(domains)] {
if ( forceReinitialization == ForceReinitialization::Yes) // New
appBoundDomains().clear(); // New
for (NSString *domain in domains.get()) {
URL url { URL(), domain };
if (!url.isValid())
continue;
WebCore::RegistrableDomain appBoundDomain { url };
if (appBoundDomain.isEmpty())
continue;
appBoundDomains().add(appBoundDomain);
if (appBoundDomains().size() >= maxAppBoundDomainCount)
break;
}
WEBSITE_DATA_STORE_ADDITIONS
hasInitializedAppBoundDomains = true;
});
});
}
Then you only have to call
initializeAppBoundDomains(ForceReinitialization::Yes) in your new
reinitializeAppBoundDomains() method.
What do you think?
More information about the webkit-reviews
mailing list