Bug 62249 - [GTK] Crash observed with nspluginwrapper and flash
Summary: [GTK] Crash observed with nspluginwrapper and flash
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Martin Robinson
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2011-06-07 16:23 PDT by Martin Robinson
Modified: 2011-08-02 01:33 PDT (History)
5 users (show)

See Also:


Attachments
Work in progress patch (11.38 KB, patch)
2011-06-23 14:02 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (15.32 KB, patch)
2011-06-23 17:00 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch making the change and test X11 only (9.51 KB, patch)
2011-06-29 18:35 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-03 (1.48 MB, application/zip)
2011-06-29 19:57 PDT, WebKit Review Bot
no flags Details
Archive of layout-test-results from ec2-cq-03 (1.77 MB, application/zip)
2011-06-30 11:32 PDT, WebKit Review Bot
no flags Details
wip (33.00 KB, patch)
2011-08-02 01:29 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2011-06-07 16:23:22 PDT
Here is the stack trace:


Program terminated with signal 11, Segmentation fault.
#0  0x00007f8f5810cd0e in WebCore::Widget::isVisible (this=0x0) at ../../Source/WebCore/platform/Widget.h:177
177	    bool isVisible() const { return m_selfVisible && m_parentVisible; } // Whether or not we are actually visible.
(gdb) where
#0  0x00007f8f5810cd0e in WebCore::Widget::isVisible (this=0x0) at ../../Source/WebCore/platform/Widget.h:177
#1  0x00007f8f58147ad3 in WebCore::PluginView::invalidateWindowlessPluginRect (this=0x0, rect=...) at ../../Source/WebCore/plugins/PluginView.cpp:1225
#2  0x00007f8f579859c9 in WebCore::PluginView::invalidateRect (this=0x0, rect=0x7fff9dd8bc80) at ../../Source/WebCore/plugins/gtk/PluginViewGtk.cpp:711
#3  0x00007f8f5812ce15 in NPN_InvalidateRect (instance=0x0, invalidRect=0x7fff9dd8bc80) at ../../Source/WebCore/plugins/npapi.cpp:124
#4  0x00007f8eed3b6bf0 in ?? () from /var/lib/flashplugin-installer/npwrapper.libflashplayer.so
#5  0x00007f8eed3b7955 in ?? () from /var/lib/flashplugin-installer/npwrapper.libflashplayer.so
#6  0x00007f8eed3c1c75 in ?? () from /var/lib/flashplugin-installer/npwrapper.libflashplayer.so
#7  0x00007f8eed3c1e67 in ?? () from /var/lib/flashplugin-installer/npwrapper.libflashplayer.so
#8  0x00007f8eed3c2013 in ?? () from /var/lib/flashplugin-installer/npwrapper.libflashplayer.so
#9  0x00007f8eed3c3b08 in ?? () from /var/lib/flashplugin-installer/npwrapper.libflashplayer.so
#10 0x00007f8eed3b738c in ?? () from /var/lib/flashplugin-installer/npwrapper.libflashplayer.so
#11 0x00007f8f534f1bcd in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#12 0x00007f8f534f23a8 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#13 0x00007f8f534f29f2 in g_main_loop_run () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#14 0x00007f8f56addd9d in gtk_main () from /usr/lib/libgtk-3.so.0
#15 0x000000000042f2a4 in main (argc=1, argv=0x7fff9dd91268) at ephy-main.c:747

It appears as though nspluginwrapper is really badly behaved here and sends a null NPInstance.
Comment 1 Martin Robinson 2011-06-07 16:23:57 PDT
The fix for this issue should be merged into the WebKitGTK+ 1.4.x stable branch.
Comment 2 Martin Robinson 2011-06-23 14:02:41 PDT
Created attachment 98394 [details]
Work in progress patch
Comment 3 Martin Robinson 2011-06-23 17:00:24 PDT
Created attachment 98436 [details]
Patch
Comment 4 Martin Robinson 2011-06-27 12:01:16 PDT
CCing some Apple people who might be in a good position to review this patch, since it touches other ports.
Comment 5 Adam Roben (:aroben) 2011-06-27 12:04:24 PDT
Comment on attachment 98436 [details]
Patch

Have you already checked the Mac WebKit1 plugin implementation?
Comment 6 Adam Roben (:aroben) 2011-06-27 12:04:56 PDT
(In reply to comment #5)
> (From update of attachment 98436 [details])
> Have you already checked the Mac WebKit1 plugin implementation?

(It's in Source/WebKit/mac/Plugins.)
Comment 7 Anders Carlsson 2011-06-27 12:10:02 PDT
Comment on attachment 98436 [details]
Patch

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

> Source/WebKit2/WebProcess/Plugins/Netscape/NetscapeBrowserFuncs.cpp:601
> +    // NSPluginWrapper will sometimes give us a null NPP here.
> +    if (!npp)
> +        return;
>      RefPtr<NetscapePlugin> plugin = NetscapePlugin::fromNPP(npp);
>      plugin->invalidate(invalidRect);

Please be more specific about what NSPluginWrapper is here, and what architectures it might run on. 
Do we need an #if PLUGIN_ARCHITECTURE(X11) here too?
Comment 8 Martin Robinson 2011-06-27 13:46:29 PDT
Comment on attachment 98436 [details]
Patch

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

>> Source/WebKit2/WebProcess/Plugins/Netscape/NetscapeBrowserFuncs.cpp:601
>>      plugin->invalidate(invalidRect);
> 
> Please be more specific about what NSPluginWrapper is here, and what architectures it might run on. 
> Do we need an #if PLUGIN_ARCHITECTURE(X11) here too?

Sure. I don't mind intensifying the comment at all. NSPluginWrapper runs only in X11 as far as I know. If I add #if PLUGIN_ARCHITECTURE(X11) should I make a similar change to the WebKit1 implementation as well? If that's the case, I should probably make the test GTK+/Qt only.
Comment 9 Martin Robinson 2011-06-27 13:48:21 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 98436 [details] [details])
> > Have you already checked the Mac WebKit1 plugin implementation?
> 
> (It's in Source/WebKit/mac/Plugins.)

I didn't make any changes there, but I when I tested the change on OS X, my test passed.
Comment 10 Martin Robinson 2011-06-29 18:35:10 PDT
Created attachment 99202 [details]
Patch making the change and test X11 only
Comment 11 WebKit Review Bot 2011-06-29 19:57:46 PDT
Comment on attachment 99202 [details]
Patch making the change and test X11 only

Attachment 99202 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8966281

New failing tests:
platform/gtk/plugins/invalidate-rect-with-null-npp-argument.html
Comment 12 WebKit Review Bot 2011-06-29 19:57:52 PDT
Created attachment 99221 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 13 Martin Robinson 2011-06-30 00:09:04 PDT
(In reply to comment #11)
> (From update of attachment 99202 [details])
> Attachment 99202 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/8966281
> 
> New failing tests:
> platform/gtk/plugins/invalidate-rect-with-null-npp-argument.html


I'm not exactly sure why the chromium bot ran a test that's in platform/gtk.
Comment 14 Martin Robinson 2011-06-30 10:42:58 PDT
Comment on attachment 99202 [details]
Patch making the change and test X11 only

Thanks for the review!
Comment 15 WebKit Review Bot 2011-06-30 11:32:12 PDT
Comment on attachment 99202 [details]
Patch making the change and test X11 only

Rejecting attachment 99202 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2

Last 500 characters of output:
shes   ( 0.5%)
      5 tests timed out          ( 0.4%)
     51 text diff mismatch       ( 4.1%)
    340 skipped                  (27.2%)

=> Tests that will only be fixed if they crash (WONTFIX) (8129):
      1 DumpRenderTree crash     ( 0.0%)
      1 test timed out           ( 0.0%)
    109 text diff mismatch       ( 1.3%)
   7969 skipped                  (98.0%)


Regressions: Unexpected DumpRenderTree crashes : (1)
  platform/gtk/plugins/invalidate-rect-with-null-npp-argument.html = CRASH



Full output: http://queues.webkit.org/results/8957808
Comment 16 WebKit Review Bot 2011-06-30 11:32:19 PDT
Created attachment 99341 [details]
Archive of layout-test-results from ec2-cq-03

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 17 Martin Robinson 2011-06-30 12:31:08 PDT
Committed r90148: <http://trac.webkit.org/changeset/90148>
Comment 18 Hayato Ito 2011-08-02 01:29:58 PDT
Created attachment 102625 [details]
wip
Comment 19 Hayato Ito 2011-08-02 01:33:27 PDT
Comment on attachment 102625 [details]
wip

Obsoleting the patch because bug id was wrong. Sorry.