Re: Eiminating nsIDOM* interfaces and brand checks

[Boris Zbarsky]

On 9/4/17 4:51 AM, Anne van Kesteren wrote:

Also, do we need this for Promise, Map, Set, etc., or just IDL-defined objects?


For the moment, just the latter, I think, in that we have no one who is 
trying to do if for the former...


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Stanimir Stamenkov via dev-platform]

On Fri, 1 Sep 2017 10:13:55 -0700, Bobby Holley wrote:

On Fri, Sep 1, 2017 at 9:59 AM, Botond Ballo wrote:

On Fri, Sep 1, 2017 at 12:11 PM, Ehsan Akhgari wrote:


How about this slight variation:

HTMLEmbedElement.isInstanceOf(obj)

instead of "is"?


Isn't that backwards, though? HTMLEmbedElement isn't an instance of 
'obj', 'obj' is an instance of HTMLEmbedElement :)


hasInstance?


FWIW, Java has .isInstance(obj):

https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#isInstance-java.lang.Object-

--
Stanimir
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Boris Zbarsky]

On 9/15/17 9:58 AM, David Bruant wrote:

Maybe @@toStringTag will end up not working well enough for your need anyway.


Not least because its use is kinda ugly, no?


But another solution could be to define a chromeonly symbol for the brand.

obj[Symbol.brand] === 'HTMLEmbedElement'
(`Symbol.brand` is chromeonly. `obj[Symbol.brand]` too)


That doesn't solve the problem of detecting whether 'obj' is an 
HTMLElement, does it?


You could have obj[Symbol.brand] be an array of all the relevant brands, 
and do:


 obj[Symbol.brand].includes('HTMLEmbedElement')

but now we're getting into pretty ugly and rather non-obvious territory.

(There's also a slight concern about practicality here, because we do 
not want to define a brand property on every single object for 
performance and memory reasons, but that can probably be addressed by 
having an accessor property for Symbol.brand on the proto that checks 
the 'this' value to decide what to return... Even then, I expect that 
this solution is much slower than the other ones proposed here, because 
it relies on property access across Xrays, whereas the other things that 
were proposed do not.)



No function call, no string to allocate, nothing to import (`Symbol` is a 
standard ECMAScript global).


There's a hidden function call here to call the getter.  And an array 
allocation for the return value, likely, or lots of machinery to try to 
cache it somewhere hidden on the proto.  But yes, nothing to import.  ;)


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[David Bruant]

Hi,

Sorry, arriving a bit late to the party.
I was about to propose something related to @@toStringTag, but reading the 
discussions about how it may/will work [1][2][3], i realize it may not be your 
preferred solution.

Maybe @@toStringTag will end up not working well enough for your need anyway.
But another solution could be to define a chromeonly symbol for the brand.

obj[Symbol.brand] === 'HTMLEmbedElement'
(`Symbol.brand` is chromeonly. `obj[Symbol.brand]` too)

No function call, no string to allocate, nothing to import (`Symbol` is a 
standard ECMAScript global).
It might look weird because symbols are new, but maybe it's just something to 
get used to, hard to tell.

David

[1] https://github.com/heycam/webidl/issues/54
[2] https://github.com/heycam/webidl/pull/357
[3] https://github.com/heycam/webidl/issues/419


Le vendredi 1 septembre 2017 17:01:58 UTC+2, Boris Zbarsky a écrit :
> Now that we control all the code that can attempt to touch 
> Components.interfaces.nsIDOM*, we can try to get rid of these interfaces 
> and their corresponding bloat.
> 
> The main issue is that we have consumers that use these for testing what 
> sort of object we have, like so:
> 
>if (obj instanceof Ci.nsIDOMWhatever)
> 
> and we'd need to find ways to make that work.  In some cases various 
> hacky workarounds are possible in terms of property names the object has 
> and maybe their values, but they can end up pretty non-intuitive and 
> fragile.  For example, this:
> 
>element instanceof Components.interfaces.nsIDOMHTMLEmbedElement
> 
> becomes:
> 
>element.localName === "embed" &&
>element.namespaceURI === "http://www.w3.org/1999/xhtml;
> 
> and even that is only OK at the callsite in question because we know it 
> came from 
> http://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/dom/interfaces/xul/nsIDOMXULCommandDispatcher.idl#17
>  
> and hence we know it really is an Element...
> 
> Anyway, we need a replacement.  Some possible options:
> 
> 1)  Use "obj instanceof Whatever".  The problem is that we'd like to 
> maybe kill off the cross-global instanceof behavior we have now for DOM 
> constructors.
> 
> 2)  Introduce chromeonly "is" methods on all DOM constructors.  So 
> "HTMLEmbedElement.is(obj)".  Possibly with some nicer but uniform name.
> 
> 3)  Introduce chromeonly "isWhatever" methods (akin to Array.isArray) on 
> DOM constructors.  So "HTMLEmbedElement.isHTMLEmbedElement(obj)".  Kinda 
> wordy and repetitive...
> 
> 4)  Something else?
> 
> Thoughts?  It really would be nice to get rid of some of this stuff 
> going forward.  And since the web platform seems to be punting on 
> branding, there's no existing solution we can use.
> 
> -Boris

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Anne van Kesteren]

On Fri, Sep 1, 2017 at 5:01 PM, Boris Zbarsky  wrote:
> Thoughts?  It really would be nice to get rid of some of this stuff going
> forward.  And since the web platform seems to be punting on branding,
> there's no existing solution we can use.

It's punting on exposing it to web developers, but we do want it
formalized per https://github.com/heycam/webidl/issues/97 in some kind
of form. An API that acts on [[PlatformBrand]] is probably the most
likely thing that other browsers might be interested in adopting down
the road, assuming they hit similar issues.

I'd personally favor a single global method with branching over
polluting each object, but I don't have strong rationale.

Also, do we need this for Promise, Map, Set, etc., or just IDL-defined objects?


-- 
https://annevankesteren.nl/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Boris Zbarsky]

On 9/1/17 4:54 PM, Kris Maglione wrote:
The other 
potential issue is that the `instanceof Ci.nsIDOM...` checks tend to 
appear in places like JSMs, where we don't have direct access to 
constructors for DOM interfaces by default.


Right, if we used one of the things that use constructors, we'd have to 
import them into the JSM scope to do these checks.


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Kris Maglione]

On Fri, Sep 01, 2017 at 04:43:31PM -0400, Ehsan Akhgari wrote:

On 09/01/2017 01:18 PM, Boris Zbarsky wrote:
That has the unfortunate side-effect of allocating a new JS string 
for every call, but we can probably optimize it if it becomes 
necessary.


We could add a hasClassName if we wanted.


I'm kind of missing what's better about this compared to the other 
suggestion of hasInstance() (thanks to Botond for catching my English 
mistake!)  I'm worried a bit because of the string compare that this 
involves.


I think for things like this, as micro-optimization-land as this may 
sound, we should keep performance in mind, and before making a call we 
should try to see which variant would be faster.  Where we're 
bikeshedding over the cosmetics of the API, we should consider letting 
the faster variant win.  It's easy to write this off as non-important 
right now but generic facilities like this are bound to be used some 
day in a hot loop somewhere and then fixing things may be too costly 
due to having to change all of the existing code.


Well, the main thing is that it already exists (well, the 
string-returning version), so it seemed worth mentioning. The other 
potential issue is that the `instanceof Ci.nsIDOM...` checks tend to 
appear in places like JSMs, where we don't have direct access to 
constructors for DOM interfaces by default.


In those cases, we tend to need something like `elem instanceof 
getGlobalForObject(elem).Element`, which is probably at least two orders 
of magnitude more expensive than the equivalent `hasClassName` call 
would be.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Ehsan Akhgari]

On 09/01/2017 01:18 PM, Boris Zbarsky wrote:
That has the unfortunate side-effect of allocating a new JS string 
for every call, but we can probably optimize it if it becomes necessary.


We could add a hasClassName if we wanted.


I'm kind of missing what's better about this compared to the other 
suggestion of hasInstance() (thanks to Botond for catching my English 
mistake!)  I'm worried a bit because of the string compare that this 
involves.


I think for things like this, as micro-optimization-land as this may 
sound, we should keep performance in mind, and before making a call we 
should try to see which variant would be faster.  Where we're 
bikeshedding over the cosmetics of the API, we should consider letting 
the faster variant win.  It's easy to write this off as non-important 
right now but generic facilities like this are bound to be used some day 
in a hot loop somewhere and then fixing things may be too costly due to 
having to change all of the existing code.


Cheers,
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Kris Maglione]

On Fri, Sep 01, 2017 at 01:18:12PM -0400, Boris Zbarsky wrote:
That has the unfortunate side-effect of allocating a new JS string 
for every call, but we can probably optimize it if it becomes 
necessary.


We could add a hasClassName if we wanted.


That's an interesting idea. It would also remove the need to 
convert to UTF-16, since we could just use EqualsASCII.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Boris Zbarsky]

On 9/1/17 1:06 PM, Kris Maglione wrote:

  if (ChromeUtils.getClassName(element) == "HTMLEmbedElement")


Ah, that works.  Though I think spidermonkey wants to maybe get rid of 
classnames and on the flip side it's not clear whether 
HTMLEmbedElement.prototype will end up with the same classname as 
HTMLEmbedElement instances. :(


That has the unfortunate side-effect of allocating a new JS string for 
every call, but we can probably optimize it if it becomes necessary.


We could add a hasClassName if we wanted.

-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Bobby Holley]

On Fri, Sep 1, 2017 at 9:59 AM, Botond Ballo  wrote:

> On Fri, Sep 1, 2017 at 12:11 PM, Ehsan Akhgari 
> wrote:
> > How about this slight variation:
> >
> > HTMLEmbedElement.isInstanceOf(obj)
> >
> > instead of "is"?
>
> Isn't that backwards, though? HTMLEmbedElement isn't an instance of
> 'obj', 'obj' is an instance of HTMLEmbedElement :)
>

hasInstance?


> Cheers,
> Botond
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Kris Maglione]

On Fri, Sep 01, 2017 at 11:01:51AM -0400, Boris Zbarsky wrote:
Now that we control all the code that can attempt to touch 
Components.interfaces.nsIDOM*, we can try to get rid of these 
interfaces and their corresponding bloat.


The main issue is that we have consumers that use these for testing 
what sort of object we have, like so:


 if (obj instanceof Ci.nsIDOMWhatever)

and we'd need to find ways to make that work.  In some cases various 
hacky workarounds are possible in terms of property names the object 
has and maybe their values, but they can end up pretty non-intuitive 
and fragile.  For example, this:


 element instanceof Components.interfaces.nsIDOMHTMLEmbedElement

becomes:

 element.localName === "embed" &&
 element.namespaceURI === "http://www.w3.org/1999/xhtml;


One possibility that should work, but is slightly ugly:

 if (ChromeUtils.getClassName(element) == "HTMLEmbedElement")

That has the unfortunate side-effect of allocating a new JS 
string for every call, but we can probably optimize it if it 
becomes necessary.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Botond Ballo]

On Fri, Sep 1, 2017 at 12:11 PM, Ehsan Akhgari  wrote:
> How about this slight variation:
>
> HTMLEmbedElement.isInstanceOf(obj)
>
> instead of "is"?

Isn't that backwards, though? HTMLEmbedElement isn't an instance of
'obj', 'obj' is an instance of HTMLEmbedElement :)

Cheers,
Botond
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Michael Layzell]

Accidentally sent this off-list.

On Fri, Sep 1, 2017 at 12:14 PM, Michael Layzell 
wrote:

> I personally like the style of (2) the most, The isWhatever style methods
> are too verbose, and I don't think that adding more code which depends on
> behavior we might want to remove from Firefox is a good idea.
>
> I'd probably not use the name `is` though, I'd be into ehsan's suggestion
> of `isInstanceOf` (or some variant on that) much more. If we make it wordy
> enough (e.g. isInstanceOf rather than is) then it'll be easier to change in
> the future if the web platform ever implements a good solution to the
> branding problem.
>
> On Fri, Sep 1, 2017 at 11:01 AM, Boris Zbarsky  wrote:
>
>> Now that we control all the code that can attempt to touch
>> Components.interfaces.nsIDOM*, we can try to get rid of these interfaces
>> and their corresponding bloat.
>>
>> The main issue is that we have consumers that use these for testing what
>> sort of object we have, like so:
>>
>>   if (obj instanceof Ci.nsIDOMWhatever)
>>
>> and we'd need to find ways to make that work.  In some cases various
>> hacky workarounds are possible in terms of property names the object has
>> and maybe their values, but they can end up pretty non-intuitive and
>> fragile.  For example, this:
>>
>>   element instanceof Components.interfaces.nsIDOMHTMLEmbedElement
>>
>> becomes:
>>
>>   element.localName === "embed" &&
>>   element.namespaceURI === "http://www.w3.org/1999/xhtml;
>>
>> and even that is only OK at the callsite in question because we know it
>> came from http://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2f
>> e7d7b6e75ad6b6b5da223/dom/interfaces/xul/nsIDOMXULCommandDis
>> patcher.idl#17 and hence we know it really is an Element...
>>
>> Anyway, we need a replacement.  Some possible options:
>>
>> 1)  Use "obj instanceof Whatever".  The problem is that we'd like to
>> maybe kill off the cross-global instanceof behavior we have now for DOM
>> constructors.
>>
>> 2)  Introduce chromeonly "is" methods on all DOM constructors.  So
>> "HTMLEmbedElement.is(obj)".  Possibly with some nicer but uniform name.
>>
>> 3)  Introduce chromeonly "isWhatever" methods (akin to Array.isArray) on
>> DOM constructors.  So "HTMLEmbedElement.isHTMLEmbedElement(obj)".  Kinda
>> wordy and repetitive...
>>
>> 4)  Something else?
>>
>> Thoughts?  It really would be nice to get rid of some of this stuff going
>> forward.  And since the web platform seems to be punting on branding,
>> there's no existing solution we can use.
>>
>> -Boris
>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Eiminating nsIDOM* interfaces and brand checks

[Ehsan Akhgari]

On 09/01/2017 11:01 AM, Boris Zbarsky wrote:

Anyway, we need a replacement.  Some possible options:

1)  Use "obj instanceof Whatever".  The problem is that we'd like to 
maybe kill off the cross-global instanceof behavior we have now for 
DOM constructors.


2)  Introduce chromeonly "is" methods on all DOM constructors.  So 
"HTMLEmbedElement.is(obj)".  Possibly with some nicer but uniform name.


How about this slight variation:

HTMLEmbedElement.isInstanceOf(obj)

instead of "is"?

Also, is this something that we can codegen in WebIDL based on the 
presence of [HTMLConstructor] or something like that?


3) Introduce chromeonly "isWhatever" methods (akin to Array.isArray) 
on DOM constructors.  So "HTMLEmbedElement.isHTMLEmbedElement(obj)".  
Kinda wordy and repetitive...


4)  Something else?

Thoughts?  It really would be nice to get rid of some of this stuff 
going forward.  And since the web platform seems to be punting on 
branding, there's no existing solution we can use.


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Eiminating nsIDOM* interfaces and brand checks

[Boris Zbarsky]

Now that we control all the code that can attempt to touch 
Components.interfaces.nsIDOM*, we can try to get rid of these interfaces 
and their corresponding bloat.


The main issue is that we have consumers that use these for testing what 
sort of object we have, like so:


  if (obj instanceof Ci.nsIDOMWhatever)

and we'd need to find ways to make that work.  In some cases various 
hacky workarounds are possible in terms of property names the object has 
and maybe their values, but they can end up pretty non-intuitive and 
fragile.  For example, this:


  element instanceof Components.interfaces.nsIDOMHTMLEmbedElement

becomes:

  element.localName === "embed" &&
  element.namespaceURI === "http://www.w3.org/1999/xhtml;

and even that is only OK at the callsite in question because we know it 
came from 
http://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/dom/interfaces/xul/nsIDOMXULCommandDispatcher.idl#17 
and hence we know it really is an Element...


Anyway, we need a replacement.  Some possible options:

1)  Use "obj instanceof Whatever".  The problem is that we'd like to 
maybe kill off the cross-global instanceof behavior we have now for DOM 
constructors.


2)  Introduce chromeonly "is" methods on all DOM constructors.  So 
"HTMLEmbedElement.is(obj)".  Possibly with some nicer but uniform name.


3)  Introduce chromeonly "isWhatever" methods (akin to Array.isArray) on 
DOM constructors.  So "HTMLEmbedElement.isHTMLEmbedElement(obj)".  Kinda 
wordy and repetitive...


4)  Something else?

Thoughts?  It really would be nice to get rid of some of this stuff 
going forward.  And since the web platform seems to be punting on 
branding, there's no existing solution we can use.


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform