When processing global vars initialization account for the case of splitted...

When processing global vars initialization account for the case of splitted
global object (using hidden prototypes):

1) setters might be not on the global object itself, but on its prototypes;
2) if property on one of prototypes is readonly, we could shadow it.

Committed: http://code.google.com/p/v8/source/detail?r=3362

[antonm, 2009-11-24 14:04:59]

That should fix

LayoutTests/fast/dom/Window/window-property-shadowing-name.html

For some reason run_webkit_tests.sh tells me it fixes two more tests:

That fixes 3 layout tests:
------------------------------------------------------------------------------

Expected to fail, but passed (2):
  LayoutTests/fast/dom/java-applet-calls.html
  LayoutTests/fast/xmlhttprequest/xmlhttprequest-nonexistent-file.html

Expected to timeout, but passed (1):
  LayoutTests/fast/dom/Window/window-property-shadowing-name.html
------------------------------------------------------------------------------

but I think it's a noise.

[Mads Ager (chromium), 2009-11-24 15:06:00]

Unsure about read_only interceptor property on a hidden prototype.  Should we
treat that the same as a normal read_only property on a hidden prototype?

http://codereview.chromium.org/434035/diff/1/2
File src/runtime.cc (right):

http://codereview.chromium.org/434035/diff/1/2#newcode800
src/runtime.cc:800: // If we found readonly property below the global object
Developers do not always agree on above/below. Can we use the formulation: If we
found readonly property on a hidden prototype...?

http://codereview.chromium.org/434035/diff/1/2#newcode822
src/runtime.cc:822: return ThrowRedeclarationError("const", name);
This seems inconsistent with the case above.  Should we shadow if there is an
interceptor readonly property on a hidden prototype?  That is what we do for a
normal readonly property on a hidden prototype.

http://codereview.chromium.org/434035/diff/1/2#newcode849
src/runtime.cc:849: // ASSERT(!lookup.IsProperty());
Code in comment.

[antonm, 2009-11-25 00:03:42]

Thanks a lot for comments, Mads.

http://codereview.chromium.org/434035/diff/1/2
File src/runtime.cc (right):

http://codereview.chromium.org/434035/diff/1/2#newcode800
src/runtime.cc:800: // If we found readonly property below the global object
On 2009/11/24 15:06:00, Mads Ager wrote:
> Developers do not always agree on above/below. Can we use the formulation: If
we
> found readonly property on a hidden prototype...? 

Sure.  Done.

http://codereview.chromium.org/434035/diff/1/2#newcode822
src/runtime.cc:822: return ThrowRedeclarationError("const", name);
On 2009/11/24 15:06:00, Mads Ager wrote:
> This seems inconsistent with the case above.  Should we shadow if there is an
> interceptor readonly property on a hidden prototype?  That is what we do for a
> normal readonly property on a hidden prototype.

Thank you very much---very well spotted.

http://codereview.chromium.org/434035/diff/1/2#newcode849
src/runtime.cc:849: // ASSERT(!lookup.IsProperty());
On 2009/11/24 15:06:00, Mads Ager wrote:
> Code in comment.

Sorry, removed.

[Mads Ager (chromium), 2009-11-25 08:14:30]

http://codereview.chromium.org/434035/diff/1002/3001
File src/runtime.cc (left):

http://codereview.chromium.org/434035/diff/1002/3001#oldcode793
src/runtime.cc:793: if (!lookup.IsProperty()) {
In the new code you are checking IsValid() instead of IsProperty().  Is that
safe?

[antonm, 2009-11-25 14:30:31]

http://codereview.chromium.org/434035/diff/1002/3001
File src/runtime.cc (left):

http://codereview.chromium.org/434035/diff/1002/3001#oldcode793
src/runtime.cc:793: if (!lookup.IsProperty()) {
On 2009/11/25 08:14:30, Mads Ager wrote:
> In the new code you are checking IsValid() instead of IsProperty().  Is that
> safe?

Oops.  Fixed.  Thanks.

[Mads Ager (chromium), 2009-11-25 14:42:29]

LGTM