|Summary:||REGRESSION: Mootools 1.2 Class inheritance broken in post-SquirrelFish merge|
|Product:||WebKit||Reporter:||Andrew Gleave <andrew>|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
Description Andrew Gleave 2008-05-31 16:26:51 PDT
Comment 1 Mark Rowe (bdash) 2008-05-31 17:08:30 PDT
If the problem is caused by a change in behaviour of WebKit, then it's definitely preferable to file a bug report in WebKit's bug tracker as it's often an unintentional change. I think we need a reduced test case to make headway on this issue.
Comment 2 Mark Rowe (bdash) 2008-05-31 18:04:18 PDT
Created attachment 21453 [details] Reduced test case The problem is that arguments.callee.caller is returning null when the function is called via Function.apply.
Comment 3 Maciej Stachowiak 2008-05-31 18:48:32 PDT
Created attachment 21454 [details] this test case shows that caller skips native function callers
Comment 4 Cameron Zwarich (cpst) 2008-06-07 23:15:49 PDT
The problem is that Machine::retrieveCaller() calls getCallerFunctionOffset(), which stops when it sees a null codeBlock. In that case, we want to check the prior call frame instead.
Comment 5 Cameron Zwarich (cpst) 2008-06-08 02:32:31 PDT
Comment 6 Cameron Zwarich (cpst) 2008-06-08 03:39:26 PDT
Interestingly enough, the patch I posted works on the two test cases but not the URL linked in the bug. I'll try to figure out the difference.
Comment 7 Cameron Zwarich (cpst) 2008-06-08 14:23:08 PDT
Created attachment 21581 [details] Reduction So, it turns out that the earlier reductions for a different bug. Here is a new reduction of the bug, working from the MooTools source. It can probably be reduced further, but I wanted to dump it on Bugzilla in case I have to do something else later. There is no immediate native caller that is the problem, and all of the callers of the functions leading up to the one that is the problem are the same in Safari 3.1 and ToT.
Comment 8 Cameron Zwarich (cpst) 2008-06-08 15:11:36 PDT
Sorry, when I said that ToT is the same as Safari 3.1 on all of those cases, I meant ToT with my patches. The call chain is as follows: initialize() in Class: Safari 3.1 and ToT agree that the caller is null. initialize() in ExampleClass.Inherited: Safari 3.1 says that the caller is initialize() in class, whereas ToT says that the caller is null. My patch fixes this, because this function is called by using apply in the body of initialize() in Class. self.parent() in Extends: Safari 3.1 says that the caller is initialize() in ExampleClass.Inherited, whereas both ToT and ToT with my patch say that the caller is null. Thus, the problem is not directly due to a native call. I'll try to reduce it further and figure out what's going on.
Comment 9 Cameron Zwarich (cpst) 2008-06-08 15:57:40 PDT
Created attachment 21582 [details] Further reduction Here is a further reduction of the bug. It has two cases to demonstrate that the native call messes up the later attempt to get a JS caller.
Comment 10 Cameron Zwarich (cpst) 2008-06-08 16:58:30 PDT
The problem is this code in Register* callerCallFrame = (*registerBase) + callerOffset; if (!callerCallFrame[Machine::CallerCodeBlock].u.codeBlock) // test for eval frame return false; It is testing whether the caller is an eval frame by testing whether the caller's caller has a codeBlock, but this also happens when the caller's caller is native code. Removing the code fixes the bug, but causes an assertion failure when the caller is eval code. The easiest fix is probably to add a CodeType field to CodeBlock.
Comment 11 Cameron Zwarich (cpst) 2008-06-08 17:19:24 PDT
Created attachment 21584 [details] Proposed patch Here's a fix. I won't put it up for review until I write some layout tests.
Comment 12 Cameron Zwarich (cpst) 2008-06-08 17:45:05 PDT
Created attachment 21585 [details] Proposed patch with test
Comment 13 Darin Adler 2008-06-08 17:48:26 PDT
Comment on attachment 21585 [details] Proposed patch with test r=me