Bug 137425 - Plugin process crashes in NPN_InvokeDefault
Summary: Plugin process crashes in NPN_InvokeDefault
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Critical
Assignee: Nobody
URL:
Keywords:
: 154888 176304 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-10-04 18:02 PDT by Michael Catanzaro
Modified: 2018-03-28 11:03 PDT (History)
8 users (show)

See Also:


Attachments
Patch (20.48 KB, patch)
2016-10-28 04:12 PDT, Carlos Garcia Campos
mcatanzaro: review-
Details | Formatted Diff | Diff
Updated patch (25.30 KB, patch)
2016-10-30 02:25 PDT, Carlos Garcia Campos
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-yosemite (917.46 KB, application/zip)
2016-10-30 03:26 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.18 MB, application/zip)
2016-10-30 03:32 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-yosemite (1.72 MB, application/zip)
2016-10-30 03:41 PDT, Build Bot
no flags Details
Updated patch (33.65 KB, patch)
2016-10-31 01:01 PDT, Carlos Garcia Campos
darin: review-
Details | Formatted Diff | Diff
Updated patch (30.84 KB, patch)
2016-10-31 01:47 PDT, Carlos Garcia Campos
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-yosemite (1.25 MB, application/zip)
2016-10-31 02:39 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-yosemite (1.82 MB, application/zip)
2016-10-31 02:54 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (1.01 MB, application/zip)
2016-10-31 02:55 PDT, Build Bot
no flags Details
Newer backtrace (48.76 KB, text/plain)
2018-03-28 10:17 PDT, Michael Catanzaro
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2014-10-04 18:02:57 PDT
We're getting downstream reports of a crash in NPN_InvokeDefault  [1] [2].  The first recorded crash is with 2.5.3, but that's the first 2.5 series release that was packaged for Fedora.

So far, everyone who provided a comment says the crash happened on extensions.gnome.org:

"I tried to upgrade the World Clock extension at extensions.gnome.org using GNOME Web."
"I surfed on extensions.gnome.org using epiphany."

From the detailed backtrace [3], it looks like the first user to report the bug was also on extensions.gnome.org.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1147314
[2] https://retrace.fedoraproject.org/faf/reports/427972/
[3] https://bugzilla.redhat.com/attachment.cgi?id=942098
Comment 1 Carlos Garcia Campos 2014-10-05 00:59:48 PDT
#0  WebKit::NPN_InvokeDefault (npp=<optimized out>, npObject=0x0, arguments=0x7fff90c65a90, argumentCount=3, result=0x7fff90c65a70) at /usr/src/debug/webkitgtk-2.6.0/Source/WebKit2/WebProcess/Plugins/Netscape/NetscapeBrowserFuncs.cpp:686
        plugin = {m_ptr = 0x7f88605d3dc0}
        protector = {m_protector = {_M_t = {<std::_Tuple_impl<0ul, WebKit::PluginController::PluginDestructionProtector*, std::default_delete<WebKit::PluginController::PluginDestructionProtector> >> = {<std::_Tuple_impl<1ul, std::default_delete<WebKit::PluginController::PluginDestructionProtector> >> = {<std::_Tuple_impl<2ul>> = {<No data fields>}, <std::_Head_base<1ul, std::default_delete<WebKit::PluginController::PluginDestructionProtector>, true>> = {<std::default_delete<WebKit::PluginController::PluginDestructionProtector>> = {<No data fields>}, <No data fields>}, <No data fields>}, <std::_Head_base<0ul, WebKit::PluginController::PluginDestructionProtector*, false>> = {_M_head_impl = 0x2017990}, <No data fields>}, <No data fields>}}}
#1  0x00007f8844008d03 in on_shell_signal (proxy=<optimized out>, sender_name=<optimized out>, signal_name=<optimized out>, parameters=<optimized out>, user_data=0x212e500) at browser-plugin.c:298
        status = 1
        uuid = 0x2038ee0 "netspeed@hedayaty.gmail.com"
        error = 0x2002480 ""
        args = {{type = NPVariantType_String, value = {boolValue = 224, intValue = 33787616, doubleValue = 1.6693300320476035e-316, stringValue = {UTF8Characters = 0x2038ee0 "netspeed@hedayaty.gmail.com", UTF8Length = 27}, objectValue = 0x2038ee0}}, {type = NPVariantType_Int32, value = {boolValue = true, intValue = 1, doubleValue = 6.9278918583434705e-310, stringValue = {UTF8Characters = 0x7f8800000001 <error: Cannot access memory at address 0x7f8800000001>, UTF8Length = 0}, objectValue = 0x7f8800000001}}, {type = NPVariantType_String, value = {boolValue = 128, intValue = 33563776, doubleValue = 1.6582708666310931e-316, stringValue = {UTF8Characters = 0x2002480 "", UTF8Length = 0}, objectValue = 0x2002480}}}
        result = {type = NPVariantType_Void, value = {boolValue = false, intValue = 0, doubleValue = 0, stringValue = {UTF8Characters = 0x0, UTF8Length = 0}, objectValue = 0x0}}
        sender_name = <optimized out>
        parameters = <optimized out>
        proxy = <optimized out>
        signal_name = <optimized out>
        user_data = 0x212e500
        obj = <optimized out>

I would say the gnome-shell browser plugin is calling invokedefault with a NULL NPObject.
Comment 2 Michael Catanzaro 2014-10-05 08:41:48 PDT
OK, moved to https://bugzilla.gnome.org/show_bug.cgi?id=737932 for now.
Comment 3 Carlos Garcia Campos 2015-11-17 02:58:33 PST
I'm checking this, and I still think it's a bug in the plugin calling invokeDefault with a null NPObject, but we shouldn't trust plugins anyway. I think we should protect all public plugins api. Firefox seems to do something like this in invokeDefault, for example:

if (!npp || !npobj || !npobj->_class || !npobj->_class->invokeDefault)
    return false;

I think we could do something similar to what we do in retainNPObject, releaseNPObject, etc. Add ASSERTS to catch those issues in Debug builds, but also early returns to not crash in Release.
Comment 4 Carlos Garcia Campos 2015-11-17 02:59:31 PST
This is not specific to WebKitGTK+, moving to WebKit2
Comment 5 Darin Adler 2015-11-18 08:51:34 PST
Since plug-ins are a cross browser API we need to be consistent with other browsers about how we handle null. I don’t think an assert is appropriate if this value is allowed. I think it’s fine to tolerate nulls for these arguments if the other browsers do.
Comment 6 Darin Adler 2015-11-18 08:53:43 PST
I think the language about “trusting” plug-ins isn’t really quite right. If a plug-in does something wrong there’s not much we can do about it. Whether we “trust” it or not doesn’t matter.

On the other hand, if other browsers tolerate null but we dereference the null and crash, then there’s a real interoperability problem and we should fix to be consistent.
Comment 7 Michael Catanzaro 2016-07-09 11:47:55 PDT
Still getting reports of this with latest GNOME Shell and WebKitGTK+.
Comment 8 Michael Catanzaro 2016-07-25 07:54:05 PDT
(In reply to comment #7)
> Still getting reports of this with latest GNOME Shell and WebKitGTK+.

3621 reports of this in Fedora so far. This one's old; reports go back to 2013, so it's not a recent regression.
Comment 9 Carlos Garcia Campos 2016-10-28 03:56:33 PDT
*** Bug 154888 has been marked as a duplicate of this bug. ***
Comment 10 Carlos Garcia Campos 2016-10-28 04:12:55 PDT
Created attachment 293142 [details]
Patch

The new test file needs to be added to Xcode.
Comment 11 Michael Catanzaro 2016-10-28 05:06:57 PDT
Comment on attachment 293142 [details]
Patch

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

Thanks very much for working on this, it will cut my downstream bug queue by about a quarter.

Unfortunately it broke macOS tests, which will need to be investigated:

Regressions: Unexpected text-only failures (1)
  plugins/browser-funcs-invalid-args.html [ Failure ]

Regressions: Unexpected timeouts (1)
  plugins/npruntime/npruntime-calls-with-null-npp.html [ Timeout ]

> Source/WebKit2/ChangeLog:9
> +        the same approach than Firefox.

than -> as
Comment 12 Carlos Garcia Campos 2016-10-28 06:26:31 PDT
(In reply to comment #11)
> Comment on attachment 293142 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=293142&action=review
> 
> Thanks very much for working on this, it will cut my downstream bug queue by
> about a quarter.

Note that this is not enough, the bug in in the plugin.

> Unfortunately it broke macOS tests, which will need to be investigated:

There's nothing to investigate, as I said the test needs to be added to xcode, I knew it was not going to work.

> Regressions: Unexpected text-only failures (1)
>   plugins/browser-funcs-invalid-args.html [ Failure ]
> 
> Regressions: Unexpected timeouts (1)
>   plugins/npruntime/npruntime-calls-with-null-npp.html [ Timeout ]
> 
> > Source/WebKit2/ChangeLog:9
> > +        the same approach than Firefox.
> 
> than -> as

English is hard.
Comment 13 Darin Adler 2016-10-28 10:25:10 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > Unfortunately it broke macOS tests, which will need to be investigated:
> 
> There's nothing to investigate, as I said the test needs to be added to
> xcode, I knew it was not going to work.

I’d like to help or get someone to help you. I don’t understand, though.

I believe you are pointing out that BrowserFunctionsInvalidArguments.cpp needs to be added to an Xcode project, and someone who develops on a Mac should help you add that to your patch.

But how does this cause the browser-funcs-invalid-args.html and npruntime-calls-with-null-npp.html tests to fail?
Comment 14 Michael Catanzaro 2016-10-28 11:42:13 PDT
And anyway: do we still want this?
Comment 15 Carlos Garcia Campos 2016-10-29 01:05:13 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Unfortunately it broke macOS tests, which will need to be investigated:
> > 
> > There's nothing to investigate, as I said the test needs to be added to
> > xcode, I knew it was not going to work.
> 
> I’d like to help or get someone to help you. I don’t understand, though.

Thanks.

> I believe you are pointing out that BrowserFunctionsInvalidArguments.cpp
> needs to be added to an Xcode project, and someone who develops on a Mac
> should help you add that to your patch.

Right.

> But how does this cause the browser-funcs-invalid-args.html and
> npruntime-calls-with-null-npp.html tests to fail?

browser-funcs-invalid-args.html is added in this patch and requires BrowserFunctionsInvalidArguments.cpp to work. I didn't realize npruntime-calls-with-null-npp.html was also failing, Michael is right and I need to investigate that one. But in any case I need BrowserFunctionsInvalidArguments.cpp to be built on mac.
Comment 16 Carlos Garcia Campos 2016-10-29 01:08:28 PDT
(In reply to comment #14)
> And anyway: do we still want this?

Yes, gnome-shell plugin is not the only plugin in the world, if a plugin calls a browser funcs with a npobject we shouldn't crash. Also for browsers interoperability as Darin said in comment #5, that's the reason why I added the early returns in the very same places that firefox.
Comment 17 Carlos Garcia Campos 2016-10-29 01:27:15 PDT
(In reply to comment #11)
> Regressions: Unexpected timeouts (1)
>   plugins/npruntime/npruntime-calls-with-null-npp.html [ Timeout ]

Ok, this is timing out because my patch changed the behavior. In current code we handle null npp in browser funcs, but in a slightly different way. NetscapePlugin::fromNPP() returns nullptr if the given instance is nullptr, and then PluginDestructionProtector does nothing if the instance is nullptr. So we end up actually calling the browser function on the given npobject even when the plugin instance is nullptr. With the new patch, we return early if npp is nullptr and the function returns false, which is not expected by npruntime-calls-with-null-npp.html. When fixing this I noticed that firefox always returns early from all the browser funcs when npp is nullptr, and I planned to check this also in WebKit in a follow up patch. I will update the test to make it work after this patch, and then I'll do the same for all other methods to keep consistency.
I also noticed that I should probably move browser-funcs-invalid-args.html to the npruntime subdir as well.
Comment 18 Carlos Garcia Campos 2016-10-30 02:25:26 PDT
Created attachment 293340 [details]
Updated patch

NOTE: still needs the xcode changes, so npruntime-calls-with-null-npobject.html is going to fail for sure in mac EWS.

Renamed the test as npruntime-calls-with-null-npobject.html for consistency with npruntime-calls-with-null-npp.html. Also updated the latter because the behavior changed.
Comment 19 Build Bot 2016-10-30 03:26:01 PDT
Comment on attachment 293340 [details]
Updated patch

Attachment 293340 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2404441

New failing tests:
plugins/npruntime/npruntime-calls-with-null-npp.html
plugins/npruntime/npruntime-calls-with-null-npobject.html
Comment 20 Build Bot 2016-10-30 03:26:06 PDT
Created attachment 293342 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 21 Build Bot 2016-10-30 03:32:45 PDT
Comment on attachment 293340 [details]
Updated patch

Attachment 293340 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2404446

New failing tests:
plugins/npruntime/npruntime-calls-with-null-npobject.html
Comment 22 Build Bot 2016-10-30 03:32:48 PDT
Created attachment 293343 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 23 Build Bot 2016-10-30 03:41:02 PDT
Comment on attachment 293340 [details]
Updated patch

Attachment 293340 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2404463

New failing tests:
plugins/npruntime/npruntime-calls-with-null-npp.html
plugins/npruntime/npruntime-calls-with-null-npobject.html
Comment 24 Build Bot 2016-10-30 03:41:05 PDT
Created attachment 293344 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 25 Carlos Garcia Campos 2016-10-31 00:40:54 PDT
Failures are in WebKit1 where we would need to apply the same fixes.
Comment 26 Carlos Garcia Campos 2016-10-31 01:01:07 PDT
Created attachment 293400 [details]
Updated patch

Apply the sames fixes to WebKit1
Comment 27 WebKit Commit Bot 2016-10-31 01:04:40 PDT
Attachment 293400 [details] did not pass style-queue:


ERROR: Source/WebCore/bridge/NP_jsobject.cpp:168:  _NPN_InvokeDefault is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:290:  _NPN_GetProperty is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:328:  _NPN_SetProperty is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:360:  _NPN_RemoveProperty is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:403:  _NPN_HasProperty is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:436:  _NPN_HasMethod is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:473:  _NPN_Enumerate is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:517:  _NPN_Construct is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 8 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 28 Darin Adler 2016-10-31 01:21:21 PDT
Comment on attachment 293400 [details]
Updated patch

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

> Source/WebCore/bridge/NP_jsobject.cpp:171
> +    if (!npp || !o || !o->_class || !o->_class->invokeDefault)
> +        return false;

This is a change in behavior. Before we would return true with a void value in result when the invokeDefault pointer was null. I don’t see any test coverage for this change in behavior.

> Source/WebCore/bridge/NP_jsobject.cpp:173
>      if (o->_class == NPScriptObjectClass) {

This is broken now, because invokeDefault is null in NPScriptObjectClass, so this code will never run. No test coverage?

> Source/WebCore/bridge/NP_jsobject.cpp:213
> +    if (!npp || !o || !o->_class || !o->_class->invoke)
> +        return false;

This is a change in behavior. Before we would return true with a void value for result when the invoke pointer was null. I don’t see any test coverage for this change in behavior.

> Source/WebCore/bridge/NP_jsobject.cpp:215
>      if (o->_class == NPScriptObjectClass) {

Also broken like the other case above.

> Source/WebCore/bridge/NP_jsobject.cpp:293
> +    if (!npp || !o || !o->_class || !o->_class->getProperty)
> +        return false;

This is a change in behavior. Before we would set result to a void value when the getProperty pointer was null. Maybe we should test this?

> Source/WebCore/bridge/NP_jsobject.cpp:295
>      if (o->_class == NPScriptObjectClass) {

Also broken like the other cases above.

> Source/WebCore/bridge/NP_jsobject.cpp:322
> -    if (o->_class->hasProperty && o->_class->getProperty) {
> -        if (o->_class->hasProperty(o, propertyName))
> -            return o->_class->getProperty(o, propertyName, variant);
> -        return false;
> -    }
> +    if (o->_class->hasProperty(o, propertyName))
> +        return o->_class->getProperty(o, propertyName, variant);

Why is it OK to drop the check of o->_class->hasProperty for null? Do we have a test case covering that?

> Source/WebCore/bridge/NP_jsobject.cpp:333
>      if (o->_class == NPScriptObjectClass) {

Also broken like the other cases above.

> Source/WebCore/bridge/NP_jsobject.cpp:408
>      if (o->_class == NPScriptObjectClass) {

Also broken like the other cases above.

> Source/WebCore/bridge/NP_jsobject.cpp:441
>      if (o->_class == NPScriptObjectClass) {

Also broken like the other cases above.

> Source/WebCore/bridge/NP_jsobject.cpp:522
>      if (o->_class == NPScriptObjectClass) {

Also broken like the other cases above.
Comment 29 Carlos Garcia Campos 2016-10-31 01:31:12 PDT
(In reply to comment #28)
> Comment on attachment 293400 [details]
> Updated patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=293400&action=review
> 
> > Source/WebCore/bridge/NP_jsobject.cpp:171
> > +    if (!npp || !o || !o->_class || !o->_class->invokeDefault)
> > +        return false;
> 
> This is a change in behavior. Before we would return true with a void value
> in result when the invokeDefault pointer was null. I don’t see any test
> coverage for this change in behavior.

Yes, this is not covered by tests, I'm afraid, I just copied the early return condition from firefox, but only null npp and null npobject is covered by the tests (I think, I'll check anyway).

> > Source/WebCore/bridge/NP_jsobject.cpp:173
> >      if (o->_class == NPScriptObjectClass) {
> 
> This is broken now, because invokeDefault is null in NPScriptObjectClass, so
> this code will never run. No test coverage?

Ah, I didn't notice that. 

> > Source/WebCore/bridge/NP_jsobject.cpp:213
> > +    if (!npp || !o || !o->_class || !o->_class->invoke)
> > +        return false;
> 
> This is a change in behavior. Before we would return true with a void value
> for result when the invoke pointer was null. I don’t see any test coverage
> for this change in behavior.
> 
> > Source/WebCore/bridge/NP_jsobject.cpp:215
> >      if (o->_class == NPScriptObjectClass) {
> 
> Also broken like the other case above.
> 
> > Source/WebCore/bridge/NP_jsobject.cpp:293
> > +    if (!npp || !o || !o->_class || !o->_class->getProperty)
> > +        return false;
> 
> This is a change in behavior. Before we would set result to a void value
> when the getProperty pointer was null. Maybe we should test this?
> 
> > Source/WebCore/bridge/NP_jsobject.cpp:295
> >      if (o->_class == NPScriptObjectClass) {
> 
> Also broken like the other cases above.
> 
> > Source/WebCore/bridge/NP_jsobject.cpp:322
> > -    if (o->_class->hasProperty && o->_class->getProperty) {
> > -        if (o->_class->hasProperty(o, propertyName))
> > -            return o->_class->getProperty(o, propertyName, variant);
> > -        return false;
> > -    }
> > +    if (o->_class->hasProperty(o, propertyName))
> > +        return o->_class->getProperty(o, propertyName, variant);
> 
> Why is it OK to drop the check of o->_class->hasProperty for null? Do we
> have a test case covering that?

I went too fast and thought this was only checking getProperty and removed the if because getProperty was already null checked before.

> > Source/WebCore/bridge/NP_jsobject.cpp:333
> >      if (o->_class == NPScriptObjectClass) {
> 
> Also broken like the other cases above.
> 
> > Source/WebCore/bridge/NP_jsobject.cpp:408
> >      if (o->_class == NPScriptObjectClass) {
> 
> Also broken like the other cases above.
> 
> > Source/WebCore/bridge/NP_jsobject.cpp:441
> >      if (o->_class == NPScriptObjectClass) {
> 
> Also broken like the other cases above.
> 
> > Source/WebCore/bridge/NP_jsobject.cpp:522
> >      if (o->_class == NPScriptObjectClass) {
> 
> Also broken like the other cases above.

I'll update the patch.
Comment 30 Carlos Garcia Campos 2016-10-31 01:47:26 PDT
Created attachment 293405 [details]
Updated patch
Comment 31 WebKit Commit Bot 2016-10-31 01:49:43 PDT
Attachment 293405 [details] did not pass style-queue:


ERROR: Source/WebCore/bridge/NP_jsobject.cpp:168:  _NPN_InvokeDefault is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:297:  _NPN_GetProperty is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:338:  _NPN_SetProperty is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:373:  _NPN_RemoveProperty is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:416:  _NPN_HasProperty is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:452:  _NPN_HasMethod is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:492:  _NPN_Enumerate is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:533:  _NPN_Construct is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 8 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 32 Build Bot 2016-10-31 02:39:10 PDT
Comment on attachment 293405 [details]
Updated patch

Attachment 293405 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2410139

New failing tests:
plugins/npruntime/npruntime-calls-with-null-npp.html
plugins/npruntime/npruntime-calls-with-null-npobject.html
Comment 33 Build Bot 2016-10-31 02:39:13 PDT
Created attachment 293408 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 34 Build Bot 2016-10-31 02:54:40 PDT
Comment on attachment 293405 [details]
Updated patch

Attachment 293405 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2410148

New failing tests:
plugins/npruntime/npruntime-calls-with-null-npp.html
plugins/npruntime/npruntime-calls-with-null-npobject.html
Comment 35 Build Bot 2016-10-31 02:54:43 PDT
Created attachment 293409 [details]
Archive of layout-test-results from ews116 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 36 Build Bot 2016-10-31 02:55:18 PDT
Comment on attachment 293405 [details]
Updated patch

Attachment 293405 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2410175

New failing tests:
plugins/npruntime/npruntime-calls-with-null-npobject.html
Comment 37 Build Bot 2016-10-31 02:55:21 PDT
Created attachment 293410 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 38 Carlos Garcia Campos 2016-10-31 04:35:08 PDT
I don't understand why plugins/npruntime/npruntime-calls-with-null-npp.html still fails in WebKit1, according to the results the methods are returning true in case of null NPP.
Comment 39 Michael Catanzaro 2017-08-30 21:55:55 PDT
Still happening as of 2.16.1:

Truncated backtrace:
Thread no. 1 (10 frames)
 #0 WebKit::NPN_InvokeDefault at /usr/src/debug/webkitgtk-2.16.1/Source/WebKit2/WebProcess/Plugins/Netscape/NetscapeBrowserFuncs.cpp:689
 #1 on_shell_signal at browser-plugin.c:179
 #2 ffi_call_unix64 at ../src/x86/unix64.S:76
 #3 ffi_call at ../src/x86/ffi64.c:525
 #4 g_cclosure_marshal_generic at gclosure.c:1490
 #9 on_signal_received at gdbusproxy.c:917
 #10 emit_signal_instance_in_idle_cb at gdbusconnection.c:3705
 #16 WTF::RunLoop::run at /usr/src/debug/webkitgtk-2.16.1/Source/WTF/wtf/glib/RunLoopGLib.cpp:94
 #17 WebKit::ChildProcessMain<WebKit::PluginProcess, WebKit::PluginProcessMain> at /usr/src/debug/webkitgtk-2.16.1/Source/WebKit2/Shared/unix/ChildProcessMain.h:61
 #19 _start
Comment 40 Michael Catanzaro 2017-11-10 07:09:55 PST
Still happening as of 2.18.2
Comment 41 Michael Catanzaro 2018-03-28 10:10:01 PDT
*** Bug 176304 has been marked as a duplicate of this bug. ***
Comment 42 Michael Catanzaro 2018-03-28 10:11:12 PDT
Still happening in 2.18.6
Comment 43 Michael Catanzaro 2018-03-28 10:17:16 PDT
Created attachment 336672 [details]
Newer backtrace

The backtrace is slightly different nowadays.