Fix stub-invoked setter callback handling.

Fix stub-invoked setter callback handling.

When invoking a setter callback for a property using
JSObject::SetPropertyWithCallback(),the callback arguments includes
a correct pair of receiver and holder objects.

Such a pair of _possibly different_ arguments (receiver, holder) must
also be supplied when invoking the same setter callback from JITed
code, when the setter is invoked through the StoreCallbackProperty
stub.

An example where this matters are the accessor properties kept on the
global scope of Worker (i.e., properties kept on the global object
itself, and not on its prototype.) Conflating the receiver with the
holder leads to general confusion when attempting to fetch out the
wrapper object.

LOG=N
R=dcarney@chromium.org, dcarney
BUG=239669
Committed: https://code.google.com/p/v8/source/detail?r=18658

[sof, 2014-01-15 22:14:54]

Please take a look when you next have a moment (or include someone even better
placed to have a look at this one.)

Note:
 - Spent much time trying, but I somewhat failed in reducing the Worker's use of
object templates on its global scope object as a testcase. Hence & instead, the
testcase "only" triggers the problematic stub code path, but doesn't mirror the
exact steps of the upstream crashing TC.
 - One could imagine having two versions of StoreCallbackProperty(), one where
holder != receiver and one not. I considered it not worth doing, at least
now/initially.

[dcarney, 2014-01-16 07:44:13]

lgtm, but please fix the test

https://codereview.chromium.org/139263008/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/139263008/diff/1/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:21638: TEST(Regress239669) {
this test doesn't test the stub you changed
please change it to the following

    LocalContext context;
  v8::Isolate* isolate = context->GetIsolate();
  v8::HandleScope scope(isolate);
  Local<ObjectTemplate> templ = ObjectTemplate::New(isolate);
  templ->SetAccessor(v8_str("x"), 0, SetterWhichExpectsThisAndHolderToDiffer);
  context->Global()->Set(v8_str("P"), templ->NewInstance());

  CompileRun(
     "function C1() {"
      "  this.x = 23;"
      "};"
      "C1.prototype = P;"
      "for (var i = 0; i < 4; i++ ) {"
      "  new C1();"
      "}");

[sof, 2014-01-16 08:00:16]

https://codereview.chromium.org/139263008/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/139263008/diff/1/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:21638: TEST(Regress239669) {
On 2014/01/16 07:44:13, dcarney wrote:
> this test doesn't test the stub you changed
> please change it to the following
> 
>     LocalContext context;
>   v8::Isolate* isolate = context->GetIsolate();
>   v8::HandleScope scope(isolate);
>   Local<ObjectTemplate> templ = ObjectTemplate::New(isolate);
>   templ->SetAccessor(v8_str("x"), 0, SetterWhichExpectsThisAndHolderToDiffer);
>   context->Global()->Set(v8_str("P"), templ->NewInstance());
> 
>   CompileRun(
>      "function C1() {"
>       "  this.x = 23;"
>       "};"
>       "C1.prototype = P;"
>       "for (var i = 0; i < 4; i++ ) {"
>       "  new C1();"
>       "}");
> 

Thanks, will do.

(But what's there exercises that code path for me (x64, bleeding_edge.) )

[sof, 2014-01-16 08:44:11]

https://codereview.chromium.org/139263008/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/139263008/diff/1/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:21638: TEST(Regress239669) {
On 2014/01/16 07:44:13, dcarney wrote:
> this test doesn't test the stub you changed
> please change it to the following
> 
>     LocalContext context;
>   v8::Isolate* isolate = context->GetIsolate();
>   v8::HandleScope scope(isolate);
>   Local<ObjectTemplate> templ = ObjectTemplate::New(isolate);
>   templ->SetAccessor(v8_str("x"), 0, SetterWhichExpectsThisAndHolderToDiffer);
>   context->Global()->Set(v8_str("P"), templ->NewInstance());
> 
>   CompileRun(
>      "function C1() {"
>       "  this.x = 23;"
>       "};"
>       "C1.prototype = P;"
>       "for (var i = 0; i < 4; i++ ) {"
>       "  new C1();"
>       "}");
> 

Done.

[dcarney, 2014-01-16 08:55:30]

On 2014/01/16 08:44:11, sof wrote:
> https://codereview.chromium.org/139263008/diff/1/test/cctest/test-api.cc
> File test/cctest/test-api.cc (right):
> 
>
https://codereview.chromium.org/139263008/diff/1/test/cctest/test-api.cc#newc...
> test/cctest/test-api.cc:21638: TEST(Regress239669) {
> On 2014/01/16 07:44:13, dcarney wrote:
> > this test doesn't test the stub you changed
> > please change it to the following
> > 
> >     LocalContext context;
> >   v8::Isolate* isolate = context->GetIsolate();
> >   v8::HandleScope scope(isolate);
> >   Local<ObjectTemplate> templ = ObjectTemplate::New(isolate);
> >   templ->SetAccessor(v8_str("x"), 0,
SetterWhichExpectsThisAndHolderToDiffer);
> >   context->Global()->Set(v8_str("P"), templ->NewInstance());
> > 
> >   CompileRun(
> >      "function C1() {"
> >       "  this.x = 23;"
> >       "};"
> >       "C1.prototype = P;"
> >       "for (var i = 0; i < 4; i++ ) {"
> >       "  new C1();"
> >       "}");
> > 
> 
> Done.

thx, will land shortly

[dcarney, 2014-01-17 10:34:51]

Committed patchset #2 manually as r18658 (presubmit successful).