Chromium Code Reviews

Issue 397953009: Bootstrapper::DetachGlobal also need to unset global_proxy's constructor to remove all refs to cont… (Closed)

Created:
6 years, 5 months ago by kouhei (in TOK)
Modified:
6 years, 4 months ago
Reviewers:
haraken, dcarney, jochen (gone - plz use gerrit)
CC:
v8-dev
Base URL:
git://github.com/v8/v8.git@master
Project:
v8
Visibility:
Public.

Description

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

Patch Set 1 #

Patch Set 2 : modify copied map #

Unified diffs Side-by-side diffs Stats (+1 line, -0 lines)
M src/bootstrapper.cc View 1 chunk +1 line, -0 lines 0 comments

Messages

Total messages: 16 (0 generated)
kouhei (in TOK)
This is my other attempt to fix the isolated world bug. I inspected |V8WindowShell::m_global| after ...
6 years, 5 months ago (2014-07-17 06:52:23 UTC) #1
haraken
dcarney: ping?(≧∇≦) We need to fix the document leak of isolated worlds since it happens ...
6 years, 5 months ago (2014-07-18 05:43:37 UTC) #2
dcarney
On 2014/07/18 05:43:37, haraken wrote: > dcarney: ping?(≧∇≦) We need to fix the document leak ...
6 years, 5 months ago (2014-07-18 05:59:03 UTC) #3
kouhei (in TOK)
On 2014/07/18 05:59:03, dcarney wrote: > On 2014/07/18 05:43:37, haraken wrote: > > dcarney: ping?(≧∇≦) ...
6 years, 5 months ago (2014-07-18 06:01:19 UTC) #4
dcarney
Committed patchset #2 manually as r22460 (presubmit successful).
6 years, 5 months ago (2014-07-18 06:06:25 UTC) #5
dcarney
On 2014/07/18 06:06:25, dcarney wrote: > Committed patchset #2 manually as r22460 (presubmit successful). we're ...
6 years, 5 months ago (2014-07-18 10:52:11 UTC) #6
haraken
On 2014/07/18 10:52:11, dcarney wrote: > On 2014/07/18 06:06:25, dcarney wrote: > > Committed patchset ...
6 years, 5 months ago (2014-07-18 11:06:33 UTC) #7
dcarney
On 2014/07/18 11:06:33, haraken wrote: > On 2014/07/18 10:52:11, dcarney wrote: > > On 2014/07/18 ...
6 years, 5 months ago (2014-07-18 11:10:10 UTC) #8
dcarney
> np - i'll revert and if it doesn't change the crashing i'll re-revert. i ...
6 years, 5 months ago (2014-07-18 12:56:05 UTC) #9
kouhei (in TOK)
I think this is resolved by blink r178818 haraken: Would you try landing this again?
6 years, 5 months ago (2014-07-25 08:51:27 UTC) #10
haraken
On 2014/07/25 08:51:27, kouhei wrote: > I think this is resolved by blink r178818 > ...
6 years, 5 months ago (2014-07-25 08:51:50 UTC) #11
haraken
On 2014/07/25 08:51:50, haraken wrote: > On 2014/07/25 08:51:27, kouhei wrote: > > I think ...
6 years, 5 months ago (2014-07-25 09:00:54 UTC) #12
dcarney
I'm off for the next 2 months. jochen, could you land this and watch the ...
6 years, 5 months ago (2014-07-25 09:23:54 UTC) #13
jochen (gone - plz use gerrit)
i'm out today, but I'll take care of it on Monday (Kentaro, to answer your ...
6 years, 5 months ago (2014-07-25 10:10:53 UTC) #14
haraken
On 2014/07/25 10:10:53, jochen wrote: > i'm out today, but I'll take care of it ...
6 years, 5 months ago (2014-07-25 10:12:49 UTC) #15
kouhei (in TOK)
6 years, 4 months ago (2014-07-28 10:35:22 UTC) #16
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

Powered by Google App Engine