RESOLVED FIXED 93644
[BlackBerry] Introduce a JavaScript Variant object
https://bugs.webkit.org/show_bug.cgi?id=93644
Summary [BlackBerry] Introduce a JavaScript Variant object
Benjamin Meyer
Reported 2012-08-09 13:12:22 PDT
The JavaScriptVariant can be used for functions that return results from JavaScript such as executeJavaScript or functions that take arguments to pass a variable to JavaScript and they want to make sure the type is correct.
Attachments
patch (19.03 KB, patch)
2012-08-09 13:19 PDT, Benjamin Meyer
no flags
patch (19.01 KB, patch)
2012-08-09 13:34 PDT, Benjamin Meyer
manyoso: review-
patch (19.13 KB, patch)
2012-08-09 14:32 PDT, Benjamin Meyer
tonikitoo: review+
patch (19.16 KB, patch)
2012-08-10 11:35 PDT, Benjamin Meyer
no flags
Benjamin Meyer
Comment 1 2012-08-09 13:19:23 PDT
WebKit Review Bot
Comment 2 2012-08-09 13:22:08 PDT
Attachment 157524 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/ChangeLog', u'Source/WebKit/..." exit_code: 1 Source/WebKit/blackberry/Api/WebPage.h:24: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Benjamin Meyer
Comment 3 2012-08-09 13:34:47 PDT
Created attachment 157529 [details] patch Fixed style error
Adam Treat
Comment 4 2012-08-09 14:04:01 PDT
Comment on attachment 157529 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=157529&action=review I think this looks good overall. Please fix the issues and then I'll look one last time. > Source/WebKit/blackberry/Api/JavaScriptVariant.cpp:87 > + // Output warning? ASSERT_NOT_REACHED() I would say. > Source/WebKit/blackberry/Api/JavaScriptVariant.h:29 > + Remove the extra new line here.
Benjamin Meyer
Comment 5 2012-08-09 14:32:31 PDT
Created attachment 157551 [details] patch Corrections from Adam's review
Antonio Gomes
Comment 6 2012-08-10 10:56:59 PDT
Comment on attachment 157551 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=157551&action=review > Source/WebKit/blackberry/Api/JavaScriptVariant.h:31 > +class BLACKBERRY_EXPORT JavaScriptVariant { > + > +public: nit: extra blank line. > Source/WebKit/blackberry/ChangeLog:16 > + Reviewed by Joe Mason Should be "Internally reviewed by Joe"
Benjamin Meyer
Comment 7 2012-08-10 11:35:53 PDT
Created attachment 157771 [details] patch Changes from Antonio Gomes comments, (remove blank link and change review by link in the changelogs and commit message)
WebKit Review Bot
Comment 8 2012-08-10 13:40:28 PDT
Comment on attachment 157771 [details] patch Clearing flags on attachment: 157771 Committed r125324: <http://trac.webkit.org/changeset/125324>
WebKit Review Bot
Comment 9 2012-08-10 13:40:34 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.