Bug 74409 - [GTK] plugins/netscape-plugin-page-cache-works.html fails
Summary: [GTK] plugins/netscape-plugin-page-cache-works.html fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-13 07:05 PST by Philippe Normand
Modified: 2012-03-05 08:55 PST (History)
2 users (show)

See Also:


Attachments
Patch (5.29 KB, patch)
2012-03-02 03:39 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (6.05 KB, patch)
2012-03-02 06:29 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (7.87 KB, patch)
2012-03-05 07:36 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2011-12-13 07:05:17 PST
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.
Comment 1 Zan Dobersek 2012-03-02 03:37:38 PST
Also affected tests:
media/crash-closing-page-with-media-as-plugin-fallback.htmlplugins/crash-restoring-plugin-page-from-page-cache.html
Comment 2 Zan Dobersek 2012-03-02 03:39:24 PST
Created attachment 129866 [details]
Patch
Comment 3 Zan Dobersek 2012-03-02 03:44:24 PST
(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
Comment 4 Zan Dobersek 2012-03-02 06:29:30 PST
Created attachment 129894 [details]
Patch
Comment 5 Zan Dobersek 2012-03-02 06:30:33 PST
(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
Comment 6 Philippe Normand 2012-03-02 07:47:40 PST
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.
Comment 7 Martin Robinson 2012-03-02 08:38:12 PST
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.
Comment 8 Zan Dobersek 2012-03-05 07:36:23 PST
Created attachment 130130 [details]
Patch
Comment 9 Martin Robinson 2012-03-05 08:38:09 PST
Comment on attachment 130130 [details]
Patch

Thanks!
Comment 10 WebKit Review Bot 2012-03-05 08:55:21 PST
Comment on attachment 130130 [details]
Patch

Clearing flags on attachment: 130130

Committed r109753: <http://trac.webkit.org/changeset/109753>
Comment 11 WebKit Review Bot 2012-03-05 08:55:26 PST
All reviewed patches have been landed.  Closing bug.