|Summary:||REGRESSION(r49365): typeof(xhr.responseText) != "string" in Windows|
|Product:||WebKit||Reporter:||Marshall Culpepper <mculpepper>|
|Severity:||Normal||CC:||ap, aroben, barraclough, ggaren, mjs, mrobinson, sfalken, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Marshall Culpepper 2009-12-30 09:51:18 PST
Hey guys.. this is a really odd one we noticed in the latest builds of the Win32/Cairo port, Brent Fulgham also verified this is happening in the official Windows port. Basically in an XHR response, the responseText is correctly filled with data, but the "typeof" the responseText won't correctly compare against the string "string", unless the typeof or the responseText itself is forced into a String context. To clarify, here are a few examples: "type="+typeof(xhr.responseText) // -> type=string typeof(xhr.responseText) == "string" // false, should be true String(typeof(xhr.responseText)) == "string" // true typeof(String(xhr.responseText)) == "string" // true var t = typeof(xhr.responseText); t == "string" // true .. weird right? tyepof(xhr.responseText).substr(0,5) == "string" // true It should be noted that performing any standard String operations on the responseText or the typeof seem to work fine. If you load this test case in WinLauncher, you should see all red text with typeof(responseText) == "string" ? false : http://arcaner.com/test/test.html
Comment 1 Alexey Proskuryakov 2009-12-30 17:58:34 PST
Comment 2 Martin Robinson 2009-12-31 19:07:22 PST
A little more information: we are only seeing this on the Windows builds. Neither the Mac build or the Linux GTK+ port show this behavior. I'm not sure how long this bug has been around, but I remember seeing it for at least a month. We can try to narrow it down a little.
Comment 3 Alexey Proskuryakov 2010-01-01 19:31:40 PST
I cannot reproduce this on Windows with nightly r52686. Please attach a complete reduced test case.
Comment 4 Martin Robinson 2010-01-05 10:48:59 PST
It could be that this is confined to the Win32 Cairo port. I'll double-check with the original reporter and update this ticket. Sorry for the confusion.
Comment 5 Alexey Proskuryakov 2010-01-06 16:07:40 PST
I can reproduce this now. I don't know what went wrong with my testing before. The only difference I'm aware of is that I was testing with XP, and am testing with Vista now - but that shouldn't affect behavior. I'm working on a fix.
Comment 6 Alexey Proskuryakov 2010-01-06 19:48:38 PST
The problem is due to the fact that jsString() and related functions are now inline, so JSString objects constructed from WebCore get a different virtual function table pointer. This breaks isJSString() function. I have added appropriate assertions to constructors, and saw them fire: ASSERT(vptr() == globalData->jsStringVPtr); The common way of telling the compiler to use the same vtable in DLL and EXE is to use __declspec(dllimport/dllexport). And this fixes the assertion indeed (that's equivalent to what we have on Mac OS X). But it doesn't fix the actual bug! Turns out that MSVC changes the vptr AFTER constructor exits, trying to fix bugs in certain kinds of code for programmers: <http://groups.google.com/group/microsoft.public.vc.language/msg/55cdcefeaf770212>. I couldn't find a way to disable this behavior. Some possible ways to fix this are: - don't construct inline (or only do that when building JSC); - fix vptr in jsString() after constructing the object; - check for type in some other way (maybe typeid().name, like dynamic_cast does? I'm not sure if that works without RTTI though, or if MSVC breaks typename identity across DLL boundaries, too). Suggestions are welcome.
Comment 7 Alexey Proskuryakov 2010-01-07 15:42:47 PST
Created attachment 46093 [details] proposed fix
Comment 8 WebKit Review Bot 2010-01-07 15:47:40 PST
Comment 9 Geoffrey Garen 2010-01-07 15:58:20 PST
Comment on attachment 46093 [details] proposed fix r=me
Comment 10 Alexey Proskuryakov 2010-01-07 16:17:32 PST
Committed <http://trac.webkit.org/changeset/52956>. This bug also affected PaceKeeper benchmark for Windows Safari, which now works for me. For the record, the issue I had with my initial testing was that I was using Safari 4.0.3 - after updating to 4.0.4 on that machine, I could reproduce.