116020 2013-05-13 03:54:00 -0700 [GTK][WK2] Java applets remain visible even if you navigate to a different page 2016-01-03 08:16:13 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) Unspecified Unspecified RESOLVED WORKSFORME P2 Normal --- 1 rego zan benjamin cgarcia csaavedra gustavo kling mcatanzaro mrobinson zan oldest_to_newest 888181 0 rego 2013-05-13 03:54:00 -0700 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. 888185 1 rego 2013-05-13 04:02:30 -0700 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); } } 888244 2 zan 2013-05-13 07:05:56 -0700 Fun! 888510 3 zan 2013-05-13 13:50:56 -0700 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). 888822 4 zan 2013-05-14 07:19:12 -0700 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 889803 5 zan 2013-05-16 07:10:55 -0700 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 889887 6 gustavo 2013-05-16 10:11:17 -0700 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? 889891 7 mrobinson 2013-05-16 10:28:30 -0700 (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. 890545 8 202127 zan 2013-05-17 11:38:26 -0700 Created attachment 202127 Patch 890548 9 202127 mrobinson 2013-05-17 11:40:32 -0700 Comment on attachment 202127 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? 890557 10 202127 zan 2013-05-17 11:48:46 -0700 Comment on attachment 202127 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. 891006 11 zan 2013-05-19 00:13:27 -0700 Benjamin, Andreas -- could one of you please take a look at this one? 891071 12 202127 andersca 2013-05-19 15:41:02 -0700 Comment on attachment 202127 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. 891122 13 202127 zan 2013-05-19 23:10:37 -0700 Comment on attachment 202127 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). 899717 14 zan 2013-06-12 12:07:43 -0700 Any remaining objections from WK2 owner(s)? 1049968 15 mrobinson 2014-11-19 15:59:00 -0800 Interested in pushing this fix still? 1150882 16 mcatanzaro 2015-12-22 06:56:17 -0800 Zan, is this patch still needed? 1152095 17 zan 2016-01-03 01:58:40 -0800 The patch in this form doesn't apply anymore, and I'm not aware of Java applets still being a problem. 1152105 18 202127 mcatanzaro 2016-01-03 08:15:47 -0800 Comment on attachment 202127 Patch Removing from request queue then, and closing the bug... please reopen if this is discovered to still be a problem. 202127 2013-05-17 11:38:26 -0700 2016-01-03 08:15:47 -0800 Patch bug-116020-20130517203719.patch text/plain 4530 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTUwMjYwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggYjI0YTg3MjBhMTU1NGM3 ZWUzNzQxMmQyMTlhZTkxODlkYWMxZjNhNi4uOGE3NWZiNjJlZjQ5YWUzNzMyZDYxNWQ4OWE1NjUy NTFkZGQxMmE2YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDEzLTA1LTE3ICBaYW4g RG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1RLXVtXSzJdIEph dmEgYXBwbGV0cyByZW1haW4gdmlzaWJsZSBldmVuIGlmIHlvdSBuYXZpZ2F0ZSB0byBhIGRpZmZl cmVudCBwYWdlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xMTYwMjAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBUaGUgR1RLIHBvcnQgc3RpbGwgc3VwcG9ydHMgdGhlIHdpbmRvd2VkIE5ldHNjYXBlIHBsdWdp bnMuIEJlY2F1c2Ugb2YgdGhhdCBpdCBzaG91bGQsIG11Y2ggbGlrZSBpbiB0aGUKKyAgICAgICAg R1RLIHBvcnQgb2YgV2ViS2l0MSwgZGVmYXVsdCB0byBub3QgY2FjaGluZyB3ZWIgcGFnZXMgdGhh dCBjb250YWluIHBsdWdpbnMuCisKKyAgICAgICAgKiBTaGFyZWQvV2ViUHJlZmVyZW5jZXNTdG9y ZS5oOgorICAgICAgICAoV2ViS2l0KTogRG8gbm90IGRlZmF1bHQgdG8gY2FjaGluZyBwYWdlcyB0 aGF0IGNvbnRhaW4gcGx1Z2lucyBmb3IgdGhlIEdUSyBwb3J0LgorICAgICAgICAqIFdlYlByb2Nl c3MvUGx1Z2lucy9OZXRzY2FwZS94MTEvTmV0c2NhcGVQbHVnaW5YMTEuY3BwOgorICAgICAgICAo V2ViS2l0Ojpzb2NrZXRQbHVnUmVtb3ZlZENhbGxiYWNrKTogV2hlbiB0aGUgd2luZG93ZWQgcGx1 Z2luJ3MgR3RrU29ja2V0IGhhcyBpdHMgR3RrUGx1ZyByZW1vdmVkLCB0aGUgR3RrUGx1ZyBiZWxv bmdpbmcKKyAgICAgICAgdG8gdGhlIFVJUHJvY2VzcydzIEd0a1NvY2tldCAodGhhdCdzIGNyZWF0 ZWQgaW4gV2ViUGFnZVByb3h5OjpjcmVhdGVQbHVnaW5Db250YWluZXIpIHNob3VsZCBiZSBkZXN0 cm95ZWQgYXMgd2VsbC4KKyAgICAgICAgKFdlYktpdDo6TmV0c2NhcGVQbHVnaW46OnBsYXRmb3Jt UG9zdEluaXRpYWxpemVXaW5kb3dlZCk6IFBhc3MgdGhlIHBvaW50ZXIgdG8gdGhlIFVJUHJvY2Vz cydzIEd0a1NvY2tldCB0byB0aGUgY2FsbGJhY2sgdGhhdCdsbAorICAgICAgICBiZSBpbnZva2Vk IG9uIHRoZSBuZXdseSBjcmVhdGVkIEd0a1NvY2tldCB3aGVuIGl0cyBHdGtQbHVnIGlzIHJlbW92 ZWQgKHdoaWNoIHNob3VsZCBvY2N1ciB3aGVuIHRoZSB3aW5kb3dlZCBwbHVnaW4gc3RvcHMpLgor CiAyMDEzLTA1LTE3ICBNaWNoYcWCIFBha3XFgmEgdmVsIFJ1dGthICA8bS5wYWt1bGFAc2Ftc3Vu Zy5jb20+CiAKICAgICAgICAgW0VGTF1bV0syXSBTdXBwb3J0IHN1YiBtZW51IGluIGV3ayBjb250 ZXh0IG1lbnVzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9TaGFyZWQvV2ViUHJlZmVyZW5j ZXNTdG9yZS5oIGIvU291cmNlL1dlYktpdDIvU2hhcmVkL1dlYlByZWZlcmVuY2VzU3RvcmUuaApp bmRleCAzZjJkNzUxYTFkNmIzNWJjOGEyODUyZTcwYmQxMGMxYzkzODlkNjFiLi5mOTliNmExZDQz OTk1M2VkZjZmYThmMTgzYjBhZmUzMTY2NDlhYTZmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9TaGFyZWQvV2ViUHJlZmVyZW5jZXNTdG9yZS5oCisrKyBiL1NvdXJjZS9XZWJLaXQyL1NoYXJl ZC9XZWJQcmVmZXJlbmNlc1N0b3JlLmgKQEAgLTM4LDggKzM4LDEwIEBAIG5hbWVzcGFjZSBXZWJL aXQgewogCiAjaWYgUExBVEZPUk0oR1RLKQogI2RlZmluZSBERUZBVUxUX1dFQktJVF9UQUJTVE9M SU5LU19FTkFCTEVEIHRydWUKKyNkZWZpbmUgREVGQVVMVF9QQUdFX0NBQ0hFX1NVUFBPUlRTX1BM VUdJTlNfRU5BQkxFRCBmYWxzZQogI2Vsc2UKICNkZWZpbmUgREVGQVVMVF9XRUJLSVRfVEFCU1RP TElOS1NfRU5BQkxFRCBmYWxzZQorI2RlZmluZSBERUZBVUxUX1BBR0VfQ0FDSEVfU1VQUE9SVFNf UExVR0lOU19FTkFCTEVEIHRydWUKICNlbmRpZgogCiAjaWYgRU5BQkxFKFNNT09USF9TQ1JPTExJ TkcpICYmICFQTEFURk9STShRVCkKQEAgLTEwMiw3ICsxMDQsNyBAQCBuYW1lc3BhY2UgV2ViS2l0 IHsKICAgICBtYWNybyhXZWJBcmNoaXZlRGVidWdNb2RlRW5hYmxlZCwgd2ViQXJjaGl2ZURlYnVn TW9kZUVuYWJsZWQsIEJvb2wsIGJvb2wsIGZhbHNlKSBcCiAgICAgbWFjcm8oTG9jYWxGaWxlQ29u dGVudFNuaWZmaW5nRW5hYmxlZCwgbG9jYWxGaWxlQ29udGVudFNuaWZmaW5nRW5hYmxlZCwgQm9v bCwgYm9vbCwgZmFsc2UpIFwKICAgICBtYWNybyhVc2VzUGFnZUNhY2hlLCB1c2VzUGFnZUNhY2hl LCBCb29sLCBib29sLCB0cnVlKSBcCi0gICAgbWFjcm8oUGFnZUNhY2hlU3VwcG9ydHNQbHVnaW5z LCBwYWdlQ2FjaGVTdXBwb3J0c1BsdWdpbnMsIEJvb2wsIGJvb2wsIHRydWUpIFwKKyAgICBtYWNy byhQYWdlQ2FjaGVTdXBwb3J0c1BsdWdpbnMsIHBhZ2VDYWNoZVN1cHBvcnRzUGx1Z2lucywgQm9v bCwgYm9vbCwgREVGQVVMVF9QQUdFX0NBQ0hFX1NVUFBPUlRTX1BMVUdJTlNfRU5BQkxFRCkgXAog ICAgIG1hY3JvKEF1dGhvckFuZFVzZXJTdHlsZXNFbmFibGVkLCBhdXRob3JBbmRVc2VyU3R5bGVz RW5hYmxlZCwgQm9vbCwgYm9vbCwgdHJ1ZSkgXAogICAgIG1hY3JvKFBhZ2luYXRlRHVyaW5nTGF5 b3V0RW5hYmxlZCwgcGFnaW5hdGVEdXJpbmdMYXlvdXRFbmFibGVkLCBCb29sLCBib29sLCBmYWxz ZSkgXAogICAgIG1hY3JvKERPTVBhc3RlQWxsb3dlZCwgZG9tUGFzdGVBbGxvd2VkLCBCb29sLCBi b29sLCBmYWxzZSkgXApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9QbHVn aW5zL05ldHNjYXBlL3gxMS9OZXRzY2FwZVBsdWdpblgxMS5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9X ZWJQcm9jZXNzL1BsdWdpbnMvTmV0c2NhcGUveDExL05ldHNjYXBlUGx1Z2luWDExLmNwcAppbmRl eCBkNzM0ZmY2ODQzMWE4NzMyYzA2OWRiMjgxNTg4M2FjMTI4N2M2OTAxLi5mN2JhOWMwYzk3MDUz NzZmMzIzMDZhMGJiZDA4MjVmYzJkODAyN2Q2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9X ZWJQcm9jZXNzL1BsdWdpbnMvTmV0c2NhcGUveDExL05ldHNjYXBlUGx1Z2luWDExLmNwcAorKysg Yi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1BsdWdpbnMvTmV0c2NhcGUveDExL05ldHNjYXBl UGx1Z2luWDExLmNwcApAQCAtMTY1LDggKzE2NSwxMSBAQCBEaXNwbGF5KiBOZXRzY2FwZVBsdWdp bjo6eDExSG9zdERpc3BsYXkoKQogfQogCiAjaWYgUExBVEZPUk0oR1RLKQotc3RhdGljIGdib29s ZWFuIHNvY2tldFBsdWdSZW1vdmVkQ2FsbGJhY2soR3RrU29ja2V0KikKK3N0YXRpYyBnYm9vbGVh biBzb2NrZXRQbHVnUmVtb3ZlZENhbGxiYWNrKEd0a1NvY2tldCosIEd0a1dpZGdldCogcGFyZW50 UGx1ZykKIHsKKyAgICAvLyBUaGUgcGFyZW50IEd0a1BsdWcgaXMgbm90IHJlcXVpcmVkIGFueW1v cmUsIHNvIGl0IG11c3QgYmUgZGVzdHJveWVkLgorICAgIGd0a193aWRnZXRfZGVzdHJveShwYXJl bnRQbHVnKTsKKwogICAgIC8vIERlZmF1bHQgYWN0aW9uIGlzIHRvIGRlc3Ryb3kgdGhlIEd0a1Nv Y2tldCwgc28gd2UganVzdCByZXR1cm4gVFJVRSBoZXJlCiAgICAgLy8gdG8gYmUgYWJsZSB0byBy ZXVzZSB0aGUgc29ja2V0LiBGb3Igc29tZSBvYnNjdXJlIHJlYXNvbiwgbmV3ZXIgdmVyc2lvbnMK ICAgICAvLyBvZiBmbGFzaCBwbHVnaW4gcmVtb3ZlIHRoZSBwbHVnIGZyb20gdGhlIHNvY2tldCwg cHJvYmFibHkgYmVjYXVzZSB0aGUgcGx1ZwpAQCAtMTkxLDcgKzE5NCw3IEBAIGJvb2wgTmV0c2Nh cGVQbHVnaW46OnBsYXRmb3JtUG9zdEluaXRpYWxpemVXaW5kb3dlZChib29sIG5lZWRzWEVtYmVk LCB1aW50NjRfdCB3CiAgICAgLy8gY29udGFpbmluZyBhIHBsdWcgd2l0aCB0aGUgVUkgcHJvY2Vz cyBzb2NrZXQgZW1iZWRkZWQuCiAgICAgbV9wbGF0Zm9ybVBsdWdpbldpZGdldCA9IGd0a19wbHVn X25ldyhzdGF0aWNfY2FzdDxXaW5kb3c+KHdpbmRvd0lEKSk7CiAgICAgR3RrV2lkZ2V0KiBzb2Nr ZXQgPSBndGtfc29ja2V0X25ldygpOwotICAgIGdfc2lnbmFsX2Nvbm5lY3Qoc29ja2V0LCAicGx1 Zy1yZW1vdmVkIiwgR19DQUxMQkFDSyhzb2NrZXRQbHVnUmVtb3ZlZENhbGxiYWNrKSwgMCk7Cisg ICAgZ19zaWduYWxfY29ubmVjdChzb2NrZXQsICJwbHVnLXJlbW92ZWQiLCBHX0NBTExCQUNLKHNv Y2tldFBsdWdSZW1vdmVkQ2FsbGJhY2spLCBtX3BsYXRmb3JtUGx1Z2luV2lkZ2V0KTsKICAgICBn dGtfY29udGFpbmVyX2FkZChHVEtfQ09OVEFJTkVSKG1fcGxhdGZvcm1QbHVnaW5XaWRnZXQpLCBz b2NrZXQpOwogICAgIGd0a193aWRnZXRfc2hvdyhzb2NrZXQpOwogICAgIGd0a193aWRnZXRfc2hv dyhtX3BsYXRmb3JtUGx1Z2luV2lkZ2V0KTsK