1027964 - Clean up Proxy Handler families

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Eric Faust [:efaust]]

Currently, there's all sorts of plans to get a handler family. Everyone seems to have their own. We should come up with one clean way to do it. It will also help bug 1027425 as some poeple use function calls, which is silly.

Personally, I like DebugScopeProxy's approach: |int DebugScopeProxy::family = 0;|

Comment 1

[Eric Faust [:efaust]]

Attachment: Patch

This patch just standardizes things. The main thing that's useful here is that we remove ProxyFamily() from the DOM code, which helps in the constexpr effort in bug 1027425

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 8477070 [details] [diff] [review]
Patch

Review of attachment 8477070 [details] [diff] [review]:
-----------------------------------------------------------------

Sure, this is an improvement.

I guess the C++-ish way to do this kind of thing is to declare a type that's explicitly empty:
    struct FamilyTag {};
(Pretty sure the Standard guarantees that global/static objects of empty types all have distinct addresses.) But I don't care that much.

Anyway---while you're at it, why not add a brief comment on BaseProxyHandler::{mFamily,family} explaining what they're for?

::: dom/base/WindowNamedPropertiesHandler.h
@@ +15,5 @@
>  class WindowNamedPropertiesHandler : public BaseDOMProxyHandler
>  {
>  public:
>    WindowNamedPropertiesHandler()
> +    : BaseDOMProxyHandler(&family, /* hasPrototype = */ true)

Why isn't nullptr OK here? I'm not super familiar with the family stuff, but I thought we only provided a non-null family pointer for families of proxies that we needed to test for dynamically somewhere.

Comment 3

[Eric Faust [:efaust]]

(In reply to Jason Orendorff [:jorendorff] from comment #2)
> Anyway---while you're at it, why not add a brief comment on
> BaseProxyHandler::{mFamily,family} explaining what they're for?
> 

Done. 
> ::: dom/base/WindowNamedPropertiesHandler.h
> @@ +15,5 @@
> >  class WindowNamedPropertiesHandler : public BaseDOMProxyHandler
> >  {
> >  public:
> >    WindowNamedPropertiesHandler()
> > +    : BaseDOMProxyHandler(&family, /* hasPrototype = */ true)
> 
> Why isn't nullptr OK here? I'm not super familiar with the family stuff, but
> I thought we only provided a non-null family pointer for families of proxies
> that we needed to test for dynamically somewhere.

It's fine. I got overzealous. Reverted.

https://hg.mozilla.org/integration/mozilla-inbound/rev/54d1a31bb34c

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/54d1a31bb34c