Bug 47690 – Assertion failure in NetscapePlugin::fromNPP when using Shockwave in WebKit2

• Component: Plug-ins
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

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.

[Adam Roben (:aroben)]

<rdar://problem/8553020>

[Adam Roben (:aroben)]

Created attachment 70777 [details]
Don't require the plugin to always use the same NPP struct we gave it in NPP_New

[WebKit Review Bot]

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.

[John Sullivan]

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?

[Adam Roben (:aroben)]

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

[Adam Roben (:aroben)]

Committed r70190: <http://trac.webkit.org/changeset/70190>

[WebKit Review Bot]

http://trac.webkit.org/changeset/70190 might have broken Qt Linux Release

[Adam Barth]

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

[Adam Barth]

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?

[Alejandro G. Castro]

(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

[Alejandro G. Castro]

Skipped in GTK until Adam can review the issue:

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

[Csaba Osztrogonác]

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

[Adam Roben (:aroben)]

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

[Adam Roben (:aroben)]

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

[Adam Roben (:aroben)]

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

[Adam Roben (:aroben)]

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