Bug 90694 – [BlackBerry] Add an API to explicitly call a JavaScript function with args.

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Benjamin Meyer]

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.

[WebKit Review Bot]

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.

[Yong Li]

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?

[WebKit Review Bot]

Comment on attachment 151098 [details]
patch

Attachment 151098 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13153358

[Benjamin Meyer]

Created attachment 151105 [details]
updated with corrections from the review

[WebKit Review Bot]

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.

[Yong Li]

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?

[Benjamin Meyer]

Created attachment 151108 [details]
updated with corrections from the review

[WebKit Review Bot]

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

[Benjamin Meyer]

Created attachment 151111 [details]
Add reviewed by line to changelog and commit msg

[WebKit Review Bot]

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>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.