[webkit-reviews] review denied: [Bug 45145] [Qt] V8 port for QT platform: v8 binding changes : [Attachment 66574] Included plugins changes b/c of dependency
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Sep 5 23:28:49 PDT 2010
Adam Barth <abarth at webkit.org> has denied Vlad <vladbph at gmail.com>'s request
for review:
Bug 45145: [Qt] V8 port for QT platform: v8 binding changes
https://bugs.webkit.org/show_bug.cgi?id=45145
Attachment 66574: Included plugins changes b/c of dependency
https://bugs.webkit.org/attachment.cgi?id=66574&action=review
------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context:
https://bugs.webkit.org/attachment.cgi?id=66574&action=prettypatch
> WebCore/bindings/v8/ScriptController.cpp:74
> +#if PLATFORM(QT)
> +PassRefPtr<DOMWrapperWorld> ScriptController::createWorld()
> +{
> + return DOMWrapperWorld::create();
> +}
> +#endif
This can't be right. It's not Qt-specific. We need to integrate V8 with
DOMWrapperWorld in genera.
> WebCore/bindings/v8/V8Binding.cpp:54
> - : m_plainString(string)
> + : m_plainString(reinterpret_cast<const WTF::String &>(string))
I don't understand this code. Does Qt use different string types than other
ports? reinterpret_cast is almost always wrong.
> WebCore/plugins/win/PluginViewWin.cpp:661
> +#if USE(JSC)
> JSC::JSLock::DropAllLocks dropAllLocks(JSC::SilenceAssertionsOnly);
> +#endif
This ifdefs point to a missing abstraction. Perhaps a ScriptLock object?
More information about the webkit-reviews
mailing list