Get rid of the system bit

RESOLVED DUPLICATE of bug 774607

Status

()

Core
JavaScript Engine
RESOLVED DUPLICATE of bug 774607
8 years ago
6 years ago

People

(Reporter: mrbkap, Assigned: mrbkap)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [compartments])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 years ago
It looks like the system bit incorrectly approximates whether or not a given global object is chrome. We should remove it. Eventually we'll replace this by using compartments and asking them (see the comment in XPCWrappedNativeScope::GetWrapperFor for why this is not exactly equivalent to just using object principals), but for now it would be good get rid of the system bit altogether.
(Assignee)

Comment 1

8 years ago
Created attachment 453452 [details] [diff] [review]
Patch
Attachment #453452 - Flags: review?(gal)

Updated

8 years ago
Attachment #453452 - Flags: review?(gal) → review+

Comment 2

8 years ago
Comment on attachment 453452 [details] [diff] [review]
Patch

>+static inline PRBool
>+IsSystemObject(JSContext *cx, JSObject *obj)
>+{
>+  const char *name = JS_GetGlobalForObject(cx, obj)->getClass()->name;
>+  return !strcmp(name, "ChromeWindow") ||
>+         !strcmp(name, "BackstagePass");
>+}

There are no singleton classes we could do a pointer comparison with. Eww.

Its a bit unfortunate to have this in two places. Easy to miss changing.
How much of a slowdown is walking the parent chain?
You missed updating this part of the comment in jsobj.h:

 * The classword member stores the JSClass pointer for this object, with the
 * least two bits encoding whether this object is a "delegate" or a "system"
 * object. We do *not* synchronize updates of classword -- API clients must
 * take care.

More importantly, this'll conflict badly with Brendan's work in 558451, which separates the delegate|system flag from the clasp.  Furthermore, Luke has done a similar flags/clasp split in the fatvals branch in partial anticipation of Brendan's change.  So I'd suggest this should wait.
(Assignee)

Comment 5

8 years ago
Okay... I'll remove the API in a separate bug.
(Assignee)

Comment 6

8 years ago
(In reply to comment #3)
> How much of a slowdown is walking the parent chain?

Probably not measurably; the problem is that we have ended up with cases where objects *should* be treated as though they were system (chrome), but because they aren't fully initialized pretend to be content. Compartments will help fix this (I think), but until then, we need some way of getting around that.
(Assignee)

Comment 7

8 years ago
Created attachment 453848 [details] [diff] [review]
patch
Attachment #453452 - Attachment is obsolete: true
Attachment #453848 - Flags: review?(gal)

Updated

8 years ago
Attachment #453848 - Flags: review?(gal) → review+
Whiteboard: [compartments]
(Assignee)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 774607
You need to log in before you can comment on or make changes to this bug.