Bug 17250 - instanceof Array test fails when using iframes
Summary: instanceof Array test fails when using iframes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC Windows XP
: P2 Major
Assignee: Nobody
URL:
Keywords: HasReduction
Depends on: 57333
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-08 22:46 PST by Sanjiv Jivan
Modified: 2011-03-29 10:47 PDT (History)
11 users (show)

See Also:


Attachments
child.html (234 bytes, text/html)
2009-01-13 16:35 PST, David Kilzer (:ddkilzer)
no flags Details
parent.html (test case) (207 bytes, text/html)
2009-01-13 16:36 PST, David Kilzer (:ddkilzer)
no flags Details
Patch (46.29 KB, patch)
2011-03-25 18:33 PDT, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (51.72 KB, patch)
2011-03-28 13:15 PDT, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (52.08 KB, patch)
2011-03-28 14:06 PDT, Oliver Hunt
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sanjiv Jivan 2008-02-08 22:46:41 PST
I have an iframe where I'm creating an Array with in the parent's context via new parent.Array(). I now call a function in the parent window passing this array. When the parent function receives this argument, it fails the instanceof test.

I understand that passing "new Array()" from the iframe to parent will cause the "instanceof Array " test to fail in the parent because the contexts of the two are different, but in this case I am creating a array from the parent context itself yet if fails the "instanceof Array" test. Below is a simple testcase illustrating the problem. This test runs fine on IE, Firefox and Opera but fails on Safari only.

parent.html
<html>
<head>
</head>
<body>
<script type="text/javascript">
    function isArray(arr) {
        alert(arr instanceof Array);
    }
</script>
</body>
<iframe src="child.html"></iframe>
</html>


child.html
<html>
<head>
</head>
<body>
 <script type="text/javascript">
     function test() {
         var arr = new parent.Array();
         parent.isArray(arr);
     }
 </script>

<button onclick="test()">Array Test</button>
</body>
</html>
Comment 1 David Kilzer (:ddkilzer) 2009-01-13 16:35:39 PST
Created attachment 26690 [details]
child.html
Comment 2 David Kilzer (:ddkilzer) 2009-01-13 16:36:23 PST
Created attachment 26691 [details]
parent.html (test case)
Comment 3 David Kilzer (:ddkilzer) 2009-01-13 16:38:12 PST
Confirmed with WebKit nightly build r39872.
Comment 4 David Kilzer (:ddkilzer) 2009-01-13 16:39:24 PST
(In reply to comment #3)
> Confirmed with WebKit nightly build r39872.

Confirmed the test case alerts "true" in Firefox 3, but alerts "false" in WebKit/Safari.
Comment 5 Matthew Lieder 2011-03-25 09:51:38 PDT
FYI, this is a problem in Firefox 4 now too (test case alerts "false")
Comment 6 Oliver Hunt 2011-03-25 18:33:25 PDT
Created attachment 87001 [details]
Patch
Comment 7 WebKit Review Bot 2011-03-25 18:35:09 PDT
Attachment 87001 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSObjectRef.cpp'..." exit_code: 1

Source/JavaScriptCore/runtime/JSCell.h:38:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 34 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Early Warning System Bot 2011-03-25 18:48:49 PDT
Attachment 87001 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8256011
Comment 9 Build Bot 2011-03-25 18:56:59 PDT
Attachment 87001 [details] did not build on win:
Build output: http://queues.webkit.org/results/8256017
Comment 10 Oliver Hunt 2011-03-28 13:15:59 PDT
Created attachment 87186 [details]
Patch
Comment 11 WebKit Review Bot 2011-03-28 13:19:11 PDT
Attachment 87186 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/JavaScriptCore/runtime/JSCell.h:38:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Build Bot 2011-03-28 13:56:18 PDT
Attachment 87186 [details] did not build on win:
Build output: http://queues.webkit.org/results/8273473
Comment 13 Oliver Hunt 2011-03-28 14:06:51 PDT
Created attachment 87200 [details]
Patch
Comment 14 WebKit Review Bot 2011-03-28 14:09:23 PDT
Attachment 87200 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/JavaScriptCore/runtime/JSCell.h:38:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 39 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Geoffrey Garen 2011-03-28 15:30:25 PDT
Comment on attachment 87200 [details]
Patch

r=me
Comment 16 Oliver Hunt 2011-03-28 16:40:25 PDT
Committed r82173
Comment 17 Adam Roben (:aroben) 2011-03-29 06:51:56 PDT
This caused assertion and test failures in run-javascriptcore-tests on Windows. Here's the assertion:

ASSERTION FAILED: isCell()

>	JavaScriptCore.dll!JSC::JSValue::asCell()  Line 559 + 0x30 bytes	C++
 	JavaScriptCore.dll!JSC::asObject(JSC::JSValue value={...})  Line 393 + 0x8 bytes	C++
 	JavaScriptCore.dll!JSC::asInternalFunction(JSC::JSValue value={...})  Line 65 + 0x12 bytes	C++
 	JavaScriptCore.dll!JSC::constructDate(JSC::ExecState * exec=0x012201a0, const JSC::ArgList & args={...})  Line 124 + 0x3e bytes	C++
 	JavaScriptCore.dll!JSObjectMakeDate(const OpaqueJSContext * ctx=0x012201a0, unsigned int argumentCount=1, const OpaqueJSValue * const * arguments=0x0012fdd8, const OpaqueJSValue * * exception=0x00000000)  Line 171 + 0x22 bytes	C++
 	testapi.exe!main(int argc=1, char * * argv=0x013c99d8)  Line 1304 + 0x19 bytes	C++
 	testapi.exe!__tmainCRTStartup()  Line 597 + 0x17 bytes	C
 	kernel32.dll!_BaseProcessStart@4()  + 0x23 bytes	

I'm rolling out this patch in bug 57333.
Comment 18 Adam Roben (:aroben) 2011-03-29 10:47:17 PDT
I guess this can be resolved again since Oliver fixed the Windows issues.