Bug 27931 – JSNodeCustom should call document() instead of scriptExecutionContext()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Adam Barth]

As required by Darin Adler.

[Darin Adler]

The key issue here is that we want to call an inline function, not a virtual function.

The inline function could be document() or it could be an inline version of scriptExecutionContext(). See the Node::prefix() function for an example of this design pattern.

[Adam Barth]

Created attachment 33947 [details]
Patch v1

[Darin Adler]

Comment on attachment 33947 [details]
Patch v1

On reflection, I don't think this patch is right.

Document is derived from ScriptExecutionContext, so there's no need to overload toJSDOMGlobalObject to take a Document*. The JSNodeCustom.cpp change can stand alone. No need to change JSDOMGLobalObject.h and .cpp.

I'm changing my review to review-.

[Adam Barth]

> No need to change JSDOMGLobalObject.h and .cpp.

True.  I guess all you're saving is the fake RTTI branch.  I'll spin up a new version of the patch tomorrow.  I have the Node* change in my try at the moment, so it will take a bit of time to rebuild.

[Darin Adler]

(In reply to comment #4)
> > No need to change JSDOMGLobalObject.h and .cpp.
> 
> True.  I guess all you're saving is the fake RTTI branch.  I'll spin up a new
> version of the patch tomorrow.  I have the Node* change in my try at the
> moment, so it will take a bit of time to rebuild.

I must be really tired! I think it *is* worthwhile to save the fake RTTI branch. Sorry for going back and forth on this so much.

[Darin Adler]

Comment on attachment 33947 [details]
Patch v1

I changed my mind again and I think this patch is good as-is.

[Adam Barth]

Comment on attachment 33947 [details]
Patch v1

Clearing review flag on attachment: 33947

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/bindings/js/JSDOMGlobalObject.cpp
	M	WebCore/bindings/js/JSDOMGlobalObject.h
	M	WebCore/bindings/js/JSNodeCustom.cpp
Committed r46710
	M	WebCore/ChangeLog
	M	WebCore/bindings/js/JSDOMGlobalObject.h
	M	WebCore/bindings/js/JSDOMGlobalObject.cpp
	M	WebCore/bindings/js/JSNodeCustom.cpp
r46710 = 8e29f864509f257cdb9711b14c33ff706cb3e96b (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46710

[Adam Barth]

All reviewed patches have been landed.  Closing bug.