Bug 20981 – Store per-class info in StructureID

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Maciej Stachowiak]

Info that varies per JSCell class, currently often handled as virtual methods, should be stored in the StructureID.

[Maciej Stachowiak]

Created attachment 23638 [details]
work in progress

[Maciej Stachowiak]

Created attachment 23639 [details]
add per-class TypeInfo to the StructureID (right now only used for JSType)

[Darin Adler]

Comment on attachment 23639 [details]
add per-class TypeInfo to the StructureID (right now only used for JSType)

I see this:

    OBJECT_OFFSET(StructureID, m_typeInfo) + OBJECT_OFFSET(TypeInfo, m_type)

Sometimes I've written that as:

    OBJECT_OFFSET(StructureID, m_typeInfo.m_type)

Not sure the pros and cons.

+#include "JSCallbackConstructor.h"
+#include "JSCallbackFunction.h"
+#include "JSCallbackObject.h"

What's this change to AllInOneFile.cpp about? Doesn't seem right to me. Maybe you pasted here by accident meaning to paste into JSGlobalObject.cpp at one point?

-        return m_structureID->type() == ObjectType;
+        return m_structureID->typeInfo().type() == ObjectType;

You could have sugared this by retaining a StructureID;:type() function. Why did you chose not to?

+#include "JSCallbackConstructor.h"
+#include "JSCallbackFunction.h"
+#include "JSCallbackObject.h"
 #include "ArrayConstructor.h"

Why aren't these sorted alphabetically in JSGlobalObject.cpp?

+++ JavaScriptCore/kjs/RegExpMatchesArray.h	(revision 0)
@@ -0,0 +1,47 @@
+/*
+ *  Copyright (C) 1999-2000 Harri Porten ([e-mail])
+ *  Copyright (C) 2003, 2007, 2008 Apple Inc. All Rights Reserved.

This class is new this year and doesn't need the pre-2008 copyrights on its source file.

+        TypeInfo(JSType type) : m_type(type) {}

We normally put spaces in those braces.

+    JSC::StructureID* createDOMStructure(JSC::ExecState*, CreateStructureIDFunction, const JSC::ClassInfo*, JSC::JSObject* prototype);

It's not helpful to pass the function for creating the structure instead of calling it. There's really no reason to do it that way. This should take a PassRefPtr<StructureID> instead, not take a prototype at all, and be renamed to cacheDOMStructure.

r=me

[Maciej Stachowiak]

Landed this with most suggestions applied.

[Darin Adler]

http://trac.webkit.org/changeset/36778