Closed Bug 963380 Opened 10 years ago Closed 10 years ago

Improve codegen for [Cached] attributes with multiple wrapping paths

Categories

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

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file)

Right now we end up duplicating all the "store in the slot" code...  For example, something like this:

  [Cached, Pure, Frozen]
  readonly attribute Dict? readonlyFrozenNullableDictionary;

leads to wrapping code like so:

  do { // block we break out of when done wrapping
    // Make sure we wrap and store in the slot in reflector's compartment
    JSAutoCompartment ac(cx, reflector);
    if (result.IsNull()) {
      args.rval().setNull();
      if (args.rval().isObject()) {
        JS::Rooted<JSObject*> rvalObj(cx, &args.rval().toObject());
        if (!JS_FreezeObject(cx, rvalObj)) {
          return false;
        }
      }
      js::SetReservedSlot(reflector, (DOM_INSTANCE_RESERVED_SLOTS + 12), args.rval());
      PreserveWrapper(self);
      break;
    }
    if (!result.Value().ToObject(cx, obj, args.rval())) {
      return false;
    }
    if (args.rval().isObject()) {
      JS::Rooted<JSObject*> rvalObj(cx, &args.rval().toObject());
      if (!JS_FreezeObject(cx, rvalObj)) {
        return false;
      }
    }
    js::SetReservedSlot(reflector, (DOM_INSTANCE_RESERVED_SLOTS + 12), args.rval());
    PreserveWrapper(self);
    break;
  } while (0);
  // And now make sure args.rval() is in the caller compartment
  return MaybeWrapNonDOMObjectOrNullValue(cx, args.rval());

whereas after the changes I'm about to post it looks more like so:

  { // Make sure we wrap and store in the slot in reflector's compartment
    JSAutoCompartment ac(cx, reflector);
    do { // block we break out of when done wrapping
      if (result.IsNull()) {
        args.rval().setNull();
        break;
      }
      if (!result.Value().ToObject(cx, obj, args.rval())) {
        return false;
      }
      break;
    } while (0);
    if (args.rval().isObject()) {
      JS::Rooted<JSObject*> rvalObj(cx, &args.rval().toObject());
      if (!JS_FreezeObject(cx, rvalObj)) {
        return false;
      }
    }
    js::SetReservedSlot(reflector, (DOM_INSTANCE_RESERVED_SLOTS + 12), args.rval());
    PreserveWrapper(self);
  }
  // And now make sure args.rval() is in the caller compartment
  return MaybeWrapNonDOMObjectOrNullValue(cx, args.rval());
Whiteboard: [need review]
Attachment #8364813 - Flags: review?(peterv) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fe4bf160969
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla30
https://hg.mozilla.org/mozilla-central/rev/6fe4bf160969
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: