.
<rdar://85277115>
Created attachment 444001 [details] Patch v1.0
Created attachment 444008 [details] Patch v1.1
Comment on attachment 444008 [details] Patch v1.1 View in context: https://bugs.webkit.org/attachment.cgi?id=444008&action=review > Source/WebKit/UIProcess/Inspector/WebInspectorUtilities.h:48 > +String bundleIdentifierForSandboxBroker(); I think this should return a CFStringRef. Also, could return ASCIILiteral if you really want to stick with WTF instead of platform-specific.
Comment on attachment 444008 [details] Patch v1.1 View in context: https://bugs.webkit.org/attachment.cgi?id=444008&action=review > Source/WebKit/UIProcess/Inspector/WebInspectorUtilities.cpp:119 > + auto sandboxBrokerBundleIdentifier = "com.apple.Safari.SandboxBroker"_s; Nit: Now that this logic is in a standalone function, can we just return the correct string inside the conditionals instead of setting a local variable?
Created attachment 444296 [details] Patch v1.2
Comment on attachment 444296 [details] Patch v1.2 View in context: https://bugs.webkit.org/attachment.cgi?id=444296&action=review > Source/WebKit/UIProcess/Cocoa/WebInspectorPreferenceObserver.mm:53 > + String sandboxBrokerBundleIdentifier = WebKit::bundleIdentifierForSandboxBroker(); I suggest not involving WTF::String at all: auto sandboxBrokerBundleIdentifier = WebKit::bundleIdentifierForSandboxBroker(); m_userDefaults = adoptNS([[NSUserDefaults alloc] initWithSuiteName:bridge_cast(sandboxBrokerBundleIdentifier)]); if (!m_userDefaults) { WTFLogAlways("Could not init user defaults instance for domain %@.", sandboxBrokerBundleIdentifier);
Comment on attachment 444296 [details] Patch v1.2 View in context: https://bugs.webkit.org/attachment.cgi?id=444296&action=review > Source/WebKit/UIProcess/Inspector/WebInspectorUtilities.cpp:122 > + if (WebCore::applicationBundleIdentifier() == "com.apple.SafariTechnologyPreview"_s) > + return CFSTR("com.apple.SafariTechnologyPreview.SandboxBroker"); > + if (WebCore::applicationBundleIdentifier() == "com.apple.Safari.automation"_s) > + return CFSTR("com.apple.Safari.automation.SandboxBroker"); One other thought: Since bundle identifiers are not case sensitive, it might be safer to call equalLettersIgnoringASCIICase instead of using ==.
Created attachment 445440 [details] Patch for Landing
Comment on attachment 445440 [details] Patch for Landing View in context: https://bugs.webkit.org/attachment.cgi?id=445440&action=review > Source/WebKit/UIProcess/Inspector/WebInspectorUtilities.cpp:119 > + if (WebCore::applicationBundleIdentifier() == "com.apple.SafariTechnologyPreview"_s) Perhaps avoid calling `WebCore::applicationBundleIdentifier` twice with a local? Is the result always `applicationBundleIdentifier() + ".SandboxBroker"?
Committed r286314 (244673@main): <https://commits.webkit.org/244673@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445440 [details].