Bug 120398

Summary: REGRESSION(r154708): It broke all plugin tests on GTK and Qt WK1
Product: WebKit Reporter: Zoltan Arvai <zarvai>
Component: Tools / TestsAssignee: Zan Dobersek <zan>
Status: RESOLVED FIXED    
Severity: Normal CC: abrhm, allan.jensen, andersca, kadam, kling, ossy, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 79666, 120376    
Attachments:
Description Flags
Provisional patch
none
Patch none

Zoltan Arvai
Reported 2013-08-28 02:59:52 PDT
Various undefined type errors occurred with plugin tests after the patch, eg: FAIL document.getElementById('plugin').hasStream should be true (of type boolean). Was undefined (of type undefined). FAIL: typeof embed.testCallback should be function but instead is undefined. CONSOLE MESSAGE: line 19: TypeError: undefined is not a function (evaluating 'testPlugin.getURLNotify("javascript:12345", null, "callback")') CONSOLE MESSAGE: line 20: TypeError: undefined is not a function (evaluating 'plugin.testPassTestObject("npapiCallback", testObject)') CONSOLE MESSAGE: line 19: TypeError: undefined is not a function (evaluating 'embed.remember(obj)') CONSOLE MESSAGE: line 14: TypeError: undefined is not a function (evaluating 'plugin.testEnumerate(testObject, outArray)') FAIL plugin.testEvaluate('1') should be 1. Threw exception TypeError: undefined is not a function (evaluating 'plugin.testEvaluate('1')') .... - http://build.webkit.org/builders/Qt%20Linux%20Release/builds/62478 - http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20WK1/builds/3852
Attachments
Provisional patch (1.27 KB, patch)
2013-08-28 06:15 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Patch (2.11 KB, patch)
2013-08-28 06:37 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Zan Dobersek
Comment 1 2013-08-28 03:05:50 PDT
Looking at the offending patch: Index: /trunk/Source/WebCore/bridge/c/c_class.cpp =================================================================== --- /trunk/Source/WebCore/bridge/c/c_class.cpp (revision 154707) +++ /trunk/Source/WebCore/bridge/c/c_class.cpp (revision 154708) ... @@ -94,16 +91,16 @@ String name(propertyName.publicName()); - Field* aField = _fields.get(name.impl()); - if (aField) - return aField; - + if (Field* field = m_fields.get(name.impl())) + return field; + NPIdentifier ident = _NPN_GetStringIdentifier(name.ascii().data()); const CInstance* inst = static_cast<const CInstance*>(instance); NPObject* obj = inst->getObject(); - if (_isa->hasProperty && _isa->hasProperty(obj, ident)){ - aField = new CField(ident); // deleted in the CClass destructor - _fields.set(name.impl(), aField); + if (m_isa->hasProperty && m_isa->hasProperty(obj, ident)) { + OwnPtr<Field> field = adoptPtr(new CField(ident)); + m_fields.set(name.impl(), field.release()); } - return aField; + + return 0; } The last return seems to break the previous behavior.
Zoltan Arvai
Comment 2 2013-08-28 05:45:48 PDT
There were a similar changes in c_class.cpp 11 months ago: http://trac.webkit.org/changeset/129969 Probably Qt needs some similar changes this time, too. WebCore/bridge/qt/qt_class.cpp WebCore/bridge/qt/qt_class.h WebCore/bridge/qt/qt_instance.cpp
Zan Dobersek
Comment 3 2013-08-28 06:15:09 PDT
Created attachment 209871 [details] Provisional patch These changes fix the failures on the GTK port. Can somebody try them on Qt?
Zoltan Arvai
Comment 4 2013-08-28 06:21:36 PDT
(In reply to comment #3) > Created an attachment (id=209871) [details] > Provisional patch > > These changes fix the failures on the GTK port. Can somebody try them on Qt? Yes, I'm on it. :)
Zan Dobersek
Comment 5 2013-08-28 06:37:54 PDT
Created attachment 209874 [details] Patch
Zan Dobersek
Comment 6 2013-08-28 07:02:29 PDT
Comment on attachment 209874 [details] Patch Clearing flags on attachment: 209874 Committed r154741: <http://trac.webkit.org/changeset/154741>
Zan Dobersek
Comment 7 2013-08-28 07:02:36 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.