Postpone counters triggered during GC, and use a HandleScope when calling back.

Postpone counters triggered during GC, and use a HandleScope when calling back.

R=jkummerow@chromium.org,hpayer@chromium.org
BUG=

Committed: https://crrev.com/5bc51bb454ba4e96e9bcbdc4106b6986a27eed6e
Cr-Commit-Position: refs/heads/master@{#28335}

[Jakob Kummerow, 2015-05-11 08:27:37]

Looks good, with two nits.

As it includes an API change, please don't land until after we've made our
branch (or promise that nothing in Chromium/Blink will depend on the new enum
value).

https://codereview.chromium.org/1125383007/diff/1/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1125383007/diff/1/src/heap/heap.cc#newcode402
src/heap/heap.cc:402: while (count != 0) {
nit: I'd suggest s/!=/>/ here, as it's slightly more accurate.

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

https://codereview.chromium.org/1125383007/diff/1/test/cctest/test-heap.cc#ne...
test/cctest/test-heap.cc:3228: CHECK_GT(forced_gc_counter, 0);
Why so conservative? Can this ever be != 1, and still be working as intended?

[Erik Corry, 2015-05-11 09:56:12]

erik.corry@gmail.com changed reviewers:
+ erik.corry@gmail.com

[Erik Corry, 2015-05-11 09:56:12]

I promise that nothing in Blink will depend on the new value.  V8 already has
some values here (3, 4, 5 and 6) that Blink knows nothing about, and that works
now.  It used to be the case that Blink was anal about it, but not recently.

https://codereview.chromium.org/1125383007/diff/1/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1125383007/diff/1/src/heap/heap.cc#newcode402
src/heap/heap.cc:402: while (count != 0) {
On 2015/05/11 08:27:37, Jakob wrote:
> nit: I'd suggest s/!=/>/ here, as it's slightly more accurate.

Done.

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

https://codereview.chromium.org/1125383007/diff/1/test/cctest/test-heap.cc#ne...
test/cctest/test-heap.cc:3228: CHECK_GT(forced_gc_counter, 0);
On 2015/05/11 08:27:37, Jakob wrote:
> Why so conservative? Can this ever be != 1, and still be working as intended?

An emergency out-of-memory GC is also implemented as 2-7 forced GCs.  They are
forced in the sense that they have to happen now, and can't be incremental.  So
the system does generate its own forced GCs, albeit rarely.

[Erik Corry, 2015-05-11 09:57:15]

The CQ bit was checked by erik.corry@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2015-05-11 09:57:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1125383007/20001

[commit-bot: I haz the power, 2015-05-11 09:57:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-11 09:57:28]

Dry run: No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Erik Corry Chromium.org, 2015-05-11 10:00:15]

The CQ bit was checked by erikcorry@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-05-11 10:00:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1125383007/20001

[Jakob Kummerow, 2015-05-11 10:38:05]

On 2015/05/11 09:56:12, Erik Corry wrote:
> I promise that nothing in Blink will depend on the new value.

OK, LGTM then.

(We need to be able to freely roll V8 back and forth in Chromium until we've
settled for a branch candidate. That implies that there's no guarantee this will
make it into the branch.)

[commit-bot: I haz the power, 2015-05-11 10:49:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-11 10:49:22]

Dry run: This issue passed the CQ dry run.

[Erik Corry Chromium.org, 2015-05-11 10:56:07]

The CQ bit was checked by erikcorry@chromium.org

[commit-bot: I haz the power, 2015-05-11 10:56:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1125383007/20001

[commit-bot: I haz the power, 2015-05-11 10:58:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-11 10:58:17]

Patchset 2 (id:??) landed as
https://crrev.com/5bc51bb454ba4e96e9bcbdc4106b6986a27eed6e
Cr-Commit-Position: refs/heads/master@{#28335}