[rfc][icedtea-web] rewert of changset 757 "Simplify IcedTeaScriptableJavaObject"
Jiri Vanek
jvanek at redhat.com
Fri Jan 17 06:06:42 PST 2014
On 01/16/2014 05:40 PM, Andrew Azores wrote:
> On 01/14/2014 10:48 AM, Jiri Vanek wrote:
>> Unluckily the regressions caused by (otherwise good)
>> http://icedtea.classpath.org/hg/icedtea-web/rev/ee92f55c69a3
>>
>> +2013-06-21 Adam Domurad <adomurad at redhat.com>
>> +
>> + * plugin/icedteanp/IcedTeaScriptablePluginObject.cc: Simplify
>> + IcedTeaScriptableJavaObject
>> + * plugin/icedteanp/IcedTeaScriptablePluginObject.h: Same
>>
>> were never successfully fixed, and I gave up on it :(
>>
>> This is revert of patch in tip. Please note, that this is *manual* revert of patch.
>> When I did it , i was just 50% sure what I done. But the result of selected set of reproducers
>> have persuaded me that I did it correctly. (three "r" attachments)
>>
>> So (any) reviwer, please (and sorry!), be double careful!-)
>>
>> J.
>
> Well, the patch generally looks okay, and your results are convincing. There seems to be no affect
> for me running the tests in Firefox, but at least reverting the changes hasn't made anything worse
> as far as I can tell. There is this that I pointed out over IRC and will repeat here though:
>
> // If object is an array and requested "method" may be a number, check for it first
> - if ( scriptable_object->is_object_array ||
> + if ( !((IcedTeaScriptableJavaObject*) npobj)->isArray() ||
> (browser_functions.intfromidentifier(name_id) < 0))
>
> it seems to me like the value of this condition has changed. But, you are restoring it back to how
> it used to be, so perhaps this is actually what the regression was in the first place. Like I said,
> it seems to make no difference in my testing, but perhaps it would be worth it if you checked the
> results of removing the negation in the condition as well. Most of the rest of the patch seems to be
> simple refactoring but this appears to actually change some behaviour.
>
> Thanks,
>
Well looks like you nailed it.
The minor one line fix looks like it have fixed the issue. Feel free to push.
J.
ps:
// If object is an array and requested "method" may be a number, check for it first
- if ( scriptable_object->is_object_array ||
+ if ( !scriptable_object->is_object_array ||
(browser_functions.intfromidentifier(name_id) < 0))
{
ook?
-------------- next part --------------
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_double_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_double_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_double_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_double_Test - epiphany
FAILED: AppletJSToJFuncParam_StringIntMixed_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: StringIntMixedParam [test, 123], but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_StringIntMixed_Test - opera
FAILED: AppletJSToJFuncParam_StringIntMixed_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: StringIntMixedParam [test, 123], but it didnt.
FAILED: AppletJSToJFuncParam_StringIntMixed_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: StringIntMixedParam [test, 123], but it didnt.
FAILED: AppletJSToJFuncParam_booleanFalseStr_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: booleanParam true, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_booleanFalseStr_Test - opera
FAILED: AppletJSToJFuncParam_booleanFalseStr_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: booleanParam true, but it didnt.
FAILED: AppletJSToJFuncParam_booleanFalseStr_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: booleanParam true, but it didnt.
FAILED: AppletJSToJFuncParam_Byte_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: ByteParam 10, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Byte_Test - opera
FAILED: AppletJSToJFuncParam_Byte_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: ByteParam 10, but it didnt.
FAILED: AppletJSToJFuncParam_Byte_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: ByteParam 10, but it didnt.
FAILED: AppletJSToJFuncParam_charArray_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: charArrayParam [a, b, c], but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_charArray_Test - opera
FAILED: AppletJSToJFuncParam_charArray_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: charArrayParam [a, b, c], but it didnt.
FAILED: AppletJSToJFuncParam_charArray_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: charArrayParam [a, b, c], but it didnt.
FAILED: AppletJSToJFuncParam_BooleanFalseStr_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: BooleanParam true, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_BooleanFalseStr_Test - opera
FAILED: AppletJSToJFuncParam_BooleanFalseStr_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: BooleanParam true, but it didnt.
FAILED: AppletJSToJFuncParam_BooleanFalseStr_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: BooleanParam true, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_long_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_long_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_long_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_long_Test - epiphany
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_float_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_float_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_float_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_float_Test - epiphany
FAILED: AppletJSToJFuncParam_DummyObjectArray_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: DummyObjectArrayParam [Dummy1, Dummy2], but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_DummyObjectArray_Test - opera
FAILED: AppletJSToJFuncParam_DummyObjectArray_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: DummyObjectArrayParam [Dummy1, Dummy2], but it didnt.
FAILED: AppletJSToJFuncParam_DummyObjectArray_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: DummyObjectArrayParam [Dummy1, Dummy2], but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Character_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Character_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Character_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Character_Test - epiphany
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Double_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Double_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Double_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Double_Test - epiphany
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_String_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_String_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_String_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_String_Test - epiphany
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_char_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_char_Test - opera
to Passed: JSToJFuncParamTest.AppletJSToJFuncParam_char_Test - midori
be Passed: JSToJFuncParamTest.AppletJSToJFuncParam_char_Test - epiphany
FAILED: AppletJSToJFuncParam_Long_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: LongParam 10000, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Long_Test - opera
FAILED: AppletJSToJFuncParam_Long_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: LongParam 10000, but it didnt.
FAILED: AppletJSToJFuncParam_Long_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: LongParam 10000, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_boolean_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_boolean_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_boolean_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_boolean_Test - epiphany
FAILED: AppletJSToJFuncParam_Float_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: FloatParam 1.1, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Float_Test - opera
FAILED: AppletJSToJFuncParam_Float_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: FloatParam 1.1, but it didnt.
FAILED: AppletJSToJFuncParam_Float_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: FloatParam 1.1, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_byte_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_byte_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_byte_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_byte_Test - epiphany
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Boolean_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Boolean_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Boolean_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Boolean_Test - epiphany
FAILED: AppletJSToJFuncParam_Integer_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: IntegerParam 1, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_Integer_Test - opera
FAILED: AppletJSToJFuncParam_Integer_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: IntegerParam 1, but it didnt.
FAILED: AppletJSToJFuncParam_Integer_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: IntegerParam 1, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_int_Test - chromium-browser
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_int_Test - opera
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_int_Test - midori
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_int_Test - epiphany
FAILED: AppletJSToJFuncParam_JSObject_Test - chromium-browser(JSToJFuncParamTest) JSToJFuncParam: the output should include: JSObjectParam 100, red, but it didnt.
Passed: JSToJFuncParamTest.AppletJSToJFuncParam_JSObject_Test - opera
FAILED: AppletJSToJFuncParam_JSObject_Test - midori(JSToJFuncParamTest) JSToJFuncParam: the output should include: JSObjectParam 100, red, but it didnt.
FAILED: AppletJSToJFuncParam_JSObject_Test - epiphany(JSToJFuncParamTest) JSToJFuncParam: the output should include: JSObjectParam 100, red, but it didnt.
Total tests run: 84; From those : 0 known to fail
Test known to fail: passed: 0; failed: 0; ignored: 0
Test results: passed: 54; failed: 30; ignored: 0
More information about the distro-pkg-dev
mailing list