[GTK] Crash observed with nspluginwrapper and flash

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

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.

[Martin Robinson]

The fix for this issue should be merged into the WebKitGTK+ 1.4.x stable branch.

[Martin Robinson]

Created attachment 98394 [details]
Work in progress patch

[Martin Robinson]

Created attachment 98436 [details]
Patch

[Martin Robinson]

CCing some Apple people who might be in a good position to review this patch, since it touches other ports.

[Adam Roben (:aroben)]

Comment on attachment 98436 [details]
Patch

Have you already checked the Mac WebKit1 plugin implementation?

[Adam Roben (:aroben)]

(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.)

[Anders Carlsson]

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?

[Martin Robinson]

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.

[Martin Robinson]

(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.

[Martin Robinson]

Created attachment 99202 [details]
Patch making the change and test X11 only

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Martin Robinson]

(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.

[Martin Robinson]

Comment on attachment 99202 [details]
Patch making the change and test X11 only

Thanks for the review!

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Martin Robinson]

Committed r90148: <http://trac.webkit.org/changeset/90148>

[Hayato Ito]

Created attachment 102625 [details]
wip

[Hayato Ito]

Comment on attachment 102625 [details]
wip

Obsoleting the patch because bug id was wrong. Sorry.