Bug 62330 - [Qt] JSC Bridge: convert QtConnectionObject to use JSC API
Summary: [Qt] JSC Bridge: convert QtConnectionObject to use JSC API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Caio Marcelo de Oliveira Filho
URL:
Keywords:
Depends on:
Blocks: 60842
  Show dependency treegraph
 
Reported: 2011-06-08 14:52 PDT by Caio Marcelo de Oliveira Filho
Modified: 2011-06-13 06:42 PDT (History)
2 users (show)

See Also:


Attachments
Patch (13.42 KB, patch)
2011-06-08 14:53 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Patch (14.23 KB, patch)
2011-06-08 16:09 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Caio Marcelo de Oliveira Filho 2011-06-08 14:52:57 PDT
[Qt] JSC Bridge: convert QtConnectionObject to use JSC API
Comment 1 Caio Marcelo de Oliveira Filho 2011-06-08 14:53:55 PDT
Created attachment 96488 [details]
Patch
Comment 2 Caio Marcelo de Oliveira Filho 2011-06-08 14:57:15 PDT
I think this patch is in a good shape to be reviewed, I still want to remove the limitation of 10 arguments, but it is a minor change.
Comment 3 Oliver Hunt 2011-06-08 15:12:21 PDT
Comment on attachment 96488 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96488&action=review

> Source/WebCore/bridge/qt/qt_runtime.h:224
> +    JSObjectRef m_receiver;
> +    JSObjectRef m_receiverFunction;

These used to be protected, and now they aren't -- you probably want to gcProtect/Unprotect these if you want to maintain the same semantics as those that already exist
Comment 4 Caio Marcelo de Oliveira Filho 2011-06-08 16:09:20 PDT
Created attachment 96503 [details]
Patch
Comment 5 Oliver Hunt 2011-06-08 16:15:07 PDT
Comment on attachment 96503 [details]
Patch

This looks sensible to me, but i'll hold off on r+ for someone familiar with qt
Comment 6 Noam Rosenthal 2011-06-08 18:00:02 PDT
(In reply to comment #5)
> (From update of attachment 96503 [details])
> This looks sensible to me, but i'll hold off on r+ for someone familiar with qt

"dry" r=me (dry since I'm not yet a reviewer)
Comment 7 Andreas Kling 2011-06-09 13:04:23 PDT
Comment on attachment 96503 [details]
Patch

rs=me based on Noam's assessment.
Comment 8 WebKit Review Bot 2011-06-13 06:41:58 PDT
Comment on attachment 96503 [details]
Patch

Clearing flags on attachment: 96503

Committed r88635: <http://trac.webkit.org/changeset/88635>
Comment 9 WebKit Review Bot 2011-06-13 06:42:02 PDT
All reviewed patches have been landed.  Closing bug.