REGRESSION (r206386): Xactimate Website Crashes @ com.apple.WebKit: WebKit::NPRuntimeObjectMap::convertJSValueToNPVariant + 255

[Chris Dumez]

Xactimate Website Crashes @ com.apple.WebKit: WebKit::NPRuntimeObjectMap::convertJSValueToNPVariant + 255:
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebKit              	0x00000001041ffe8c JSC::JSCell::isString() const + 12
1   com.apple.WebKit              	0x00000001041fd48e JSC::JSValue::isString() const + 62
2   com.apple.WebKit              	0x00000001041fd0e7 WebKit::NPRuntimeObjectMap::convertJSValueToNPVariant(JSC::ExecState*, JSC::JSValue, _NPVariant&) + 375
3   com.apple.WebKit              	0x00000001041e05b9 WebKit::NPJSObject::invoke(JSC::ExecState*, JSC::JSGlobalObject*, JSC::JSValue, _NPVariant const*, unsigned int, _NPVariant*) + 537
4   com.apple.WebKit              	0x00000001041e037b WebKit::NPJSObject::invoke(void*, _NPVariant const*, unsigned int, _NPVariant*) + 315
5   com.apple.WebKit              	0x00000001041e1b2b WebKit::NPJSObject::NP_Invoke(NPObject*, void*, _NPVariant const*, unsigned int, _NPVariant*) + 59
6   com.apple.WebKit              	0x00000001041e5cde WebKit::NPObjectMessageReceiver::invoke(WebKit::NPIdentifierData const&, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> const&, bool&, WebKit::NPVariantData&) + 398
7   com.apple.WebKit              	0x00000001041eac9c void IPC::callMemberFunctionImpl<WebKit::NPObjectMessageReceiver, void (WebKit::NPObjectMessageReceiver::*)(WebKit::NPIdentifierData const&, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> const&, bool&, WebKit::NPVariantData&), std::__1::tuple<WebKit::NPIdentifierData, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> >, 0ul, 1ul, std::__1::tuple<bool, WebKit::NPVariantData>, 0ul, 1ul>(WebKit::NPObjectMessageReceiver*, void (WebKit::NPObjectMessageReceiver::*)(WebKit::NPIdentifierData const&, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> const&, bool&, WebKit::NPVariantData&), std::__1::tuple<WebKit::NPIdentifierData, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> >&&, std::__1::tuple<bool, WebKit::NPVariantData>&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) + 284
8   com.apple.WebKit              	0x00000001041e9800 void IPC::callMemberFunction<WebKit::NPObjectMessageReceiver, void (WebKit::NPObjectMessageReceiver::*)(WebKit::NPIdentifierData const&, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> const&, bool&, WebKit::NPVariantData&), std::__1::tuple<WebKit::NPIdentifierData, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> >, std::__1::integer_sequence<unsigned long, 0ul, 1ul>, std::__1::tuple<bool, WebKit::NPVariantData>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(std::__1::tuple<WebKit::NPIdentifierData, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> >&&, std::__1::tuple<bool, WebKit::NPVariantData>&, WebKit::NPObjectMessageReceiver*, void (WebKit::NPObjectMessageReceiver::*)(WebKit::NPIdentifierData const&, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> const&, bool&, WebKit::NPVariantData&)) + 96
9   com.apple.WebKit              	0x00000001041e85f4 void IPC::handleMessage<Messages::NPObjectMessageReceiver::Invoke, WebKit::NPObjectMessageReceiver, void (WebKit::NPObjectMessageReceiver::*)(WebKit::NPIdentifierData const&, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> const&, bool&, WebKit::NPVariantData&)>(IPC::Decoder&, IPC::Encoder&, WebKit::NPObjectMessageReceiver*, void (WebKit::NPObjectMessageReceiver::*)(WebKit::NPIdentifierData const&, WTF::Vector<WebKit::NPVariantData, 0ul, WTF::CrashOnOverflow, 16ul> const&, bool&, WebKit::NPVariantData&)) + 388
10  com.apple.WebKit              	0x00000001041e7c91 WebKit::NPObjectMessageReceiver::didReceiveSyncNPObjectMessageReceiverMessage(IPC::Connection&, IPC::Decoder&, std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&) + 561
11  com.apple.WebKit              	0x00000001041f5d67 WebKit::NPRemoteObjectMap::didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&) + 103

[Chris Dumez]

<rdar://problem/31093005>

[Chris Dumez]

Created attachment 311791 [details]
Patch

[Chris Dumez]

Comment on attachment 311791 [details]
Patch

Clearing flags on attachment: 311791

Committed r217695: <http://trac.webkit.org/changeset/217695>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Mark Lam]

Comment on attachment 311791 [details]
Patch

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

> Source/WebKit2/WebProcess/Plugins/Netscape/NPJSObject.cpp:-316
> -    scope.clearException();

After thinking about this some more, I wonder if convertJSValueToNPVariant() can produce an exception too.  If so, you will need the above exception treatment here as well.  What do you think?

[Chris Dumez]

(In reply to Mark Lam from comment #5)
> Comment on attachment 311791 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=311791&action=review
> 
> > Source/WebKit2/WebProcess/Plugins/Netscape/NPJSObject.cpp:-316
> > -    scope.clearException();
> 
> After thinking about this some more, I wonder if convertJSValueToNPVariant()
> can produce an exception too.  If so, you will need the above exception
> treatment here as well.  What do you think?

I believe you are right. I believe convertJSValueToNPVariant() can indeed throw.

[Chris Dumez]

Created attachment 311834 [details]
Follow-up fix

[Mark Lam]

Comment on attachment 311834 [details]
Follow-up fix

r=me

[WebKit Commit Bot]

Comment on attachment 311834 [details]
Follow-up fix

Clearing flags on attachment: 311834

Committed r217729: <http://trac.webkit.org/changeset/217729>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.