RESOLVED FIXED Bug 57963
Make the style of createFunctionOnlyCallback in V8 consistent with the JavaScriptCore version. 
https://bugs.webkit.org/show_bug.cgi?id=57963
Summary Make the style of createFunctionOnlyCallback in V8 consistent with the JavaSc...
Leandro Graciá Gil
Reported 2011-04-06 11:04:52 PDT
Introduced by https://bugs.webkit.org/show_bug.cgi?id=57760, apply the same style changes that were suggested to the JavaScriptCore version (https://bugs.webkit.org/show_bug.cgi?id=57770) during the review process.
Attachments
Patch (4.19 KB, patch)
2011-04-06 11:24 PDT, Leandro Graciá Gil 		no flags
DetailsFormatted DiffDiff
Patch (4.67 KB, patch)
2011-04-06 11:59 PDT, Leandro Graciá Gil 		no flags
DetailsFormatted DiffDiff
Patch (5.36 KB, patch)
2011-04-06 12:51 PDT, Leandro Graciá Gil 		no flags
DetailsFormatted DiffDiff
Patch (4.58 KB, patch)
2011-04-06 13:58 PDT, Leandro Graciá Gil 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Leandro Graciá Gil
Comment 1 2011-04-06 11:24:19 PDT
Created attachment 88471 [details] Patch
Steve Block
Comment 2 2011-04-06 11:50:30 PDT
Comment on attachment 88471 [details] Patch Can you split the 'if' in the implementation into two separate statements to match JSC? Also, is there any advantage in moving this to the cpp to avoid header bloat?
Leandro Graciá Gil
Comment 3 2011-04-06 11:59:19 PDT
Created attachment 88479 [details] Patch
Leandro Graciá Gil
Comment 4 2011-04-06 12:01:24 PDT
(In reply to comment #2) > (From update of attachment 88471 [details]) > Can you split the 'if' in the implementation into two separate statements to match JSC? Also, is there any advantage in moving this to the cpp to avoid header bloat? 'if' split. However, in this case only the v8 value is required by the function, and it is also a return value to other functions in the header. We're saving headers already with the exception throwing moved into the cpp file.
Leandro Graciá Gil
Comment 5 2011-04-06 12:51:22 PDT
Created attachment 88491 [details] Patch
Leandro Graciá Gil
Comment 6 2011-04-06 12:52:31 PDT
(In reply to comment #5) > Created an attachment (id=88491) [details] > Patch Added a small fix to the WebCore.pro file, moving CallbackFunction.cpp to its corresponding place. This was introduced by https://bugs.webkit.org/show_bug.cgi?id=57770.
Steve Block
Comment 7 2011-04-06 13:46:40 PDT
Comment on attachment 88491 [details] Patch r=me
Leandro Graciá Gil
Comment 8 2011-04-06 13:58:19 PDT
Created attachment 88507 [details] Patch
Leandro Graciá Gil
Comment 9 2011-04-06 13:59:14 PDT
(In reply to comment #8) > Created an attachment (id=88507) [details] > Patch Removed the WebCore.pro fix as it has been already provided by http://trac.webkit.org/changeset/83086.
Steve Block
Comment 10 2011-04-06 14:01:10 PDT
Comment on attachment 88507 [details] Patch r=me
WebKit Commit Bot
Comment 11 2011-04-06 15:58:46 PDT
The commit-queue encountered the following flaky tests while processing attachment 88507 [details]: security/block-test.html bug 55741 (authors: beidson@apple.com, mrowe@apple.com, and sam@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 12 2011-04-06 16:01:48 PDT
Comment on attachment 88507 [details] Patch Clearing flags on attachment: 88507 Committed r83115: <http://trac.webkit.org/changeset/83115>
WebKit Commit Bot
Comment 13 2011-04-06 16:02:00 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.