Bug 74409 – [GTK] plugins/netscape-plugin-page-cache-works.html fails

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Philippe Normand]

Patch from bug 13634 introduced this test. It seems we'd need a new WebSetting for pageCacheSupportsPlugins, at least.
I'll skip the test for now.

[Zan Dobersek]

Also affected tests:
media/crash-closing-page-with-media-as-plugin-fallback.htmlplugins/crash-restoring-plugin-page-from-page-cache.html

[Zan Dobersek]

Created attachment 129866 [details]
Patch

[Zan Dobersek]

(In reply to comment #2)
> Created an attachment (id=129866) [details]
> Patch

I've avoided adding new property to WebKitWebSettings and rather piped the setting through DumpRenderTreeSupport. It should be discussed if new API is welcome for this setting.

The patch fixes two tests from comment #1 but the test in the title is flaky. It seems the plugin is loaded before the page, causing such failures:

--- WebKitBuild/Release/layout-test-results/plugins/netscape-plugin-page-cache-works-expected.txt
+++ WebKitBuild/Release/layout-test-results/plugins/netscape-plugin-page-cache-works-actual.txt
@@ -5,9 +5,9 @@
 
 Unfortunately there is no reliable way to get affirmative confirmation that the plugin was destroyed upon navigation away from the page. For now we'll assume recreation means it had successfully been destroyed.
 
+null: Plugin created
 Initial load: Page loaded
 Initial load: Page shown
-Initial load: Plugin created
 Initial load: Accessing testObject.property
 Initial load: Accessed testObject.property without exception
 Initial load: Assigning to testObject.property

[Zan Dobersek]

Created attachment 129894 [details]
Patch

[Zan Dobersek]

(In reply to comment #3)
> (In reply to comment #2)
> > Created an attachment (id=129866) [details] [details]
> > Patch
> 
> The patch fixes two tests from comment #1 but the test in the title is flaky. 

Now covered by https://bugs.webkit.org/show_bug.cgi?id=80158

[Philippe Normand]

Comment on attachment 129894 [details]
Patch

Patch looks good, the only small doubt I have is wether a new websetting would be needed or not.

[Martin Robinson]

Comment on attachment 129894 [details]
Patch

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

> Tools/DumpRenderTree/gtk/LayoutTestControllerGtk.cpp:841
>          return;
> +    } else if (g_str_equal(originalName.get(), "WebKitPageCacheSupportsPluginsPreferenceKey")) {
> +        DumpRenderTreeSupportGtk::setPageCacheSupportsPlugins(webkit_web_frame_get_web_view(mainFrame), !g_ascii_strcasecmp(valueAsString.get(), "true") || !g_ascii_strcasecmp(valueAsString.get(), "1"));
> +        return;

Please turn !g_ascii_strcasecmp(valueAsString.get(), "true") || !g_ascii_strcasecmp(valueAsString.get() into a helper function, now that it's repeated three times.

[Zan Dobersek]

Created attachment 130130 [details]
Patch

[Martin Robinson]

Comment on attachment 130130 [details]
Patch

Thanks!

[WebKit Review Bot]

Comment on attachment 130130 [details]
Patch

Clearing flags on attachment: 130130

Committed r109753: <http://trac.webkit.org/changeset/109753>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.