Bug 66994 - Implement Error.stack
: Implement Error.stack
Status: RESOLVED FIXED
: WebKit
JavaScriptCore
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
: 67010 68927 79029
:
  Show dependency treegraph
 
Reported: 2011-08-25 15:51 PST by
Modified: 2012-03-07 18:12 PST (History)


Attachments
proposed patch (21.56 KB, patch)
2011-08-25 22:38 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
sunspider test results showing no measurable speed regression (3.41 KB, text/plain)
2011-08-25 22:39 PST, Juan C. Montemayor
no flags Details
patch (22.90 KB, patch)
2011-08-26 00:40 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
patch w win and chromium fixes (23.99 KB, patch)
2011-08-26 09:39 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
patch with changes (26.29 KB, patch)
2011-08-26 15:55 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
Patch (26.74 KB, patch)
2011-09-12 19:14 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
proposed patch that will apply (26.81 KB, patch)
2011-09-13 06:37 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
patch (26.92 KB, patch)
2011-09-13 07:18 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
fixed patch for win-bot (26.87 KB, patch)
2011-09-13 13:00 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
updated patch (25.40 KB, patch)
2011-09-14 20:31 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
updated patch (25.03 KB, patch)
2011-09-27 07:42 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
fixing win bot (24.17 KB, patch)
2011-09-27 08:59 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
patch (25.06 KB, patch)
2011-09-27 09:58 PST, Juan C. Montemayor
no flags Review Patch | Details | Formatted Diff | Diff
Patch (40.96 KB, patch)
2012-01-31 15:02 PST, Oliver Hunt
no flags Review Patch | Details | Formatted Diff | Diff
Patch (44.72 KB, patch)
2012-02-16 13:57 PST, Oliver Hunt
barraclough: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-08-25 15:51:28 PST
Continuation of Bug 66571 - Keep track of topCallFrame for Stack traces
https://bugs.webkit.org/show_bug.cgi?id=66571

This is the second patch of two to implement stack traces in jsc. This patch utilizes topCallFrame to create a stack trace when an error is thrown. Users will also be able to use the stack() command in jsc to get stack traces.
------- Comment #1 From 2011-08-25 22:38:12 PST -------
Created an attachment (id=105313) [details]
proposed patch
------- Comment #2 From 2011-08-25 22:39:27 PST -------
Created an attachment (id=105314) [details]
sunspider test results showing no measurable speed regression
------- Comment #3 From 2011-08-25 23:11:16 PST -------
(From update of attachment 105313 [details])
Attachment 105313 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9508831

New failing tests:
fast/js/stack-trace.html
------- Comment #4 From 2011-08-26 00:40:02 PST -------
Created an attachment (id=105322) [details]
patch

why don't I get the results of the chromium bot? Oh well.. This patch will attempt to pass all tests...
------- Comment #5 From 2011-08-26 01:02:30 PST -------
(From update of attachment 105322 [details])
Attachment 105322 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9512846

New failing tests:
fast/js/stack-trace.html
------- Comment #6 From 2011-08-26 07:57:11 PST -------
(In reply to comment #4)
> Created an attachment (id=105322) [details] [details]
> patch
> 
> why don't I get the results of the chromium bot? Oh well.. This patch will attempt to pass all tests...

I would just add the test to the chrome skip list, it's probably the easiest way.
------- Comment #7 From 2011-08-26 09:39:40 PST -------
Created an attachment (id=105364) [details]
patch w win and chromium fixes
------- Comment #8 From 2011-08-26 10:41:08 PST -------
(From update of attachment 105364 [details])
Please return an array rather than a string.  There are plenty of places where having programatic knowledge of the stack trace would be useful (say the inspector), by creating a single string you're essentially requiring users to manually parse the string you just built up back into an array.
------- Comment #9 From 2011-08-26 11:13:51 PST -------
(From update of attachment 105364 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=105364&action=review

Have included a number of suggestions and questions

> Source/JavaScriptCore/interpreter/Interpreter.cpp:756
> +const UString Interpreter::getStackTrace(JSGlobalData* globalData, int line)

void Interpreter::getStackTrace(JSGlobalData& globalData, int line, Vector<TraceInfo>& result)

> Source/JavaScriptCore/interpreter/Interpreter.cpp:768
> +        while (callFrame && callFrame != CallFrame::noCaller() && !callFrame->codeBlock())
> +            callFrame = callFrame->callerFrame()->removeHostCallFrameFlag();

This has the effect of removing native calls from the stack -- we may want to show these, but currently it's not necessary

> Source/JavaScriptCore/interpreter/Interpreter.cpp:775
> +        trace += getTraceLine(callFrame, callFrame->codeBlock()->codeType(), sourceURL, line, i);

result.append(TraceInfo(globalData, callFrame->codeBlock()->ownerExecutable(), line));

> Source/JavaScriptCore/interpreter/Interpreter.cpp:777
> +        getCallerLine(globalData, callFrame, line);

If you're called by a host function, this will not update line, so your next iteration may produce a bogus line number, what happens if I do:
> var o = {}; o.toString = function() {  throw {}; }
function () {  throw {}; }
> print(o)

> Source/JavaScriptCore/interpreter/Interpreter.cpp:802
> +            UString stackTrace = getStackTrace(globalData, codeBlock->lineNumberForBytecodeOffset(bytecodeOffset));

Vector<TraceInfo> trace;
getStackTrace(*globalData, codeBlock->lineNumberForBytecodeOffset(bytecodeOffset), trace);
addErrorInfo(callFrame, exception, codeBlock->lineNumberForBytecodeOffset(bytecodeOffset), codeBlock->ownerExecutable()->source(), stackTrace);

> Source/JavaScriptCore/interpreter/Interpreter.h:131
> +        static const UString getStackTrace(JSGlobalData*, int);

I don't like this string return, i think you're better doing:

struct TraceInfo {
    Strong<Executable> code;
    int lineNumber;
}

static void getStackTrace(JSGlobalData&, Vector<TraceInfo>& result);

> Source/JavaScriptCore/runtime/Error.cpp:119
> +JSObject* addErrorInfo(JSGlobalData* globalData, JSObject* error, int line, const SourceCode& source, const UString& stackTrace)

JSObject* addErrorInfo(JSGlobalData* globalData, JSObject* error, int line, const SourceCode& source, const Vector<TraceInfo>& stackTrace)

> Source/JavaScriptCore/runtime/Error.cpp:131
> +        error->putWithAttributes(globalData, globalData->propertyNames->stack, jsString(globalData, stackTrace), ReadOnly | DontDelete);

JSArray* stackTrace = JSArray::createEmpty(*globalData, globalObject);
for (unsigned i = 0; i < stackTrace.size(); i++) { JSString* string = jsString(globalData, formatTraceLine(stackTrace[i])); stackTrace->push(*globalData, string); }
error->putWithAttributes(globalData, globalData->propertyNames->stack, stackTrace, ReadOnly | DontDelete);
------- Comment #10 From 2011-08-26 15:50:30 PST -------
> > Source/JavaScriptCore/interpreter/Interpreter.cpp:777
> > +        getCallerLine(globalData, callFrame, line);
> 
> If you're called by a host function, this will not update line, so your next iteration may produce a bogus line number

If a line number is not available for the callFrame, then a line number will not be printed out.
------- Comment #11 From 2011-08-26 15:55:02 PST -------
Created an attachment (id=105417) [details]
patch with changes
------- Comment #12 From 2011-08-26 16:44:39 PST -------
(From update of attachment 105417 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=105417&action=review

> Source/JavaScriptCore/interpreter/Interpreter.cpp:742
> +static void updateSourceURL(CallFrame* callFrame, UString& sourceURL) 

Just return the sourceURL -- const UString& is your friend.

Need to check callframe for host flag

> Source/JavaScriptCore/interpreter/Interpreter.cpp:767
> +        while (callFrame && callFrame != CallFrame::noCaller() && !callFrame->codeBlock())
> +            callFrame = callFrame->callerFrame()->removeHostCallFrameFlag();

Remove these lines

> Source/JavaScriptCore/interpreter/Interpreter.cpp:774
> +        traceLevel = getTraceLine(callFrame, callFrame->codeBlock()->codeType(), sourceURL, line).impl();

remove this as it won't be necessary

> Source/JavaScriptCore/interpreter/Interpreter.cpp:776
> +        StackTraceLevel s = {Strong<ScriptExecutable>(*globalData, callFrame->codeBlock()->ownerExecutable()), line, callFrame->codeBlock()->codeType(), sourceURL, traceLevel};

Strong<ScriptExecutable> -> Strong<Executable>

Make a helper function to return the correct value for your "code type" enum from the callFrame, eg.
if (callFrame->hasHostCa...) return StackFrameTypeNative;
switch (callFrame->codeBlock()->codeType()) {
   case Blah: return StackFrameTypeBlah;
   ...
}

> Source/JavaScriptCore/interpreter/Interpreter.h:66
> +    struct StackTraceLevel {

rename this to StackFrame

> Source/JavaScriptCore/interpreter/Interpreter.h:67
> +        Strong<ScriptExecutable> executable;

Make this an ordinary Strong<Executable>
and add
Strong<JSObject> callee

> Source/JavaScriptCore/interpreter/Interpreter.h:69
> +        int codeType;

Make your own enum here

> Source/JavaScriptCore/interpreter/Interpreter.h:71
> +        UString stackLevelString;

I don't think this is necessary -- the Vector<> implies the level you're at

> Source/JavaScriptCore/interpreter/Interpreter.h:140
> +        static const UString getStackTrace(JSGlobalData*, int);

die!!!!!1111!!!!1111one!

Might be worth moving the prettyprint function to StackFrame

> Source/JavaScriptCore/runtime/CommonIdentifiers.h:64
> +    macro(stack) \

change this to jscStack

> Source/JavaScriptCore/runtime/Error.cpp:134
> +            JSString* string = jsString(globalData, stackTrace[i].stackLevelString); 

stackTrace[i].toString() or whatever
------- Comment #13 From 2011-09-12 19:14:43 PST -------
Created an attachment (id=107128) [details]
Patch
------- Comment #14 From 2011-09-12 21:28:51 PST -------
It seems like the current implementation will potentially leak function names cross origin.  This should probably be avoided.
------- Comment #15 From 2011-09-13 06:37:41 PST -------
Created an attachment (id=107169) [details]
proposed patch that will apply

Sam, I'm going to try and find you on #webkit so we can talk about this
------- Comment #16 From 2011-09-13 06:53:11 PST -------
(From update of attachment 107169 [details])
Attachment 107169 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9661100
------- Comment #17 From 2011-09-13 07:18:44 PST -------
Created an attachment (id=107174) [details]
patch

This will hopefully fix the build errors in qt et al.
------- Comment #18 From 2011-09-13 13:00:01 PST -------
Created an attachment (id=107215) [details]
fixed patch for win-bot
------- Comment #19 From 2011-09-14 10:51:40 PST -------
(In reply to comment #14)
> It seems like the current implementation will potentially leak function names cross origin.  This should probably be avoided.

How does this compare to the leak from walking the call stack with function.caller?  Afaict if you were able to call a function (directly or indirectly) you must have some origin permissions.  Should we maybe restrict .caller if the global objects don't match?  It seems sufficiently edge case to probably be safe?

That said I did just realize something more significant that the origin bypass -> isolated worlds aren't sufficiently isolated: they share a global data, so a change that triggers synchronous JS by code in an isolated world (changing text or something with a world containing a change handler in an isolated world).  If such case should be created it may be possible to create a stack trace that leaked the presence of a particular extension, etc.
------- Comment #20 From 2011-09-14 14:18:51 PST -------
(In reply to comment #19)
> (In reply to comment #14)
> > It seems like the current implementation will potentially leak function names cross origin.  This should probably be avoided.
> 
> How does this compare to the leak from walking the call stack with function.caller?  Afaict if you were able to call a function (directly or indirectly) you must have some origin permissions.  Should we maybe restrict .caller if the global objects don't match?  It seems sufficiently edge case to probably be safe?

I think you are right, I retract my comment/objection.
------- Comment #21 From 2011-09-14 15:10:58 PST -------
(From update of attachment 107215 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=107215&action=review

r- due to the problem with printStack, but that's it.

> LayoutTests/fast/js/stack-trace-expected.txt:9
> +    0   normalInner at file:///Users/jmont/webkit/OpenSource/LayoutTests/fast/js/script-tests/stack-trace.js:21
> +    1   normalOuter at file:///Users/jmont/webkit/OpenSource/LayoutTests/fast/js/script-tests/stack-trace.js:20
> +    2   at file:///Users/jmont/webkit/OpenSource/LayoutTests/fast/js/script-tests/stack-trace.js:26

These will lead to sadness

> LayoutTests/fast/js/script-tests/stack-trace.js:9
> +		debug("    " + i + "   " + stackTrace[level]);

I think you want to do stackTrace[level].replace(location, "") here to make these tests happier
------- Comment #22 From 2011-09-14 20:31:26 PST -------
Created an attachment (id=107446) [details]
updated patch
------- Comment #23 From 2011-09-14 20:34:13 PST -------
(In reply to comment #21)
> These will lead to sadness

good catch. I fixed these with the new patch.

Also, although I am fairly certain this patch will work with the win-bot, I never confirmed this since the win-bot has been backed up lately.
------- Comment #24 From 2011-09-23 18:14:13 PST -------
(From update of attachment 107446 [details])
r=me, sorry i missed the updated patch going up
------- Comment #25 From 2011-09-23 18:17:50 PST -------
(From update of attachment 107446 [details])
Rejecting attachment 107446 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
file LayoutTests/fast/js/stack-trace.html
patching file LayoutTests/fast/js/script-tests/exception-properties.js
patching file LayoutTests/fast/js/script-tests/stack-trace.js
patching file LayoutTests/platform/chromium/test_expectations.txt
Hunk #1 FAILED at 477.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Oliver Hunt', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/9839128
------- Comment #26 From 2011-09-24 06:55:51 PST -------
(In reply to comment #25)
> (From update of attachment 107446 [details] [details])
> Rejecting attachment 107446 [details] [details] from commit-queue.
> 
> Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2
> 
> Last 500 characters of output:
> file LayoutTests/fast/js/stack-trace.html
> patching file LayoutTests/fast/js/script-tests/exception-properties.js
> patching file LayoutTests/fast/js/script-tests/stack-trace.js
> patching file LayoutTests/platform/chromium/test_expectations.txt
> Hunk #1 FAILED at 477.
> 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej
> 
> Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Oliver Hunt', u'--force']" exit_code: 1
> 
> Full output: http://queues.webkit.org/results/9839128

Seems like a lot of time passed since the patch upload till now, hehe. I'll upload an updated patch later tonight.
------- Comment #27 From 2011-09-27 07:42:10 PST -------
Created an attachment (id=108847) [details]
updated patch

updated so that patch will succeed. Oliver, I'll send you an email this time to remind you to r+
------- Comment #28 From 2011-09-27 08:59:47 PST -------
Created an attachment (id=108854) [details]
fixing win bot

please disregard this patch, i want to see what win-bot tells me it needs.
------- Comment #29 From 2011-09-27 09:11:27 PST -------
(From update of attachment 108854 [details])
Looks like your patch has lost the changes to JavaScriptCore.def
------- Comment #30 From 2011-09-27 09:13:32 PST -------
(In reply to comment #29)
> (From update of attachment 108854 [details] [details])
> Looks like your patch has lost the changes to JavaScriptCore.def

Yes, I don't have a windows machine to test on, so I want the bot to tell me what it wants in JavaScriptCore.def
------- Comment #31 From 2011-09-27 09:17:17 PST -------
(In reply to comment #30)
> (In reply to comment #29)
> > (From update of attachment 108854 [details] [details] [details])
> > Looks like your patch has lost the changes to JavaScriptCore.def
> 
> Yes, I don't have a windows machine to test on, so I want the bot to tell me what it wants in JavaScriptCore.def

Is there a better way to to this?
------- Comment #32 From 2011-09-27 09:37:30 PST -------
(In reply to comment #31)
> (In reply to comment #30)
> > (In reply to comment #29)
> > > (From update of attachment 108854 [details] [details] [details] [details])
> > > Looks like your patch has lost the changes to JavaScriptCore.def
> > 
> > Yes, I don't have a windows machine to test on, so I want the bot to tell me what it wants in JavaScriptCore.def
> 
> Is there a better way to to this?

Historically my "better way" has been to wish for a faster EWS bot :D
------- Comment #33 From 2011-09-27 09:58:25 PST -------
Created an attachment (id=108861) [details]
patch

this should be it
------- Comment #34 From 2011-09-27 10:58:02 PST -------
(From update of attachment 108861 [details])
Clearing flags on attachment: 108861

Committed r96131: <http://trac.webkit.org/changeset/96131>
------- Comment #35 From 2011-09-27 10:58:09 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #36 From 2011-09-27 11:02:30 PST -------
Yay!
------- Comment #37 From 2011-09-27 13:12:17 PST -------
(In reply to comment #34)
> (From update of attachment 108861 [details] [details])
> Clearing flags on attachment: 108861
> 
> Committed r96131: <http://trac.webkit.org/changeset/96131>

It made many tests crash on all platform:
GTK: http://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release/builds/17784
Qt: http://build.webkit.org/builders/Qt%20Linux%20Release/builds/37937
Leopard: 
http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Tests%29/builds/33790
...
------- Comment #38 From 2011-09-27 13:17:25 PST -------
Rolled out by http://trac.webkit.org/changeset/96146
------- Comment #39 From 2011-09-27 13:20:17 PST -------
(In reply to comment #38)
> Rolled out by http://trac.webkit.org/changeset/96146

It seems the problem occured only on 32 bit platoforms.
------- Comment #40 From 2011-09-27 13:21:27 PST -------
(In reply to comment #39)
> (In reply to comment #38)
> > Rolled out by http://trac.webkit.org/changeset/96146
> 
> It seems the problem occured only on 32 bit platoforms.

Will investigate and roll out a patch (hopefully later today).
------- Comment #41 From 2012-01-31 15:02:17 PST -------
Created an attachment (id=124830) [details]
Patch
------- Comment #42 From 2012-01-31 15:25:48 PST -------
Committed r106407
------- Comment #43 From 2012-02-01 00:41:01 PST -------
(In reply to comment #42)
> Committed r106407

Reopen, because it made many test crash on 32 bit platforms again:
- Qt: http://build.webkit.org/results/Qt%20Linux%20Release/r106434%20%2842748%29/results.html
- GTK: http://build.webkit.org/results/GTK%20Linux%2032-bit%20Release/r106432%20%2821296%29/results.html

Could you fix it?
------- Comment #44 From 2012-02-01 02:06:59 PST -------
I tried to ping you on IRC and here, but it's night now.

I rolled it out again, because crash is always serious bug: http://trac.webkit.org/changeset/106454 (I had to solve conflicts with http://trac.webkit.org/changeset/106414 and http://trac.webkit.org/changeset/106433 manually.)
------- Comment #45 From 2012-02-16 13:57:01 PST -------
Created an attachment (id=127437) [details]
Patch
------- Comment #46 From 2012-02-16 14:36:09 PST -------
(From update of attachment 127437 [details])
Stack tracing should not really be in interpreter.cpp, but of course nor should the rest of the exception handling code.  Really should move this out some time.
------- Comment #47 From 2012-02-16 14:39:01 PST -------
Committed r107980: <http://trac.webkit.org/changeset/107980>
------- Comment #48 From 2012-02-16 14:44:40 PST -------
Seems like a feature possibly worth blogging about. :)
------- Comment #49 From 2012-02-16 15:49:30 PST -------
Sorry for being late to the party.

What other engine does this match?

I agree that having a structure is better than a formatted string but having an array of strings does not seem good to me. Either go all the way where the array contains objects with lineNumber, url etc or stick with what other browsers do, which is a single formatted string.
------- Comment #50 From 2012-02-16 15:54:37 PST -------
(In reply to comment #49)
> Sorry for being late to the party.
> 
> What other engine does this match?
> 
> I agree that having a structure is better than a formatted string but having an array of strings does not seem good to me. Either go all the way where the array contains objects with lineNumber, url etc or stick with what other browsers do, which is a single formatted string.

The other engines don't agree on formatting of the string, so matching other browsers isn't something that is possible.  I'm unaware of any compatibility concerns involving stack, and implicit toString on an array will produce something perfectly sensible, so i'm not sure what there is to gain from changing this.
------- Comment #51 From 2012-02-16 16:08:53 PST -------
(In reply to comment #50)
> (In reply to comment #49)
> > Sorry for being late to the party.
> > 
> > What other engine does this match?
> > 
> > I agree that having a structure is better than a formatted string but having an array of strings does not seem good to me. Either go all the way where the array contains objects with lineNumber, url etc or stick with what other browsers do, which is a single formatted string.
> 
> The other engines don't agree on formatting of the string, so matching other browsers isn't something that is possible.  I'm unaware of any compatibility concerns involving stack, and implicit toString on an array will produce something perfectly sensible, so i'm not sure what there is to gain from changing this.

What is the benefit of the Array if the individual items have to be parsed anyway?

Matching V8 would at least have simplified the tests and 4 different formats are better than 5.
------- Comment #52 From 2012-02-16 16:13:01 PST -------
(In reply to comment #51)
> (In reply to comment #50)
> > (In reply to comment #49)
> > > Sorry for being late to the party.
> > > 
> > > What other engine does this match?
> > > 
> > > I agree that having a structure is better than a formatted string but having an array of strings does not seem good to me. Either go all the way where the array contains objects with lineNumber, url etc or stick with what other browsers do, which is a single formatted string.
> > 
> > The other engines don't agree on formatting of the string, so matching other browsers isn't something that is possible.  I'm unaware of any compatibility concerns involving stack, and implicit toString on an array will produce something perfectly sensible, so i'm not sure what there is to gain from changing this.
> 
> What is the benefit of the Array if the individual items have to be parsed anyway?
> 
> Matching V8 would at least have simplified the tests and 4 different formats are better than 5.

Step one in every use of single string stack traces that i've seen is a split() to get an array of strings, given this is purely a debug feature doing this ahead of time seems sensible.  I would move to an array of custom descriptor objects first rather than a single string if it weren't for the need to then implement a bunch of toString methods on those objects.  I'm kind of leaning in that way anyway (as it would make built-in vs. js functions vs. program code vs. eval code easier to distinguish)
------- Comment #53 From 2012-02-16 22:52:52 PST -------
(In reply to comment #47)
> Committed r107980: <http://trac.webkit.org/changeset/107980>

It broke 32 bit platforms again and again. :(( And you left the tree broken of course ... It isn't a fairplay thing. Please don't break other platforms again and again and leave them broken deliberately.

I tried to ping you on IRC again when I noticed ... but you didn't answer, so I had to rollout the patch again - http://trac.webkit.org/changeset/108036
(Otherwise you should have watch the bots and do the rollout yourself.)

I can understand if you can't test on a 32 bit machine. But why don't you ask for a help to test your patch before landing? I willingly help you in testing if you ask me. Maybe Zoltán or Gábor can help you to debug this bug on 32 bit Qt platform.
------- Comment #54 From 2012-02-17 11:47:36 PST -------
(In reply to comment #53)
> (In reply to comment #47)
> > Committed r107980: <http://trac.webkit.org/changeset/107980>
> 
> It broke 32 bit platforms again and again. :(( And you left the tree broken of course ... It isn't a fairplay thing. Please don't break other platforms again and again and leave them broken deliberately.
> 
> I tried to ping you on IRC again when I noticed ... but you didn't answer, so I had to rollout the patch again - http://trac.webkit.org/changeset/108036
> (Otherwise you should have watch the bots and do the rollout yourself.)
> 
> I can understand if you can't test on a 32 bit machine. But why don't you ask for a help to test your patch before landing? I willingly help you in testing if you ask me. Maybe Zoltán or Gábor can help you to debug this bug on 32 bit Qt platform.

I tested on 32 bit, and I waited for the qt bots to go green.

If the qt bots going green != the qt bots working then there's a problem.
------- Comment #55 From 2012-02-17 13:18:53 PST -------
Committed r108112: <http://trac.webkit.org/changeset/108112>
------- Comment #56 From 2012-03-07 18:12:36 PST -------
*** Bug 13646 has been marked as a duplicate of this bug. ***