Bug 56346 – Made JSWrapperObject and subclasses moving-GC-safe

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Geoffrey Garen]

Made JSWrapperObject and subclasses moving-GC-safe

[Geoffrey Garen]

Created attachment 85745 [details]
Patch

[Darin Adler]

Comment on attachment 85745 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=85745&action=review

> Source/JavaScriptCore/ChangeLog:21
> +        no longer does so for us. Also added an ASSERT to catch a latent bug,
> +        where DatePrototype stomped on its base class's anonymous slot. Hard-coded

What was the symptom of this “stomping”?

> Source/JavaScriptCore/ChangeLog:30
> +        data member which was not always visited during GC. Now, we only use the
> +        data member, and we always visit it.

I don’t see any change to the code to always visit the data member. Is the change just the change to StructureFlags? If so it might be better to say so outright.

> Source/JavaScriptCore/ChangeLog:34
> +        * runtime/NumberObject.cpp:
> +        (JSC::NumberObject::NumberObject):
> +        * runtime/NumberObject.h: Removed meaningless declaration.

NumberObject.cpp here doesn’t have a comment.

> Source/JavaScriptCore/runtime/BooleanObject.cpp:31
> +BooleanObject::BooleanObject(JSGlobalData&, NonNullPassRefPtr<Structure> structure)
> +    : JSWrapperObject(structure)

Do we want to change the constructor arguments eventually to not even pass the global data?

[Geoffrey Garen]

Committed r81086: <http://trac.webkit.org/changeset/81086>

[Geoffrey Garen]

(In reply to comment #2)
> What was the symptom of this “stomping”?

Updated ChangeLog to explain the symptom.

> I don’t see any change to the code to always visit the data member. Is the change just the change to StructureFlags? If so it might be better to say so outright.

Yes. Updated ChangeLog to mention the flag.

> NumberObject.cpp here doesn’t have a comment.

Fixed.

> Do we want to change the constructor arguments eventually to not even pass the global data?

Yes.