We should add some logging to gather telemetry on script injection for app-bound domains.
<rdar://problem/70111380>
Created attachment 411152 [details] Patch
Created attachment 411155 [details] Patch
Created attachment 411171 [details] Patch
Comment on attachment 411171 [details] Patch I don't think it's necessary to land this in OpenSource.
Comment on attachment 411171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411171&action=review > Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp:223 > + for (auto& process : m_processes) { > +#if ENABLE(APP_BOUND_DOMAINS) > + if (appBoundDomainsLoggingEnabled) > + logInBatches(process, userScript.userScript().source()); > +#endif > process.send(Messages::WebUserContentController::AddUserScripts({ { userScript.identifier(), world->identifier(), userScript.userScript() } }, immediately), identifier()); > + } Sounds like this might not actually need to get landed, but none-the-less, if you were going to land something like this, I would recommend going with something like this: static void logScriptIfAppropriate(const WeakHashSet<WebProcessProxy>& processes, StringView userScript) { #if ENABLE(APP_BOUND_DOMAINS) Boolean keyExistsAndHasValidFormat = false; if (!CFPreferencesGetAppBooleanValue(CFSTR("LogAppBoundDomains"), kCFPreferencesCurrentApplication, &keyExistsAndHasValidFormat)) return; bool anyProcessHasAlwaysOnLoggingAllowed = false; for (auto& process : m_processes) { if (process.websiteDataStore().sessionID().isAlwaysOnLoggingAllowed()) anyProcessHasAlwaysOnLoggingAllowed = true; break; } } if (!anyProcessHasAlwaysOnLoggingAllowed) return; static constexpr unsigned maxIndividualLogLength = 950; unsigned totalBatches = userScript.length() / maxIndividualLogLength; for (unsigned currentBatch = 0; currentBatch < totalBatches; ++currentBatch) auto toLog = userScript.substring(maxIndividualLogLength * currentBatch, maxIndividualLogLength); RELEASE_LOG(AppBoundDomains, "(%d/%d) %.*s", currentBatch + 1, totalBatches + 1, maxIndividualLogLength, toLog.utf8().data()); } #endif } ... void WebUserContentControllerProxy::addUserScript(API::UserScript& userScript, InjectUserScriptImmediately immediately) { logScriptIfAppropriate(m_processes, userScript.userScript().source()); ... (not tested, but something along this lines). This uses avoids potentially logging the same script multiple times if the multiple processes have isAlwaysOnLoggingAllowed enabled for their associated data stores, uses StringView to avoid unnecessary copies of the UserScript's source, and partitions the code in a single function, to be minimally invasive to the existing code.
Thanks for the comments! If I post another patch, I'll align it with your suggestions Sam, it has a lot of improvements.