Summary: | [BlackBerry] Add an API to explicitly call a JavaScript function with args. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Meyer <ben> | ||||||||||
Component: | WebKit BlackBerry | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | dglazkov, mifenton, tonikitoo, webkit.review.bot, yong.li.webkit | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Other | ||||||||||||
OS: | Other | ||||||||||||
Attachments: |
|
Attachment 151098 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/blackberry/Api/WebPage.cpp',..." exit_code: 1
Source/WebKit/blackberry/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5]
Source/WebKit/blackberry/Api/WebPage.cpp:863: Declaration has space between type name and * in JSStringRef *argList [whitespace/declaration] [3]
Source/WebKit/blackberry/Api/WebPage.cpp:864: Declaration has space between type name and * in JSValueRef *argListRef [whitespace/declaration] [3]
Source/WebKit/blackberry/Api/WebPage.cpp:873: Use 0 instead of NULL. [readability/null] [5]
Source/WebKit/blackberry/Api/WebPage.cpp:878: Use 0 instead of NULL. [readability/null] [5]
Source/WebKit/blackberry/Api/WebPage.cpp:892: One line control clauses should not use braces. [whitespace/braces] [4]
Source/WebKit/blackberry/Api/WebPage.cpp:933: Use 0 instead of NULL. [readability/null] [5]
Source/WebKit/blackberry/Api/WebPage.h:30: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 8 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 151098 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=151098&action=review > Source/WebKit/blackberry/Api/WebPage.cpp:951 > + size_t bufferSize = JSStringGetMaximumUTF8CStringSize(stringRef); > + char* buffer = new char[bufferSize]; > + size_t s2 = JSStringGetUTF8CString(stringRef, buffer, bufferSize); > + returnValue = WebString(buffer, s2); > + delete buffer; should be delete[]. actually we should use Vector<char>. Also, WebString is supposed to be either UTF16 or latin1. probably we should use WebString::fromUtf8()? or consider using std::string? Comment on attachment 151098 [details] patch Attachment 151098 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13153358 Created attachment 151105 [details]
updated with corrections from the review
Attachment 151105 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/blackberry/Api/WebPage.cpp',..." exit_code: 1
Source/WebKit/blackberry/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5]
Total errors found: 1 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 151105 [details] updated with corrections from the review View in context: https://bugs.webkit.org/attachment.cgi?id=151105&action=review LGTM > Source/WebKit/blackberry/Api/WebPage.cpp:897 > + JSObjectRef fn = obj; minor: I prefer long names to short ones. > Source/WebKit/blackberry/ChangeLog:3 > + > + Add an API to explicitly call a JavaScript function with args. should we also put webkit bug number here? Created attachment 151108 [details]
updated with corrections from the review
Comment on attachment 151108 [details] updated with corrections from the review Rejecting attachment 151108 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Source/WebKit/blackberry/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/13143555 Created attachment 151111 [details]
Add reviewed by line to changelog and commit msg
Comment on attachment 151111 [details] Add reviewed by line to changelog and commit msg Clearing flags on attachment: 151111 Committed r122034: <http://trac.webkit.org/changeset/122034> All reviewed patches have been landed. Closing bug. |
Created attachment 151098 [details] patch Add an API to explicitly call a JavaScript function with args. Currently the Blackberry port doesn't expose the JavaScript engine to 3rd parties so they rely upon executeJavaScript which can be slower than necessary and unsafe as eval is used. This new API provides a way to explicitly call a specific JavaScript function with a list of args preventing the case where an argument comes from a untrusted source and tries to escape the arg list to take control of the JavaScript engine. In the future if the Blackberry port introduces a formal way to interact with the JavaScript engine this function should be removed.