Bug 47690

Summary: Assertion failure in NetscapePlugin::fromNPP when using Shockwave in WebKit2
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: Plug-insAssignee: Adam Roben (:aroben) <aroben>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, alex, andersca, eric, jhoneycutt, ossy, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.nga.gov/kids/zone/dollhouse.htm
Attachments:
Description Flags
Don't require the plugin to always use the same NPP struct we gave it in NPP_New none

Description Adam Roben (:aroben) 2010-10-14 13:44:42 PDT
To reproduce:

1. Go to http://www.nga.gov/kids/zone/dollhouse.htm

You'll hit an assertion in NetscapePlugin::fromNPP:

        NetscapePlugin* plugin = static_cast<NetscapePlugin*>(npp->ndata);
        ASSERT(npp == &plugin->m_npp);

This assertion seems to be incorrect. WebCore doesn't have it, WebKit/mac doesn't have it, Firefox doesn't have it. Removing it allows the page to work just fine.
Comment 1 Adam Roben (:aroben) 2010-10-14 13:45:30 PDT
<rdar://problem/8553020>
Comment 2 Adam Roben (:aroben) 2010-10-14 14:06:47 PDT
Created attachment 70777 [details]
Don't require the plugin to always use the same NPP struct we gave it in NPP_New
Comment 3 WebKit Review Bot 2010-10-14 14:09:01 PDT
Attachment 70777 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKitTools/DumpRenderTree/TestNetscapePlugIn/Tests/PassDifferentNPPStruct.cpp:26:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/DumpRenderTree/TestNetscapePlugIn/Tests/PassDifferentNPPStruct.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 John Sullivan 2010-10-14 14:27:16 PDT
Comment on attachment 70777 [details]
Don't require the plugin to always use the same NPP struct we gave it in NPP_New

Maybe you can make the stylebot slightly happier by reordering #includes?
Comment 5 Adam Roben (:aroben) 2010-10-14 14:38:22 PDT
(In reply to comment #4)
> (From update of attachment 70777 [details])
> Maybe you can make the stylebot slightly happier by reordering #includes?

I think to make the style bot happy we will have to teach it about the non-standard #include style of these test files.

Thanks for reviewing!
Comment 6 Adam Roben (:aroben) 2010-10-20 16:42:01 PDT
Committed r70190: <http://trac.webkit.org/changeset/70190>
Comment 7 WebKit Review Bot 2010-10-20 17:22:13 PDT
http://trac.webkit.org/changeset/70190 might have broken Qt Linux Release
Comment 9 Adam Barth 2010-10-21 01:43:49 PDT
I've added this test to the Skipped list for Mac in http://trac.webkit.org/changeset/70213.  Can you look into why it's failing?
Comment 10 Alejandro G. Castro 2010-10-21 01:52:13 PDT
(In reply to comment #9)
> I've added this test to the Skipped list for Mac in http://trac.webkit.org/changeset/70213.  Can you look into why it's failing?

This test is also failing:

http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r70211%20(14993)/plugins/pass-different-npp-struct-pretty-diff.html
Comment 11 Alejandro G. Castro 2010-10-21 02:08:32 PDT
Skipped in GTK until Adam can review the issue:

http://trac.webkit.org/changeset/70215
Comment 12 Csaba Osztrogon√°c 2010-10-21 04:32:02 PDT
(In reply to comment #7)
> http://trac.webkit.org/changeset/70190 might have broken Qt Linux Release

Any progression with fixing? 

This patch broke the whole world, I don't think if it is a good 
idea to put this failing test to the skipped list on all platform.
Comment 13 Adam Roben (:aroben) 2010-10-21 08:44:24 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > I've added this test to the Skipped list for Mac in http://trac.webkit.org/changeset/70213.  Can you look into why it's failing?
> 
> This test is also failing:
> 
> http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r70211%20(14993)/plugins/pass-different-npp-struct-pretty-diff.html

I think PassDifferentNPPStruct.cpp just needs to be added to the GTK build. Sorry about that.
Comment 14 Adam Roben (:aroben) 2010-10-21 08:44:37 PDT
(In reply to comment #8)
> This new test doesn't seem to pass:
> 
> http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r70205%20(19462)/plugins/pass-different-npp-struct-pretty-diff.html

I'm looking into it.
Comment 15 Adam Roben (:aroben) 2010-10-21 08:46:00 PDT
(In reply to comment #12)
> (In reply to comment #7)
> > http://trac.webkit.org/changeset/70190 might have broken Qt Linux Release
> 
> Any progression with fixing? 

I added PassDifferentNPPStruct.cpp to TestNetscapePlugin.pro in <http://trac.webkit.org/changeset/70196>. I don't know why it's continuing to fail after that. Hopefully I can find a friendly Qt person to help me figure it out. Sorry for the trouble!
Comment 16 Adam Roben (:aroben) 2010-10-21 10:00:41 PDT
Fixed GTK and Qt in r70238 and r70242 with help from Andreas and Anders:

http://trac.webkit.org/changeset/70238
http://trac.webkit.org/changeset/70242

Unskipped on SnowLeopard after running Software Update on the bots in r70243:

http://trac.webkit.org/changeset/70243