Revert of IDL: Add support for [Unscopeable] on attributes and methods

Revert of IDL: Add support for [Unscopeable] on attributes and methods (patchset #2 id:20001 of https://codereview.chromium.org/1085453003/)

Reason for revert:
This change caused 5k+ leaks on WebKit Linux Leak.
Confirmed via bisect.
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/builds/10565

Original issue's description:
> IDL: Add support for [Unscopeable] on attributes and methods
> 
> Adding [Unscopeable] to an attribute or method means it won't be found
> during name lookup with an object that has the attribute or method in
> the scope (i.e. using the "with" statement.)
> 
> The mechanism that makes this happen is that the interface prototype
> object has a property named @@unscopables that is an object with a
> property for every unscopeable member named as the member.
> 
> BUG=462916
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194091

TBR=haraken@chromium.org,bashi@chromium.org,jl@opera.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=462916
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194191

[alancutter (OOO until 2018), 2015-04-22 04:05:45]

Created Revert of IDL: Add support for [Unscopeable] on attributes and methods

[commit-bot: I haz the power, 2015-04-22 04:05:59]

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

[commit-bot: I haz the power, 2015-04-22 04:06:37]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194191

[haraken, 2015-04-22 05:17:25]

LGTM, thanks for the revert.

(I'm happy to see that the leak detector is working, btw :-)

[Jens Widell, 2015-04-22 06:13:54]

Thanks for reverting!

haraken@: do you have any guess how the patch caused a leak?

[haraken, 2015-04-22 06:25:11]

On 2015/04/22 06:13:54, Jens Widell wrote:
> Thanks for reverting!
> 
> haraken@: do you have any guess how the patch caused a leak?

Not sure but:

> v8::Local<v8::Object> unscopeables(v8::Object::New(isolate));

^^^ unscopeables creates a strong reference to the context that created the
unscopeables (i.e., unscopeables->CreationContext()).

Thus it will create a reference chain like:

  prototypeTemplate => unscopeables => context => everything in the context

Given that prototypeTemplate is kept alive as long as the isolate is alive, it
will leak the objects in the context. In other words, we shouldn't add a strong
reference from prototypeTemplate to some object.

[Jens Widell, 2015-04-22 06:28:18]

On 2015/04/22 06:25:11, haraken wrote:
> On 2015/04/22 06:13:54, Jens Widell wrote:
> > Thanks for reverting!
> > 
> > haraken@: do you have any guess how the patch caused a leak?
> 
> Not sure but:
> 
> > v8::Local<v8::Object> unscopeables(v8::Object::New(isolate));
> 
> ^^^ unscopeables creates a strong reference to the context that created the
> unscopeables (i.e., unscopeables->CreationContext()).
> 
> Thus it will create a reference chain like:
> 
>   prototypeTemplate => unscopeables => context => everything in the context
> 
> Given that prototypeTemplate is kept alive as long as the isolate is alive, it
> will leak the objects in the context. In other words, we shouldn't add a
strong
> reference from prototypeTemplate to some object.

Should we create the unscopables object when creating the prototype object
instead of when setting up the prototype template, then?

[haraken, 2015-04-22 06:31:37]

On 2015/04/22 06:28:18, Jens Widell wrote:
> On 2015/04/22 06:25:11, haraken wrote:
> > On 2015/04/22 06:13:54, Jens Widell wrote:
> > > Thanks for reverting!
> > > 
> > > haraken@: do you have any guess how the patch caused a leak?
> > 
> > Not sure but:
> > 
> > > v8::Local<v8::Object> unscopeables(v8::Object::New(isolate));
> > 
> > ^^^ unscopeables creates a strong reference to the context that created the
> > unscopeables (i.e., unscopeables->CreationContext()).
> > 
> > Thus it will create a reference chain like:
> > 
> >   prototypeTemplate => unscopeables => context => everything in the context
> > 
> > Given that prototypeTemplate is kept alive as long as the isolate is alive,
it
> > will leak the objects in the context. In other words, we shouldn't add a
> strong
> > reference from prototypeTemplate to some object.
> 
> Should we create the unscopables object when creating the prototype object
> instead of when setting up the prototype template, then?

Sounds reasonable to me. It seems safe to create a reference from the prototype
object to the unscopeable object.