[webkit-reviews] review granted: [Bug 113864] [Qt] Add getter for JSC TestRunner to DumpRenderTreeQt. : [Attachment 196318] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Zeno Albisser
<zeno at webkit.org>'s request for review:
Bug 113864: [Qt] Add getter for JSC TestRunner to DumpRenderTreeQt.
https://bugs.webkit.org/show_bug.cgi?id=113864

Attachment 196318: Patch
https://bugs.webkit.org/attachment.cgi?id=196318&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=196318&action=review


r=me, but the getter needs to be fixed before landing IMHO

> Tools/ChangeLog:7
> +	   As long as we instantiate both TestRunner and TestRunnerQt,
> +	   we need a separate getter for the non-Qt TestRunner instance.

ChangeLog shoudl also mention that this moves setDefersLoading at the same
time. I think given the size of this patch it's okay if both go in at the same
time. Just maybe fix the ChangeLog text before landing :)

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.h:92
> +    PassRefPtr<TestRunner> jscTestRunner() const { return m_jscController; }


I think the common pattern is to use PassRefPtr as return value for create
functions, but use raw pointers for getters, i.e. TestRunner* jscTestRunner() {
return m_jscController.get(); }

If the caller wants to keep a ref, then the RefPtr ctor that takes a raw
pointer will do the ref. Otherwise I think PassRefPtr doesn't give us anything
here.