All DOMConstructorObjects should hold a pointer to the JSDOMGlobalObject All DOMConstructorObject subclasses are already passed a JSDOMGlobalObject* during construction. Only 4 of them actually hold onto this pointer though. We should make all constructor objects hold onto this pointer, and share the marking and casting code using a subclass instead of the current copy/paste code.
Created attachment 33135 [details] patch
+ explicit DOMObjectWithGlobalPointer(PassRefPtr<JSC::Structure> structure, JSDOMGlobalObject* globalObject) Explicit only makes sense for one-argument constructors. WebCore/bindings/js/JSMessageChannelConstructor.cpp + , m_globalObject(globalObject) Can't we remove this line now? Maciej had some suggestions in the channel. I'll wait for you to address those comments before reviewing this in more detail.
Created attachment 33140 [details] Update patch
Comment on attachment 33140 [details] Update patch This patch is awesome. Redundant code: BE GONE! + I've added two new classes: DOMObjectWithGlobalPointer and DOMConstructorObjectInDocument. The ChangeLog uses the old class names. + class DOMConstructorWithDocument : public DOMConstructorObject { + public: + Document* document() const + { + return static_cast<Document*>(scriptExecutionContext()); + } + protected: Missing a blank line here.
(In reply to comment #4) > (From update of attachment 33140 [details]) > This patch is awesome. Redundant code: BE GONE! > > + I've added two new classes: DOMObjectWithGlobalPointer and > DOMConstructorObjectInDocument. > > The ChangeLog uses the old class names. My bad. Will fix. > + class DOMConstructorWithDocument : public DOMConstructorObject { > + public: > + Document* document() const > + { > + return static_cast<Document*>(scriptExecutionContext()); > + } > + protected: > > Missing a blank line here. Where? Before protected?
> Where? Before protected? Yes.
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/bindings/js/JSAudioConstructor.cpp M WebCore/bindings/js/JSAudioConstructor.h M WebCore/bindings/js/JSDOMBinding.h M WebCore/bindings/js/JSImageConstructor.cpp M WebCore/bindings/js/JSImageConstructor.h M WebCore/bindings/js/JSMessageChannelConstructor.cpp M WebCore/bindings/js/JSMessageChannelConstructor.h M WebCore/bindings/js/JSOptionConstructor.cpp M WebCore/bindings/js/JSOptionConstructor.h M WebCore/bindings/js/JSWebKitCSSMatrixConstructor.cpp M WebCore/bindings/js/JSWebKitPointConstructor.cpp M WebCore/bindings/js/JSWorkerConstructor.cpp M WebCore/bindings/js/JSXMLHttpRequestConstructor.cpp M WebCore/bindings/js/JSXMLHttpRequestConstructor.h M WebCore/bindings/js/JSXSLTProcessorConstructor.cpp Committed r46191 M WebCore/ChangeLog M WebCore/bindings/js/JSXMLHttpRequestConstructor.h M WebCore/bindings/js/JSAudioConstructor.h M WebCore/bindings/js/JSWorkerConstructor.cpp M WebCore/bindings/js/JSOptionConstructor.h M WebCore/bindings/js/JSWebKitPointConstructor.cpp M WebCore/bindings/js/JSOptionConstructor.cpp M WebCore/bindings/js/JSXMLHttpRequestConstructor.cpp M WebCore/bindings/js/JSImageConstructor.cpp M WebCore/bindings/js/JSDOMBinding.h M WebCore/bindings/js/JSMessageChannelConstructor.h M WebCore/bindings/js/JSAudioConstructor.cpp M WebCore/bindings/js/JSWebKitCSSMatrixConstructor.cpp M WebCore/bindings/js/JSImageConstructor.h M WebCore/bindings/js/JSXSLTProcessorConstructor.cpp M WebCore/bindings/js/JSMessageChannelConstructor.cpp r46191 = df9f155678da8d894d73bfb7bf58ab24e14fc872 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46191