[webkit-reviews] review denied: [Bug 61343] [Qt] JSC bridge: implement __qt_sender__ without using Scope Chain : [Attachment 94569] Patch

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has denied Caio Marcelo de Oliveira Filho
<cmarcelo at webkit.org>'s request for review:
Bug 61343: [Qt] JSC bridge: implement __qt_sender__ without using Scope Chain
https://bugs.webkit.org/show_bug.cgi?id=61343

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

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=94569&action=review

> Source/WebCore/bridge/qt/qt_instance.h:79
> +	   void pop() { Q_ASSERT(!m_stack.isEmpty()); m_stack.pop(); }

Curious to have a stack with a pop() method that doesn't return the popped
value. No real objection though.

> Source/WebKit/qt/Api/qwebframe.cpp:496
> +    // ### Is this the correct RootObject?

This sounds like something that should be answered before landing.

> Source/WebKit/qt/Api/qwebframe.cpp:516
> +    // ### Can we do this with public JSC API?

Ditto.

> Source/WebKit/qt/Api/qwebframe_p.h:111
> +    void didClearedWindowObject();

Should be didClearWindowObject().

> Source/WebKit/qt/Api/qwebframe_p.h:134
> +    void addQtSenderToGlobalObject();

Missing #if USE(JSC) guard.