[webkit-gtk] Block/Unblock plugin API

Thu Apr 9 03:38:44 PDT 2015

El jue, 19-03-2015 a las 10:52 +0100, Marcos Chavarría Teijeiro
escribió:
> I have been implementing this feature during the last month.

Sorry for the delay replying to this, but I've been quite busy with
releases and other bugs and I haven't had time to look at this in
detail.

>  The aproximation I took was to use a separate type for each signal.
> So the result was:
> 
> ->  WebKitPluginLoadPolicy (* load_plugin) (WebKitWebView *web_view,
> WebKitPluginLoadRequest *request);
> ->  void (* plugin_blocked) (WebKitWebView *web_view,
> WebKitPluginBlockedInformation *info);
> ->  void (* unavailable_plugin_activated) (WebKitWebView *web_view,
> WebKitPluginUnavailabilityInformation *info);
> 
> 
> The resulting path with the tests can be found here
> (https://bugs.webkit.org/attachment.cgi?id=248058&action=prettypatch).
> 
> 
> In the meanwhile the following patch has been landed  "Allow clients
> to selectively disable
> plug-ins" (https://bugs.webkit.org/show_bug.cgi?id=142506). This patch
> implements a way to set the load policy for each plugin and host. The
> defined policies are the following:
> 
> * PluginLoadClientPolicyUndefined -> No client-specific plug-in load
> policy has been defined. The plug-in should be visible in
> navigator.plugins and WebKit should synchronously ask the client
> whether the plug-in should be loaded.
> * PluginLoadClientPolicyBlock -> The plug-in module should be blocked
> from being instantiated. The plug-in should be hidden in
> navigator.plugins.
> * PluginLoadClientPolicyAsk -> WebKit should synchronously ask the
> client whether the plug-in should be loaded. The plug-in should be
> visible in navigator.plugins. 
> * PluginLoadClientPolicyAllow -> The plug-in module may be loaded if
> WebKit is not blocking it.
> * PluginLoadClientPolicyAllowAlways -> The plug-in module should be
> loaded irrespective of whether WebKit has asked it to be blocked.
> 
> 
> It implements the methods setPluginLoadClientPolicy and
> clearPluginClientPolicies on WebProcessPool to be able to set the
> plugin load client policy and to clear all existent policies.
> 
> 
> So, we should modify our block/unblock API to support this new client
> policies.

I don't think so . . . If I understood correctly the new policies patch,
there are two different things:

 - Blocking plugins: this is what the original patch was for. All
plugins are visible in DOM and loaded from the WebCore point of view.
When the plugin is loaded, the creation might fail, among other things,
because the plugin is blocked. In that case the unavailability message
is used and the user can click on it to request the load of the
individual plugin instance. This only affects the plugin instance, I
think, so if there are multiple plugins of the same type in the page,
you would need to unblock them individually. A use case for this would
be: a web page full of flash crap and ads, but with a valid flash video
you actually want to watch. You unblock the video, but keeping all other
flash things blocked.

 - Disabling plugins: this is what the new patch added to wk was for. In
this case, the plugins are not even visible in the DOM, and then never
loaded by WebCore, so they don't appear in the web page in any way. In
this case, a particular version of a particular plugin can be disabled
for a particular host, but all plugins of that type and version will be
disabled for the given host. In case of ask policy, a permission request
to load the plugin will be asked to the user, if user accepts, all
plugins will become visible, and I guess that will happen every time the
page is loaded. A use case for this would be: I never want to run any
flash crap by default (in any website or in a particular website I know
is annoying), or I know a particular version of flash is insecure, and I
want to make sure it's never used.

So, they are somehow related, but different things.

> In my opinion the only signal that should modify its behaviour is
> load-plugin. This signal only needs to be called if the ClientPolicies
> are Undefined or Ask.

I'm not sure, I don't think we want to ask the user for every plugin
instance, but only once when the page is loading.

>  In other cases we should return Block or Allow without asking to the
> client application. The other signal maintain their behaviour.
> 
> On the other hand we should add some API to allow the client to set
> this policies for each plugin and host. We could add three methods to
> Plugin class:
> 
> WebKitPluginLoadClientPolicy webkit_plugin_get_client_policy
> (WebKitPlugin* plugin, const gchar* host);
> This method allows the client to get the current policy for the given
> host.

I find the current WebCore and WebKit2 terminology quite confusing to be
honest, with PluginLoadClientPolicy and PluginModuleLoadPolicy. I would
not expose those names in the API.

> 
> void
>  webkit_plugin_set_client_policy
> (WebKitPlugin* plugin, const gchar* host, WebKitPluginLoadClientPolicy
> policy);
> This method allows the client to set the current policy for the given
> host.
> 
> GList
>  webkit_plugin_get_client_policy_hosts               (WebKitPlugin*
> plugin);
> 
> This method allows the client to get a list of host which have a
> custom policy.
> 

And the same for these methods. Also, I'm not sure WebKitPlugin is the
best place, maybe it should be in web context, passing the plugin you
want to set a policy for.

> 
> The WebKitPluginLoadClientPolicy enum contains the following values:
> 
> WEBKIT_PLUGIN_CLIENT_LOAD_POLICY_UNDEFINED

I think we should have a default one instead of undefined.

> WEBKIT_PLUGIN_CLIENT_LOAD_POLICY_BLOCK
> 
> WEBKIT_PLUGIN_CLIENT_LOAD_POLICY_ASK
> WEBKIT_PLUGIN_CLIENT_LOAD_POLICY_ALLOW
> WEBKIT_PLUGIN_CLIENT_LOAD_POLICY_ALLOW_ALWAYS
> 
> 
> In addition we could add a method to the WebContext class for been
> able to clear all plugin policies:
> void
> webkit_web_context_clear_plugin_client_policies (WebKitWebContext*
> context);
> 
> This method allows the client to clears all custom policies.

It's a bit weird that you set the policy in WebKitPlugin, but clear it
in WebKitWebContext, so the API to manage policies definitely fits
better in WebKitWebContext.

> 
> 
> I see a few problems with the proposed new API:
> 
> 
> * The only methods exposed in the WebProcessPool are
> setPluginLoadClientPolicy and clearPluginClientPolicies this methods
> could be used to implement webkit_plugin_set_client_policy and
> webkit_context_clear_plugin_client_policies but the remaining two API
> methods need that we implement some methods in the WebProcessPool
> class which should not be difficult but this patch has not implement
> this methods in spite of the fact that it do implement
> WebPlatformStrategies::pluginLoadClientPolicyForHost. This method does
> what we need for webkit_plugin_get_Client_policy_hosts but its not
> exposed in the C API. There could be some side effects that I dont
> see.
> 
> 
> * I really don't like the name for
> webkit_plugin_get_client_policy_hosts. 

Since they are different things in the end, I would forget about the
disabling policies, and focus again on blocking plugins API. 

Note that I haven't tried anything myself, this is my understanding just
reading the code, so I might be wrong in any of my assumptions. 

>  
> 2015-01-08 18:20 GMT+01:00 Gustavo Noronha Silva <gns at gnome.org>:
> >
> > On Tue, 2014-12-16 at 13:47 +0100, Carlos Garcia Campos wrote:
> > > I think this API provides all the flexibility that the internal or
> C API
> > > allows. There are a few things I'm not completely sure about:
> >
> > I agree, it looks sound to me.
> >
> > >   - WebKitPluginLoadRequest: do we really need the get_page_uri?
> > > wouldn't that be always the current web view uri? I've added the
> method
> > > because that information is provided by the loader client
> callback, but
> > > we need to make sure it's actually needed. For a first version we
> could
> > > avoid that method, and add it later if it's actually useful. But
> without
> > > method, the request is a very simple object, so maybe we could
> pass the
> > > WebKitPlugin directly to the signal and pass an out parameter for
> the
> > > button description. That would make everything simpler, but we
> won't be
> > > able to add more info to the signal without breaking the API.
> >
> > I would keep LoadRequest instead of passing the plugin directly, but
> I
> > also see no reason to provide get_page_uri at this point.
> >
> > >   - plugin-blocked: In this case I did the opposite, passing the
> > > information as signal arguments. Currently wk passes the frame
> URL, page
> > > URL, plugin mime type and whether the plugin is obscured. We might
> use a
> > > boxed type here too, for example to also provide the page uri and
> the
> > > plugin mime type if needed/useful.
> >
> > I think a separate type is better, for flexibility. I'd like to
> think we
> > would not need to add anything, but we can't be sure, so…
> >
> > >   - plugin-unavailability-button-clicked: the signal name and the
> boxed
> > > type used to provide the information are too long, we could think
> about
> > > better names. Again I'm not sure about the needed to provide the
> page
> > > uri. Also internally, wk passes the plugin page attribute (I guess
> this
> > > is the page where the plugin can be downloaded in case of missing
> > > plugins).
> >
> > unavailable-plugin-activated?
> >
> > I wouldn't mention a button on the API, in any case.
> >
> > Cheers,
> > --
> > Gustavo Noronha Silva <gns at gnome.org>
> > GNOME Project
> >
> > _______________________________________________
> > webkit-gtk mailing list
> > webkit-gtk at lists.webkit.org
> > https://lists.webkit.org/mailman/listinfo/webkit-gtk
> 
> 
> _______________________________________________
> webkit-gtk mailing list
> webkit-gtk at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-gtk

-- 
Carlos Garcia Campos
http://pgp.rediris.es:11371/pks/lookup?op=get&search=0xF3D322D0EC4582C3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part
URL: <https://lists.webkit.org/pipermail/webkit-gtk/attachments/20150409/3e0e4503/attachment.sig>