Bug 57963

Summary:

Make the style of createFunctionOnlyCallback in V8 consistent with the JavaScriptCore version.

Product:

WebKit

Reporter:

Leandro Graciá Gil <leandrogracia>

Component:

WebCore Misc.

Assignee:

Leandro Graciá Gil <leandrogracia>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, steveblock

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

57760

Bug Blocks:

56586

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (4.67 KB, patch) 2011-04-06 11:59 PDT, Leandro Graciá Gil	no flags	Details Formatted Diff Diff
Patch (5.36 KB, patch) 2011-04-06 12:51 PDT, Leandro Graciá Gil	no flags	Details Formatted Diff Diff
Patch (4.58 KB, patch) 2011-04-06 13:58 PDT, Leandro Graciá Gil	no flags	Details Formatted Diff Diff
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.