Bug 116020 – [GTK][WK2] Java applets remain visible even if you navigate to a different page

• Component: WebKitGTK
• Status: RESOLVED WORKSFORME

[Manuel Rego Casasnovas]

Steps to reproduce in MiniBrowser:
1) Go to http://lessonplans.btskinner.com/applet.html
2) Change the URL in MiniBrowser to http://www.webkit.org
3) You'll see that the applet is still visible and working in WebKit page.

BTW, you need to install "icedtea-plugin" in your machine in order to have the Java applets working.

[Manuel Rego Casasnovas]

Some more information about the bug.

In WebPageProxy::createPluginContainer() we connect a callback to "plug-removed" signal (https://developer.gnome.org/gtk3/stable/GtkSocket.html#GtkSocket-plug-removed). But this signal is never emitted.

I've been doing some tests and if I emit manually the signal from WebKitWebView::webkit_web_view_load_uri() the applet is still visible in the other page.

I had to destroy manually the applet to make it disappear.

I've added the following lines to WebKitWebView::webkit_web_view_load_uri() in order to do this quick hack:

    GList* items = gtk_container_get_children(GTK_CONTAINER(webView));
    GList* iter;

    for (iter = items; iter; iter = g_list_next(iter)) {
        GtkWidget* item = GTK_WIDGET(iter->data);

        if (GTK_IS_SOCKET(item)) {
            gboolean result;
            // With this the callback is called, but the applet is not destroyed.
            g_signal_emit_by_name(item, "plug-removed", &result);
            // This destroys the applet, gtk_container_remove() also works here.
            gtk_widget_destroy(item);
        }
    }

[Zan Dobersek]

Fun!

[Zan Dobersek]

Spent a good amount of time on this, no success yet.

When navigating away from the test page, I can see that the HTMLAppletElement is not getting detached from the DOM, and that's why its renderer isn't getting destroyed. HTMLAppletElement inherits from HTMLPlugInImageElement, which inherist from HTMLPluginElement, which inherist from HTMLElement and so on to the Node class.

The renderer is a RenderApplet, which inherits all the way from RenderWidget, which holds a reference to the Widget that it renders, the Widget being the PluginView that's created in the WebFrameLoaderClient::createPlugin method (called from SubframeLoader::loadPlugin).

Continuing on the note of HTMLAppletElement not detaching, the node normally detaches (and has its render destroyed) in its deconstructor. Since this is not happening in WK2, I think there's a reference of the HTMLAppletElement object being kept somewhere, preventing for the element to be destroyed. Not 100% sure about this, but it certainly seems so to me given that in WK1, the HTML element is destroyed first, with the renderer destroyed second and the PluginView following immediately after.

The PluginView class actually holds a RefPtr for the HTMLPluginElement, the very element for which the plugin is loaded. Replacing the RefPtr with a raw pointer to the HTMLPluginElement object doesn't help, the problem persists, though it's kind of unusual to do so, especially given that this is not done in the WK1 layer (in Source/WebCore/plugins/PluginView.cpp).

[Zan Dobersek]

The HTMLPlugInElement not being detached is due to the page cache enabled when using the MiniBrowser. Specifically, the whole DOM document is not detached & destroyed  (initiaded at [1]) if the page cache is in use. As proof, with the page cache disabled, the Java applet renderer is properly destroyed when navigating away from the test page.

Still, even with the page cache enabled, the plugin renderer should be removed.
When the FrameView is being replaced in Frame::createView (which happens when navigating to another location), the current FrameView's parent is set to not visible[2]. The FrameView inherits from the ScrollView, meaning that ScrollView::setParentVisible[3] is called. That method calls the same method on every child widget, which, on the test page, is also the WebKit2 PluginView[4], the Widget-implementing object that controls the plugin. The PluginView doesn't doesn't implement the setParentVisible function, but when added, it does get called with the 'visible' argument being false. Still, that has no effect on hiding the widget when navigating away (which of course doesn't happen when it should).

[1] http://trac.webkit.org/browser/trunk/Source/WebCore/page/Frame.cpp#L262
[2] http://trac.webkit.org/browser/trunk/Source/WebCore/page/Frame.cpp#L772
[3] http://trac.webkit.org/browser/trunk/Source/WebCore/platform/ScrollView.cpp#L1249
[4] http://trac.webkit.org/browser/trunk/Source/WebKit2/WebProcess/Plugins/PluginView.h

[Zan Dobersek]

The main problem here seems to be that the pages containing the plugin are still stored into the page cache when navigating away from them. For the GTK port, I don't believe that's a good idea.

For instance, when leaving Youtube with a video playing, the Flash plugin would also be stored in the page cache had it not been for unload event listeners on the page that actually prevent the page from entering the cache[1]. Similarly with the pages containing Java applets, the applet's PluginView is kept as is instead of being destroyed, meaning that the windowed plugin is both not hidden and is kept alive and functioning - even if it would be hidden, it would most likely continue to run in the background (note that I haven't really tested that, but can if someone explicitly requests for it) which I believe is not expected.

The pages that contain plugins can be prevented from entering the cache by adjusting the according setting[2] which defaults to false. In WebKit1GTK+ the setting was not adjusted at all (meaning its value was kept at false), while in WebKit2GTK+ the setting defaults to true[3].

I'd recommend to use false as the default for this setting in WebKit2GTK+. If approved, the patch is pretty straight-forward.

[1] http://trac.webkit.org/browser/trunk/Source/WebCore/history/PageCache.cpp#L305
[2] http://trac.webkit.org/browser/trunk/Source/WebCore/page/Settings.in#L72
[3] http://trac.webkit.org/browser/trunk/Source/WebKit2/Shared/WebPreferencesStore.h#L105

[Gustavo Noronha (kov)]

It makes sense to me. On Mac all plugins are windowless AFAIK. I remember talking about this with Martin at some point and he mentioned forcing all plugins to go windowless for wk2gtk, was that not possible after all or was it decided against?

[Martin Robinson]

(In reply to comment #6)
> It makes sense to me. On Mac all plugins are windowless AFAIK. I remember talking about this with Martin at some point and he mentioned forcing all plugins to go windowless for wk2gtk, was that not possible after all or was it decided against?

Hrm. I don't remember the specifics, but we support windowed plugins now.

[Zan Dobersek]

Created attachment 202127 [details]
Patch

[Martin Robinson]

Comment on attachment 202127 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=202127&action=review

Looks okay, but needs a WebKit2 owner to review it formally.

> Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:197
> +    g_signal_connect(socket, "plug-removed", G_CALLBACK(socketPlugRemovedCallback), m_platformPluginWidget);

Are you sure that removing the widget doesn't destroy it as a side-effect of decreasing the reference count?

[Zan Dobersek]

Comment on attachment 202127 [details]
Patch

Removing it from where exactly?

This callback gets fired when the windowed plugin is destroyed/stopped, so the intermediate GtkSocket (the one created on the previous line) has this signal emitted and the callback is removed.

This GtkSocket is inside the GtkPlug that's belonging to the GtkSocket that was created in the UIProcess. That GtkSocket does not get removed until its GtkPlug is removed.

Not destroying the parent plug of the windowed widget's socket leaves a gray area where the UIProcess socket is displaying the contents of the now-defunct parent plug.

[Zan Dobersek]

Benjamin, Andreas -- could one of you please take a look at this one?

[Anders Carlsson]

Comment on attachment 202127 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=202127&action=review

> Source/WebKit2/Shared/WebPreferencesStore.h:107
> -    macro(PageCacheSupportsPlugins, pageCacheSupportsPlugins, Bool, bool, true) \
> +    macro(PageCacheSupportsPlugins, pageCacheSupportsPlugins, Bool, bool, DEFAULT_PAGE_CACHE_SUPPORTS_PLUGINS_ENABLED) \

I don't understand why this change is needed.

[Zan Dobersek]

Comment on attachment 202127 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=202127&action=review

>> Source/WebKit2/Shared/WebPreferencesStore.h:107
>> +    macro(PageCacheSupportsPlugins, pageCacheSupportsPlugins, Bool, bool, DEFAULT_PAGE_CACHE_SUPPORTS_PLUGINS_ENABLED) \
> 
> I don't understand why this change is needed.

The GTK port still supports windowed Netscape plugins. When entered into the page cache, their PluginView is not destroyed, so these plugins continue to run as if nothing happened. Because they are windowed, their display window does not get hidden or destroyed, so they are still displayed when navigating away from the page that contained them. Disabling this setting for the GTK port fixes this without any unnecessary hacks that would be required in the PluginView and would extend all the way into the plugin process.

This setting was also disabled for the WebKit1 GTK port, so this wasn't happening there. Furthermore, of the WebKit1 layer ports, only the Mac port was enabling this setting (or at least exposing the enabling and disabling mechanism), meaning that Qt and EFL ports of the WK2 layer may now be exposed to the very same problem (if they support windowed plugins).

[Zan Dobersek]

Any remaining objections from WK2 owner(s)?

[Martin Robinson]

Interested in pushing this fix still?

[Michael Catanzaro]

Zan, is this patch still needed?

[Zan Dobersek]

The patch in this form doesn't apply anymore, and I'm not aware of Java applets still being a problem.

[Michael Catanzaro]

Comment on attachment 202127 [details]
Patch

Removing from request queue then, and closing the bug... please reopen if this is discovered to still be a problem.