Bug 45147 – [Qt] V8 port for QT platform: Plugin changes

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Vlad]

Get rid of JSC::JSLock. Use coomon types definitions for V8 and JSC from scriptcontroller. Implemented PassScriptInstance PluginView::bindingInstance() for V8.

[Vlad]

Created attachment 66441 [details]
Plugin changes

[Vlad]

Created attachment 66454 [details]
Plugin changes

Get rid of JSC::JSLock. Use coomon types definitions for V8 and JSC from scriptcontroller. Implemented PassScriptInstance pluginView::bindingInstance() for V8.

[Vlad]

Changes moved to https://bugs.webkit.org/show_bug.cgi?id=45145 b/c of dependencies

[Andreas Kling]

Comment on attachment 66454 [details]
Plugin changes

> +#if PLATFORM(QT)
> +    #undef True
> +    #undef False

Why is this needed? There's no reference to it in the ChangeLog.

>  #if USE(JSC)
>          ScriptState* scriptState = parentFrame->script()->globalObject(pluginWorld())->globalExec();
> -#elif USE(V8)
> +#else
>          ScriptState* scriptState = 0; // Not used with V8
>  #endif

This part should be left alone.

> +// definition fits V8 and JSC

Unnecessary comment.
More importantly though, bindingInstance() will now be compiled but never used for Chromium. We should be nice and use an "#if USE(JSC) || (USE(V8) && !PLATFORM(CHROMIUM))" guard.

> +#else
> +#include "V8NPObject.h"
> +#endif

Should be "#elif USE(V8)".

r- because of the bindingInstance() on Chromium thing.

[Andreas Kling]

(In reply to comment #4)
> (From update of attachment 66454 [details])
...

Oops, looks like I just reviewed an obsolete patch! In the future please remove the "review?" flag from patches that are no longer relevant, to prevent them from showing up in the review queue. :-)

[Kenneth Rohde Christiansen]

Also please use 'Qt' and not 'QT' when you do Qt related patches as QT means QuickTime and that is used in the WebKit project as well.