[webkit-reviews] review denied: [Bug 96497] [GTK] Add API to get/set the security policy of a given URI scheme to WebKit2 GTK+ : [Attachment 163597] Try to fix the mac build

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 96497: [GTK] Add API to get/set the security policy of a given URI scheme
to WebKit2 GTK+
https://bugs.webkit.org/show_bug.cgi?id=96497

Attachment 163597: Try to fix the mac build
https://bugs.webkit.org/attachment.cgi?id=163597&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=163597&action=review


> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:648
> +    // We keep the WebCore::SchemeRegistry of the UI process in sync with
the
> +    // web process one, so that we can return the WebKitSecurityPolicy for
> +    // a given URI scheme synchronously without blocking.
> +    if (policy & WEBKIT_SECURITY_POLICY_LOCAL) {
> +	   WebCore::SchemeRegistry::registerURLSchemeAsLocal(urlScheme);
> +	   webContext->registerURLSchemeAsLocal(urlScheme);
> +    }
> +    if (policy & WEBKIT_SECURITY_POLICY_NO_ACCESS_TO_OTHER_SCHEME) {
> +	   WebCore::SchemeRegistry::registerURLSchemeAsNoAccess(urlScheme);
> +	   webContext->registerURLSchemeAsNoAccess(urlScheme);
> +    }
> +    if (policy & WEBKIT_SECURITY_POLICY_DISPLAY_ISOLATED) {
> +	  
WebCore::SchemeRegistry::registerURLSchemeAsDisplayIsolated(urlScheme);
> +	   webContext->registerURLSchemeAsDisplayIsolated(urlScheme);
> +    }
> +    if (policy & WEBKIT_SECURITY_POLICY_SECURE) {
> +	   WebCore::SchemeRegistry::registerURLSchemeAsSecure(urlScheme);
> +	   webContext->registerURLSchemeAsSecure(urlScheme);
> +    }
> +    if (policy & WEBKIT_SECURITY_POLICY_CORS_ENABLED) {
> +	   WebCore::SchemeRegistry::registerURLSchemeAsCORSEnabled(urlScheme);
> +	   webContext->registerURLSchemeAsCORSEnabled(urlScheme);
> +    }
> +    if (policy & WEBKIT_SECURITY_POLICY_EMPTY_DOCUMENT) {
> +	  
WebCore::SchemeRegistry::registerURLSchemeAsEmptyDocument(urlScheme);
> +	   webContext->registerURLSchemeAsEmptyDocument(urlScheme);
> +    }
> +}
> +

One thing I notice here is that if you say, activate
WEBKIT_SECURITY_POLICY_LOCAL, and then call this method again with the same
scheme without WEBKIT_SECURITY_POLICY_LOCAL removeURLSchemeRegisteredAsLocal is
never called. It seems that you can never undo any of the actions you take. On
the other hand there doesn't seem to be a way in WebCore to undo registration
for all other types of schemes. Perhaps that means this API doesn't map to the
one in WebCore. Perhaps you need to add a method in WebCore to remove all
registrations for a scheme and call that first here.