Fix |RunMicrotasks()| leaking reference to the last context being run on.

Fix |RunMicrotasks()| leaking reference to the last context being run on.

RunMicrotasks() executes pending tasks and swaps the old array with a new array. However, the new array contains the reference to the current context as its creation context. This prevents the context from gc-ed until RunMicrotasks() is executed in the different context.

BUG=crbug.com/367016
LOG=y
R=rafaelw@chromium.org, rossberg@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=21111

[kouhei (in TOK), 2014-04-25 07:42:02]

Would you take a look?

[Yang, 2014-04-25 07:47:42]

On 2014/04/25 07:42:02, kouhei wrote:
> Would you take a look?

drive by: using delete would convert the object into slow mode, which is not
that nice. It should work if you set microtaskState.queue to undefined, since
undefined is a context independent value, right?

[rossberg, 2014-04-25 07:50:14]

On 2014/04/25 07:47:42, Yang wrote:
> On 2014/04/25 07:42:02, kouhei wrote:
> > Would you take a look?
> 
> drive by: using delete would convert the object into slow mode, which is not
> that nice. It should work if you set microtaskState.queue to undefined, since
> undefined is a context independent value, right?

Indeed. Don't use delete in JavaScript. Ever.

[kouhei (in TOK), 2014-04-25 08:02:53]

PTAL.

On 2014/04/25 07:50:14, rossberg wrote:
> On 2014/04/25 07:47:42, Yang wrote:
> > On 2014/04/25 07:42:02, kouhei wrote:
> > > Would you take a look?
> > 
> > drive by: using delete would convert the object into slow mode, which is not
> > that nice. It should work if you set microtaskState.queue to undefined,
since
> > undefined is a context independent value, right?
> 
> Indeed. Don't use delete in JavaScript. Ever.

Done. Changed to undefined.

[rossberg, 2014-04-25 09:15:25]

LGTM, but perhaps let Rafael have a look, too, before committing.

[rafaelw, 2014-04-25 19:58:19]

Will you please refactor this so that clients call EnqueueMicrotask(fn) rather
than GetMicrotaskQueue(). This patch really isn't making this code more brittle,
it's just making it more *obviously* brittle =-).

[kouhei (in TOK), 2014-04-28 03:47:07]

On 2014/04/25 19:58:19, rafaelw wrote:
> Will you please refactor this so that clients call EnqueueMicrotask(fn) rather
> than GetMicrotaskQueue(). This patch really isn't making this code more
brittle,
> it's just making it more *obviously* brittle =-).

Done. PTAL

[rafaelw, 2014-04-28 05:09:33]

lgtm w/ nits

https://codereview.chromium.org/250883002/diff/60001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/250883002/diff/60001/src/bootstrapper.cc#newc...
src/bootstrapper.cc:1556: enqueue_external_microtask);
Can you please update

enqueue_external_microtask -> enqueue_microtask

and

ENQUEUE_EXTERNAL_MICROTASK -> ENQUEUE_MICROTASK

in contexts.h & execution.cc

?

https://codereview.chromium.org/250883002/diff/60001/src/v8natives.js
File src/v8natives.js (right):

https://codereview.chromium.org/250883002/diff/60001/src/v8natives.js#newcode...
src/v8natives.js:1903: delete microtaskState.queue;
nit: assign NULL here (and check for NULL as well as UNDEFINED below). Deleting
puts the object into dictionary storage mode.

[kouhei (in TOK), 2014-04-28 05:22:29]

https://codereview.chromium.org/250883002/diff/60001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/250883002/diff/60001/src/bootstrapper.cc#newc...
src/bootstrapper.cc:1556: enqueue_external_microtask);
On 2014/04/28 05:09:34, rafaelw wrote:
> Can you please update
> 
> enqueue_external_microtask -> enqueue_microtask
> 
> and
> 
> ENQUEUE_EXTERNAL_MICROTASK -> ENQUEUE_MICROTASK
> 
> in contexts.h & execution.cc
> 
> ?

Done.

https://codereview.chromium.org/250883002/diff/60001/src/v8natives.js
File src/v8natives.js (right):

https://codereview.chromium.org/250883002/diff/60001/src/v8natives.js#newcode...
src/v8natives.js:1903: delete microtaskState.queue;
On 2014/04/28 05:09:34, rafaelw wrote:
> nit: assign NULL here (and check for NULL as well as UNDEFINED below).
Deleting
> puts the object into dictionary storage mode.
Sorry I reverted the change by mistake. Done.

[kouhei (in TOK), 2014-04-30 01:05:17]

haraken: Would you land this patch for me? I'm not a v8 committer.

[haraken, 2014-04-30 01:11:20]

On 2014/04/30 01:05:17, kouhei wrote:
> haraken: Would you land this patch for me? I'm not a v8 committer.

The bots look happy. Let me land this.

[haraken, 2014-04-30 01:15:16]

On 2014/04/30 01:11:20, haraken wrote:
> On 2014/04/30 01:05:17, kouhei wrote:
> > haraken: Would you land this patch for me? I'm not a v8 committer.
> 
> The bots look happy. Let me land this.

The V8 tree is now closed, so I'll land this once it's reopened.

[kouhei (in TOK), 2014-05-01 00:43:31]

haraken: Would you try again?

[kouhei (in TOK), 2014-05-02 01:18:24]

On 2014/05/01 00:43:31, kouhei wrote:
> haraken: Would you try again?

ping

[haraken, 2014-05-02 08:01:00]

Committed patchset #5 manually as r21111 (presubmit successful).