796410 - "Assertion failure: !defined || enabled (We defined a constructor but the new bindings are disabled?)"

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Jesse Ruderman]

Attachment: stack trace

Starting Firefox with
    user_pref("dom.experimental_bindings", false);

Fails:
Assertion failure: !defined || enabled (We defined a constructor but the new bindings are disabled?), at /Users/jruderman/trees/mozilla-central/dom/bindings/DOMJSProxyHandler.cpp:57

Regression from something in
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=fb076a446870
I'm guessing bug 791774

Comment 1

[Boris Zbarsky [:bzbarsky]]

Yes, indeed.

Peter, why are we even checking ExperimentalBindingsEnabled in DefineDOMInterface and Wrap() in the Paris bindings code?

Also, in the case when !enabled what happens in HTMLCollectionWrapper::DefineDOMInterface (why _are_ we calling that at all?) is that we call getPrototype() and return the result but leave the boolean as false.  So yeah, this is broken.

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

All I can find is

bool
DefineDOMInterface(JSContext* aCx, JSObject* aReceiver, bool* aEnabled)
{
  JSObject* global = JS_GetGlobalForObject(aCx, aReceiver);

  {
    XPCWrappedNativeScope* scope =
      XPCWrappedNativeScope::FindInJSObjectScope(aCx, global);
    if (!scope) {
      *aEnabled = false;
      return false;
    }

    if (!scope->ExperimentalBindingsEnabled()) {
      *aEnabled = false;
      return false;
    }
  }

  *aEnabled = true;
  return !!GetProtoObject(aCx, global, aReceiver);
}

which seems like it shouldn't hit the assertion. Am I looking at the wrong code?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yes, you are.  You should be looking at the generated code in js/xpconnect/src/dombindings_gen.cpp, which is what ends up being called over here.

Comment 4

[Boris Zbarsky [:bzbarsky]]

And the reason for that is:

<Ms2ger>   mozilla::dom::Register(nameSpaceManager);
<Ms2ger>   // This needs to happen after the call to mozilla::dom::Register because we
<Ms2ger>   // overwrite some values.
<Ms2ger>   mozilla::dom::oldproxybindings::Register(nameSpaceManager);

Comment 5

[Boris Zbarsky [:bzbarsky]]

Peter, what's the actual goal here?  Do we want to allow preffing off the Paris bindings for lists but not the old proxy bindings or something?

Comment 6

[Peter Van der Beken [:peterv]]

Yes. So the DefineDOMInterface for old proxy bindings just needs to set *enabled to true. The old bindings haven't been controlled by the pref for a while.

Comment 7

[Boris Zbarsky [:bzbarsky]]

But why are the new bindings looking at the boolean from the _old_ ones?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Or is that just to make it easy to flip all the new list bindings off without flipping off any other Paris bindings?

Comment 9

[Peter Van der Beken [:peterv]]

(In reply to Boris Zbarsky (:bz) from comment #7)
> But why are the new bindings looking at the boolean from the _old_ ones?

Where are they doing that? I don't see any calls to NewDOMBindingsEnabled.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Oh, hmm.  "dom.experimental_bindings" is the pref for the Paris bindings?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Make things still work right if we disable Paris list bindings.

Comment 12

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/221cf927f71d

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/221cf927f71d