Bug 45149 – [Qt] V8 port for Qt platform: Qt WebCoreSupport changes

• Component: New Bugs
• Status: RESOLVED FIXED

[Vlad]

V8 changes in
WebCoreSupport/FrameLoaderClientQt.cpp
WebCoreSupport/FrameLoaderClientQt.h
WebCoreSupport/DumpRenderTreeSupportQt.cpp

[Vlad]

Created attachment 66444 [details]
QT WebCoreSupport changes

[Andreas Kling]

Comment on attachment 66444 [details]
QT WebCoreSupport changes

> +#if USE(JSC)
>      return JSDOMWindowBase::commonJSGlobalData()->heap.globalObjectCount();
> +#else
> +    // TBD
> +    return 1;
> +#endif

What's the plan for this?

> +#if 0

For disabled code blocks (and workarounds like the above), please include a comment saying why it's disabled (and if applicable, a bug# where we track any dependencies of the disabled code.)

This patch then digresses into style cleanups. While nice, please do that in a separate patch.

[Vlad]

(In reply to comment #2)
There is no such API in v8 AFAIK
> (From update of attachment 66444 [details])
> > +#if USE(JSC)
> >      return JSDOMWindowBase::commonJSGlobalData()->heap.globalObjectCount();
> > +#else
> > +    // TBD
> > +    return 1;
> > +#endif
> 
> What's the plan for this?
> 
Has to be enabled. #if 0 removed.
> > +#if 0
> 
> For disabled code blocks (and workarounds like the above), please include a comment saying why it's disabled (and if applicable, a bug# where we track any dependencies of the disabled code.)
> 
> This patch then digresses into style cleanups. While nice, please do that in a separate patch.

[Vlad]

Created attachment 66578 [details]
Review comments changes

Changed 
#if USE(JSC)
...
#elif USE(V8)
...
#endif
Enabled garbageCollectorCollect() v8 implementation.

[Andreas Kling]

Comment on attachment 66578 [details]
Review comments changes

> +        Comment out references to JSC.

This isn't a very accurate summary of the changes below.

Patch contents are looking pretty good though like I've said before I'd personally prefer it without all the unrelated style fixups.
FIXME's are acceptable at this stage IMO, but for longer-term things there should be bugs opened and referenced in the code.

r- for lack of relevant ChangeLog.

[Kent Hansen]

Comment on attachment 66578 [details]
Review comments changes

>  void DumpRenderTreeSupportQt::garbageCollectorCollect()
>  {
> +#if USE(JSC)
>      gcController().garbageCollectNow();
> +#elif USE(V8)
> +    v8::Local<v8::Context> context = v8::Context::GetCurrent();
> +    Frame* frame = V8Proxy::retrieveFrame(context);
> +    V8Proxy* proxy = V8Proxy::retrieve(frame);
> +    if (proxy)
> +        proxy->evaluate(ScriptSourceCode("if (window.gc) void(gc());"), 0);
> +#endif
>  }

You can call v8::LowMemoryNotification() instead (it calls an internal CollectAllGarbage() function).

And please please get rid of all the style changes; you can disregard anything the style checker complains about for lines that _you_ didn't touch.

[Andreas Kling]

Committed r67304: <http://trac.webkit.org/changeset/67304>

[WebKit Review Bot]

http://trac.webkit.org/changeset/67304 might have broken Leopard Intel Debug (Tests)