Heap snapshot: Update user roots definition.

Heap snapshot: Update user roots definition.

Do not define an object as a user root if its context is not
present in Global handles.

R=mstarzinger@chromium.org, yurys@chromium.org

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

[alph, 2013-06-26 18:18:15]

Folks, could you please take a look.

[yurys, 2013-06-27 16:26:00]

https://codereview.chromium.org/17881005/diff/1/src/heap-snapshot-generator.cc
File src/heap-snapshot-generator.cc (right):

https://codereview.chromium.org/17881005/diff/1/src/heap-snapshot-generator.c...
src/heap-snapshot-generator.cc:1000: "native_context", proxy->native_context(),
Can we have a test for this?

As far as I understand global object in Blink consists of 3 objects [1] which
look as a single entity from the application's point of view. One of them is
created to keep global object identity on navigation while the shadow one
actually keeps all user properties. Which one of them is pointed at by the
native_context link?


[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

https://codereview.chromium.org/17881005/diff/1/src/heap-snapshot-generator.c...
src/heap-snapshot-generator.cc:1770: is_debug_object =
Isolate::Current()->debug()->IsDebugGlobal(global);
Please use heap_->isolate() instead of Isolate::Current()

[yurys, 2013-06-28 09:42:18]

https://codereview.chromium.org/17881005/diff/1/src/heap-snapshot-generator.cc
File src/heap-snapshot-generator.cc (right):

https://codereview.chromium.org/17881005/diff/1/src/heap-snapshot-generator.c...
src/heap-snapshot-generator.cc:1767: CHECK(global == context->global_object());
Ok, I looked more closely at how we setup global object in bootstraper.cc, in
particular the inner global object [1] and I think we should use
context->global_object() instead.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/bootstrappe...

[alph, 2013-06-28 11:58:04]

PTAL

https://codereview.chromium.org/17881005/diff/1/src/heap-snapshot-generator.cc
File src/heap-snapshot-generator.cc (right):

https://codereview.chromium.org/17881005/diff/1/src/heap-snapshot-generator.c...
src/heap-snapshot-generator.cc:1000: "native_context", proxy->native_context(),
On 2013/06/27 16:26:00, Yury Semikhatsky wrote:
> Can we have a test for this?
> 
> As far as I understand global object in Blink consists of 3 objects [1] which
> look as a single entity from the application's point of view. One of them is
> created to keep global object identity on navigation while the shadow one
> actually keeps all user properties. Which one of them is pointed at by the
> native_context link?
> 
> 
> [1]
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Don't think checking that we've reported this the only propery of JSGlobalProxy
deserves a test.

https://codereview.chromium.org/17881005/diff/1/src/heap-snapshot-generator.c...
src/heap-snapshot-generator.cc:1767: CHECK(global == context->global_object());
On 2013/06/28 09:42:18, Yury Semikhatsky wrote:
> Ok, I looked more closely at how we setup global object in bootstraper.cc, in
> particular the inner global object [1] and I think we should use
> context->global_object() instead.
> 
> [1]
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/bootstrappe...

Done.

https://codereview.chromium.org/17881005/diff/1/src/heap-snapshot-generator.c...
src/heap-snapshot-generator.cc:1770: is_debug_object =
Isolate::Current()->debug()->IsDebugGlobal(global);
On 2013/06/27 16:26:00, Yury Semikhatsky wrote:
> Please use heap_->isolate() instead of Isolate::Current()

Done.

[yurys, 2013-06-28 12:17:13]

lgtm

[Michael Starzinger, 2013-06-28 12:40:52]

LGTM (rubber-stamped).

[alph, 2013-06-28 12:54:04]

Committed patchset #3 manually as r15381 (presubmit successful).