Bug 57963 - Make the style of createFunctionOnlyCallback in V8 consistent with the JavaScriptCore version.
Summary: Make the style of createFunctionOnlyCallback in V8 consistent with the JavaSc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Leandro Graciá Gil
URL:
Keywords:
Depends on: 57760
Blocks: 56586
  Show dependency treegraph
 
Reported: 2011-04-06 11:04 PDT by Leandro Graciá Gil
Modified: 2012-09-19 07:18 PDT (History)
2 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Leandro Graciá Gil 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.
Comment 1 Leandro Graciá Gil 2011-04-06 11:24:19 PDT
Created attachment 88471 [details]
Patch
Comment 2 Steve Block 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?
Comment 3 Leandro Graciá Gil 2011-04-06 11:59:19 PDT
Created attachment 88479 [details]
Patch
Comment 4 Leandro Graciá Gil 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.
Comment 5 Leandro Graciá Gil 2011-04-06 12:51:22 PDT
Created attachment 88491 [details]
Patch
Comment 6 Leandro Graciá Gil 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.
Comment 7 Steve Block 2011-04-06 13:46:40 PDT
Comment on attachment 88491 [details]
Patch

r=me
Comment 8 Leandro Graciá Gil 2011-04-06 13:58:19 PDT
Created attachment 88507 [details]
Patch
Comment 9 Leandro Graciá Gil 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.
Comment 10 Steve Block 2011-04-06 14:01:10 PDT
Comment on attachment 88507 [details]
Patch

r=me
Comment 11 WebKit Commit Bot 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.
Comment 12 WebKit Commit Bot 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>
Comment 13 WebKit Commit Bot 2011-04-06 16:02:00 PDT
All reviewed patches have been landed.  Closing bug.