Bug 21294 – Devirtualize getOwnPropertySlot()

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Cameron Zwarich (cpst)]

We always use a virtual call for getOwnPropertySlot(), when we should really be able to inline the common cases in many of its callers, or at least use non-virtual calls. This could be achieved by a bit in the StructureID TypeInfo, just like was done for 'instanceof'.

[Cameron Zwarich (cpst)]

Sam is now working on other tasks, so I will assign this to myself.

[Darin Adler]

*** Bug 21793 has been marked as a duplicate of this bug. ***

[Darin Adler]

Created attachment 24559 [details]
patch, working but ChangeLog not done yet

[Darin Adler]

Created attachment 24561 [details]
patch

[Sam Weinig]

Comment on attachment 24561 [details]
patch

-    // WebCore uses this to make document.all and style.filter undetectable.
     static const unsigned MasqueradesAsUndefined = 1;
+        // WebCore uses MasqueradesAsUndefined to make document.all and style.filter undetectable.
     static const unsigned ImplementsHasInstance = 1 << 1;
The whitespace for this comment seems wrong.

+JSGlueGlobalObject::JSGlueGlobalObject(PassRefPtr<StructureID> structure, JSFlags flags)
+    : JSGlobalObject(structure, new Data, this)
+{
+    d()->flags = flags;
+    d()->userObjectStructure = UserObjectImp::createStructureID(jsNull());
+}
Might be cleaner to make the Data constructor take arguments.

+    if (prototype->isNull()) {
+        static StructureID* structureID = JSInspectorCallbackWrapper::createStructureID(jsNull()).releaseRef();
+        return new (unwrappedExec) JSInspectorCallbackWrapper(unwrappedExec, unwrappedObject, structureID);
+    }
Why is leaking the StructureID okay here?

For QtRuntimeMethod::createStructureID, why does it have the ImplementsHasInstance flag like InternalFunction?

r=me.

[Darin Adler]

(In reply to comment #5)
> (From update of attachment 24561 [details] [edit])
> -    // WebCore uses this to make document.all and style.filter undetectable.
>      static const unsigned MasqueradesAsUndefined = 1;
> +        // WebCore uses MasqueradesAsUndefined to make document.all and
> style.filter undetectable.
>      static const unsigned ImplementsHasInstance = 1 << 1;
> The whitespace for this comment seems wrong.

I'll move the comment back.

> +    d()->flags = flags;
>
> Might be cleaner to make the Data constructor take arguments.

Sure, but since this is just JavaScriptGlue, I think I'll leave it as-is.

> +    if (prototype->isNull()) {
> +        static StructureID* structureID =
> JSInspectorCallbackWrapper::createStructureID(jsNull()).releaseRef();
> +        return new (unwrappedExec) JSInspectorCallbackWrapper(unwrappedExec,
> unwrappedObject, structureID);
> +    }
>
> Why is leaking the StructureID okay here?

As we discussed, I'll use StructureID::startIgnoringLeaks().

> For QtRuntimeMethod::createStructureID, why does it have the
> ImplementsHasInstance flag like InternalFunction?

Oops, will fix.

[Darin Adler]

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