Closed Bug 796410 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: XPConnect, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: jruderman, Assigned: bzbarsky)

References

Details

(Keywords: assertion, regression)

Attachments

(2 files)

Attached file 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
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.
Blocks: 791774
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?
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.
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);
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?
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.
But why are the new bindings looking at the boolean from the _old_ ones?
Or is that just to make it easy to flip all the new list bindings off without flipping off any other Paris bindings?
(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.
Oh, hmm.  "dom.experimental_bindings" is the pref for the Paris bindings?
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Attachment #668708 - Flags: review?(peterv) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/221cf927f71d
Whiteboard: [need review]
Target Milestone: --- → mozilla18
https://hg.mozilla.org/mozilla-central/rev/221cf927f71d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.