1297717 - Global objects shouldn't have a unforgeable holder

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Anthony Ramine [:nox]]

The unforgeable holder is just here for performance for non-global interfaces to be initialised fast, it is unnecessary overhead to use one in the case of globals.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Stop using an unforgeable holder for global objects with unforgeable properties, since it's just pure slowdown in that case

Comment 2

[Peter Van der Beken [:peterv]]

Comment on attachment 8785021 [details] [diff] [review]
Stop using an unforgeable holder for global objects with unforgeable properties, since it's just pure slowdown in that case

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

::: dom/bindings/Codegen.py
@@ +3726,5 @@
>              $*{assertions}
>              MOZ_ASSERT(ToSupportsIsOnPrimaryInheritanceChain(aObject, aCache),
>                         "nsISupports must be on our primary inheritance chain");
>  
> +            CreateGlobal<${nativeType}, GetProtoObjectHandle>(aCx,

I was going to ask you to change the signature of CreateGlobal to return void, but it's kind of messed up already. I think it should change to bool and instead of checking !aReflector we should check the bool to return false on failure (maybe even using failureCode here). In any case, not using the return value in the only caller signals a problem with the signature :-).

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yeah, good catch.  The code before my changes is daft too (e.g. will treat success allocating but failure to define various stuff as all ok, because aReflector is non-null)!  Will fix.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Of course if you look up the callstack, the caller kinda ignores the return value anyway, sigh.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Stop using an unforgeable holder for global objects with unforgeable properties, since it's just pure slowdown in that case

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Interdiff from first patch

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 8787373 [details] [diff] [review]
Stop using an unforgeable holder for global objects with unforgeable properties, since it's just pure slowdown in that case

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

Thanks for cleaning that up, it was pretty messed up already.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/256ea0d0de34
Stop using an unforgeable holder for global objects with unforgeable properties, since it's just pure slowdown in that case.  r=peterv

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/256ea0d0de34