Bug 33057 – REGRESSION(r49365): typeof(xhr.responseText) != "string" in Windows

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Marshall Culpepper]

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

[Alexey Proskuryakov]

Just to clarify, did this work before? Can you check when exactly this broke (which is the latest nightly build where this works)?

Sounds like this could be related to recent JavaScript string refactoring work. I can't reproduce this with nightly r52591 on Mac OS X, and didn't try on Windows though.

[Martin Robinson]

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.

[Alexey Proskuryakov]

I cannot reproduce this on Windows with nightly r52686. Please attach a complete reduced test case.

[Martin Robinson]

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.

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

Created attachment 46093 [details]
proposed fix

[WebKit Review Bot]

Attachment 46093 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/cf/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/runtime/JSString.h:359:  More than one command on the same line  [whitespace/newline] [4]
JavaScriptCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:13:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:14:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:15:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 9

[Geoffrey Garen]

Comment on attachment 46093 [details]
proposed fix

r=me

[Alexey Proskuryakov]

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.