Bootstrapper::DetachGlobal also need to unset global_proxy's constructor to remove all refs to cont…

Bootstrapper::DetachGlobal also need to unset global_proxy's constructor to remove all refs to context

DetachGlobal detaches original context of a global proxy object.

Before this patch, the constructor JSFunction still carried a reference to the old context after |Bootstrapper::DetachGlobal| call.
This patch removes the reference by setting the constructor null.

TEST=http/tests/security/isolatedWorld w/ --enable-leak-detection
LOG=N
BUG=364377
R=dcarney@chromium.org

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

[kouhei (in TOK), 2014-07-17 06:52:23]

This is my other attempt to fix the isolated world bug.

I inspected |V8WindowShell::m_global| after navigation, and found that it was
still carrying a reference to the old context via |__proto__.constructor|.
I think this fix is OK as the global_proxy's prototype is invisible from user,
and the prototype is recovered in next call to |Genesis::CreatenewGlobals|
anyway.
However, I'm not an v8 expert so I'd appreciate if you would take a close look.
Thanks!

[haraken, 2014-07-18 05:43:37]

dcarney: ping?(≧∇≦) We need to fix the document leak of isolated worlds since it
happens on all Blink-in-JS.

[dcarney, 2014-07-18 05:59:03]

On 2014/07/18 05:43:37, haraken wrote:
> dcarney: ping?(≧∇≦) We need to fix the document leak of isolated worlds since
it
> happens on all Blink-in-JS.

i think this is okay.  i'll land it and see what happens.  kouhei, did you run
all the blink tests with this patch applied?

lgtm

[kouhei (in TOK), 2014-07-18 06:01:19]

On 2014/07/18 05:59:03, dcarney wrote:
> On 2014/07/18 05:43:37, haraken wrote:
> > dcarney: ping?(≧∇≦) We need to fix the document leak of isolated worlds
since
> it
> > happens on all Blink-in-JS.
> 
> i think this is okay.  i'll land it and see what happens.  kouhei, did you run
> all the blink tests with this patch applied?
> 
> lgtm

Yes, I run the layout tests:
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_layout_dbg/builds/284

[dcarney, 2014-07-18 06:06:25]

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

[dcarney, 2014-07-18 10:52:11]

On 2014/07/18 06:06:25, dcarney wrote:
> Committed patchset #2 manually as r22460 (presubmit successful).

we're seeing some potential crashes from that cl on mac:

http://build.chromium.org/p/client.v8/builders/Webkit%20Mac

do you guys have time to investigate?  i don't right now, and we'll revert the
cl if we can't resolve this fairly soon

[haraken, 2014-07-18 11:06:33]

On 2014/07/18 10:52:11, dcarney wrote:
> On 2014/07/18 06:06:25, dcarney wrote:
> > Committed patchset #2 manually as r22460 (presubmit successful).
> 
> we're seeing some potential crashes from that cl on mac:
> 
> http://build.chromium.org/p/client.v8/builders/Webkit%20Mac
> 
> do you guys have time to investigate?  i don't right now, and we'll revert the
> cl if we can't resolve this fairly soon

Japan is running into the weekend, so it would make sense to revert the CL.
Sorry about that.

[dcarney, 2014-07-18 11:10:10]

On 2014/07/18 11:06:33, haraken wrote:
> On 2014/07/18 10:52:11, dcarney wrote:
> > On 2014/07/18 06:06:25, dcarney wrote:
> > > Committed patchset #2 manually as r22460 (presubmit successful).
> > 
> > we're seeing some potential crashes from that cl on mac:
> > 
> > http://build.chromium.org/p/client.v8/builders/Webkit%20Mac
> > 
> > do you guys have time to investigate?  i don't right now, and we'll revert
the
> > cl if we can't resolve this fairly soon
> 
> Japan is running into the weekend, so it would make sense to revert the CL.
> Sorry about that.

np - i'll revert and if it doesn't change the crashing i'll re-revert.  i don't
see how this cl could be at fault.  have a good weekend

[dcarney, 2014-07-18 12:56:05]

> np - i'll revert and if it doesn't change the crashing i'll re-revert.  i
don't
> see how this cl could be at fault.  have a good weekend


fyi - the revert fixed the crashing, so i'll leave it to you guys to fix on next
week

[kouhei (in TOK), 2014-07-25 08:51:27]

I think this is resolved by blink r178818

haraken: Would you try landing this again?

[haraken, 2014-07-25 08:51:50]

On 2014/07/25 08:51:27, kouhei wrote:
> I think this is resolved by blink r178818
> 
> haraken: Would you try landing this again?

sure.

[haraken, 2014-07-25 09:00:54]

On 2014/07/25 08:51:50, haraken wrote:
> On 2014/07/25 08:51:27, kouhei wrote:
> > I think this is resolved by blink r178818
> > 
> > haraken: Would you try landing this again?
> 
> sure.

Should I commit to bleeding_edge or master? (Or I'd be happy if you could land
this on behalf of us.)

[dcarney, 2014-07-25 09:23:54]

I'm off for the next 2 months.  jochen, could you land this and watch the tree
for breakage?

[jochen (gone - plz use gerrit), 2014-07-25 10:10:53]

i'm out today, but I'll take care of it on Monday (Kentaro, to answer your
question: bleeding_edge.

fetch v8
cd v8
git cl patch -b land 397953009
git cl dcommit -c '...'

)

[haraken, 2014-07-25 10:12:49]

On 2014/07/25 10:10:53, jochen wrote:
> i'm out today, but I'll take care of it on Monday (Kentaro, to answer your
> question: bleeding_edge.
> 
> fetch v8
> cd v8
> git cl patch -b land 397953009
> git cl dcommit -c '...'
> 
> )

For safety, let me ask you to take care of landing the CL. Monday is totally
fine :)

[kouhei (in TOK), 2014-07-28 10:35:22]

On 2014/07/25 10:12:49, haraken wrote:
> On 2014/07/25 10:10:53, jochen wrote:
> > i'm out today, but I'll take care of it on Monday (Kentaro, to answer your
> > question: bleeding_edge.
> > 
> > fetch v8
> > cd v8
> > git cl patch -b land 397953009
> > git cl dcommit -c '...'
> > 
> > )
> 
> For safety, let me ask you to take care of landing the CL. Monday is totally
> fine :)

Thanks jochen! Relanded as https://codereview.chromium.org/424703002