Remove ExpandoProxyHandler.

ExpandoProxyHandler gives no benefit in the current taming architecture;
it is a relic of times when we were supporting the notion of multiple
views of the same feral DOM node which should share properties, and this
capability is never used in the current architecture (and was only
present on proxy-supporting platforms).

Instead, the only Domado objects wrapped in proxies are those which need
it (e.g. <form> elements' nodelistiness).

The WeakMap support magic has a new home in ProxyHandler.

Rough benchmarks: in ES5 mode, jQuery test 'traversing' takes 87% of
its previous execution time, and 'manipulation' 80%.

@r5393

[ihab.awad, 2013-04-30 17:09:27]

lgtm++

Please don't forget to add to the CL description some benchmarks of before and
after performance. It would be interesting to see some metric of startup time
(likely unaffected) and run time in exercising the methods of a whole bunch of
DOM nodes (likely affected).

Perhaps we should add to our source somewhere a simple Caja page that does some
Domado-intensive stuff and spits out some numbers? We can just run it before and
after and copy-paste in the numbers when we do something that we think affects
performance significantly.

https://codereview.appspot.com/8700044/diff/1/src/com/google/caja/plugin/doma...
File src/com/google/caja/plugin/domado.js (right):

https://codereview.appspot.com/8700044/diff/1/src/com/google/caja/plugin/doma...
src/com/google/caja/plugin/domado.js:612: var weakMapMagicName = {};
Is it possible to move this stuff -- the declaration of weakMapMagicName and the
associated magic that initializes it -- closer to ProxyHandler? Can it in fact
be absorbed *into* the ProxyHandler closure?

Too much magic by half. Oy vey. :)

https://codereview.appspot.com/8700044/diff/1/src/com/google/caja/plugin/doma...
src/com/google/caja/plugin/domado.js:1815: // wrong if our objects are mutated
(the expando proxy handler
Stale comment?

[kpreid2, 2013-04-30 18:57:46]

ExpandoProxyHandler gives no benefit in the current taming architecture;
it is a relic of times when we were supporting the notion of multiple
views of the same feral DOM node which should share properties, and this
capability is never used in the current architecture (and was only
present on proxy-supporting platforms).

Instead, the only Domado objects wrapped in proxies are those which need
it (e.g. <form> elements' nodelistiness).

The WeakMap support magic has a new home in ProxyHandler.

[kpreid2, 2013-04-30 18:57:52]

New snapshot.

https://codereview.appspot.com/8700044/diff/1/src/com/google/caja/plugin/doma...
File src/com/google/caja/plugin/domado.js (right):

https://codereview.appspot.com/8700044/diff/1/src/com/google/caja/plugin/doma...
src/com/google/caja/plugin/domado.js:612: var weakMapMagicName = {};
On 2013/04/30 17:09:27, ihab.awad wrote:
> Is it possible to move this stuff -- the declaration of weakMapMagicName and
> the associated magic that initializes it -- closer to ProxyHandler?

Done.

> Can it in fact be absorbed *into* the ProxyHandler closure?

No, because other proxy handlers (only CollectionProxyHandler, at the moment)
need it too — the magic name handling needs to happen *first*, so it can't just
be inherited.

> Too much magic by half. Oy vey. :)

Great, let's ask MarkM to implement a Proxy monkeypatch which hides the magic
name. This also means we can expose Proxy to guests, and won't that be fun?

https://codereview.appspot.com/8700044/diff/1/src/com/google/caja/plugin/doma...
src/com/google/caja/plugin/domado.js:1815: // wrong if our objects are mutated
(the expando proxy handler
On 2013/04/30 17:09:27, ihab.awad wrote:
> Stale comment?

Not just stale comment, stale and now-incorrect code. Added a test to verify,
deleted.

[Mark S. Miller, 2013-04-30 22:13:25]

On Tue, Apr 30, 2013 at 11:57 AM, <kpreid.switchb.org@gmail.com> wrote:

> New snapshot.
>
>
>
> https://codereview.appspot.**com/8700044/diff/1/src/com/**
>
google/caja/plugin/domado.js<https://codereview.appspot.com/8700044/diff/1/src/com/google/caja/plugin/domado.js>
> File src/com/google/caja/plugin/**domado.js (right):
>
> https://codereview.appspot.**com/8700044/diff/1/src/com/**
>
google/caja/plugin/domado.js#**newcode612<https://codereview.appspot.com/8700044/diff/1/src/com/google/caja/plugin/domado.js#newcode612>
> src/com/google/caja/plugin/**domado.js:612: var weakMapMagicName = {};
> On 2013/04/30 17:09:27, ihab.awad wrote:
>
>> Is it possible to move this stuff -- the declaration of
>>
> weakMapMagicName and
>
>> the associated magic that initializes it -- closer to ProxyHandler?
>>
>
> Done.
>
>
>  Can it in fact be absorbed *into* the ProxyHandler closure?
>>
>
> No, because other proxy handlers (only CollectionProxyHandler, at the
> moment) need it too — the magic name handling needs to happen *first*,
> so it can't just be inherited.
>
>
>  Too much magic by half. Oy vey. :)
>>
>
> Great, let's ask MarkM to implement a Proxy monkeypatch which hides the
> magic name. This also means we can expose Proxy to guests, and won't
> that be fun?


Context?




>
>
> https://codereview.appspot.**com/8700044/diff/1/src/com/**
>
google/caja/plugin/domado.js#**newcode1815<https://codereview.appspot.com/8700044/diff/1/src/com/google/caja/plugin/domado.js#newcode1815>
> src/com/google/caja/plugin/**domado.js:1815: // wrong if our objects are
> mutated (the expando proxy handler
> On 2013/04/30 17:09:27, ihab.awad wrote:
>
>> Stale comment?
>>
>
> Not just stale comment, stale and now-incorrect code. Added a test to
> verify, deleted.
>
>
>
https://codereview.appspot.**com/8700044/<https://codereview.appspot.com/8700...
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "Google Caja Discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
google-caja-discuss+**unsubscribe@googlegroups.com<google-caja-discuss%2Bunsu...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>


-- 
    Cheers,
    --MarkM

[kpreid2, 2013-04-30 22:21:53]

On 2013/04/30 22:13:25, Mark S. Miller wrote:
> On Tue, Apr 30, 2013 at 11:57 AM, <mailto:kpreid.switchb.org@gmail.com> wrote:
> > Great, let's ask MarkM to implement a Proxy monkeypatch which hides the
> > magic name. This also means we can expose Proxy to guests, and won't
> > that be fun?
> 
> Context?

In your own words, from WeakMap.js:

   * <p>TODO(erights): On a platform with built-in Proxies, proxies
   * could be used to trap and thereby discover the HIDDEN_NAME, so we
   * need to monkey patch Proxy.create, Proxy.createFunction, etc, in
   * order to wrap the provided handler with the real handler which
   * filters out all traps using HIDDEN_NAME.

The rest of the context is that if this TODO were done, then Domado would no
longer need the kludge we're discussing.

I think this would be a very good thing to have, but I don't know offhand how
hard it would be to do. I'm willing to give it a try if you aren't, but I have
less confidence in my ability to write a 100% correct wrapper than yours.

[MarkM, 2013-04-30 22:25:05]

If you do it, I'll review it carefully.


On Tue, Apr 30, 2013 at 3:21 PM, <kpreid.switchb.org@gmail.com> wrote:

> On 2013/04/30 22:13:25, Mark S. Miller wrote:
>
>> On Tue, Apr 30, 2013 at 11:57 AM,
>>
> <mailto:kpreid.switchb.org@**gmail.com <kpreid.switchb.org@gmail.com>>
> wrote:
>
>> > Great, let's ask MarkM to implement a Proxy monkeypatch which hides
>>
> the
>
>> > magic name. This also means we can expose Proxy to guests, and won't
>> > that be fun?
>>
>
>  Context?
>>
>
> In your own words, from WeakMap.js:
>
>    * <p>TODO(erights): On a platform with built-in Proxies, proxies
>    * could be used to trap and thereby discover the HIDDEN_NAME, so we
>    * need to monkey patch Proxy.create, Proxy.createFunction, etc, in
>    * order to wrap the provided handler with the real handler which
>    * filters out all traps using HIDDEN_NAME.
>
> The rest of the context is that if this TODO were done, then Domado
> would no longer need the kludge we're discussing.
>
> I think this would be a very good thing to have, but I don't know
> offhand how hard it would be to do. I'm willing to give it a try if you
> aren't, but I have less confidence in my ability to write a 100% correct
> wrapper than yours.
>
>
>
https://codereview.appspot.**com/8700044/<https://codereview.appspot.com/8700...
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "Google Caja Discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
google-caja-discuss+**unsubscribe@googlegroups.com<google-caja-discuss%2Bunsu...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>


-- 
Text by me above is hereby placed in the public domain

  Cheers,
  --MarkM

[kpreid2, 2013-05-02 19:04:18]

On 2013/04/30 17:09:27, ihab.awad wrote:
> Please don't forget to add to the CL description some benchmarks of before and
> after performance. It would be interesting to see some metric of startup time
> (likely unaffected) and run time in exercising the methods of a whole bunch of
> DOM nodes (likely affected).

Done (only run time, not startup).

Waiting for review of snapshot 2.

[ihab.awad, 2013-05-02 19:40:24]

lgtm++