REGRESSION(r154708): It broke all plugin tests on GTK and Qt WK1

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Zoltan Arvai]

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

[Zan Dobersek]

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]

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]

Created attachment 209871 [details]
Provisional patch

These changes fix the failures on the GTK port. Can somebody try them on Qt?

[Zoltan Arvai]

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

Created attachment 209874 [details]
Patch

[Zan Dobersek]

Comment on attachment 209874 [details]
Patch

Clearing flags on attachment: 209874

Committed r154741: <http://trac.webkit.org/changeset/154741>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.