Bug 143413 - [WK2] API::Array::copy() should move the resulting Vector<> of copies into the Array::create() call
Summary: [WK2] API::Array::copy() should move the resulting Vector<> of copies into th...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-04 22:38 PDT by Zan Dobersek
Modified: 2015-04-06 02:56 PDT (History)
0 users

See Also:

Attachments
Patch (1.33 KB, patch)
2015-04-04 22:40 PDT, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Patch (2.90 KB, patch)
2015-04-05 00:27 PDT, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Patch (2.86 KB, patch)
2015-04-05 00:31 PDT, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Patch for landing (3.05 KB, patch)
2015-04-06 02:03 PDT, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2015-04-04 22:38:11 PDT 
[WK2] API::Array::copy() should move the resulting Vector<> of copies into the Array::create() call
Comment 1 Zan Dobersek 2015-04-04 22:40:59 PDT 
Created attachment 250149 [details]
Patch
Comment 2 Zan Dobersek 2015-04-05 00:27:34 PDT 
Created attachment 250151 [details]
Patch
Comment 3 Zan Dobersek 2015-04-05 00:31:21 PDT 
Created attachment 250152 [details]
Patch
Comment 4 Darin Adler 2015-04-05 13:08:41 PDT 
Comment on attachment 250152 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=250152&action=review

> Source/WebKit2/Shared/API/APIArray.cpp:81
> +Array::Array(Vector<RefPtr<Object>>&& elements)

Seems like this function should be inlined.

> Source/WebKit2/Shared/API/APIArray.h:55
> +    static PassRefPtr<Array> create(Vector<RefPtr<Object>>&& elements);

I don’t think the name “elements” here adds anything.

> Source/WebKit2/Shared/API/APIArray.h:88
> +    explicit Array(Vector<RefPtr<Object>>&& elements);

I don’t think the name “elements” here adds anything.
Comment 5 Darin Adler 2015-04-05 13:09:13 PDT 
Comment on attachment 250152 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=250152&action=review

> Source/WebKit2/Shared/API/APIArray.h:54
>      static PassRefPtr<Array> create();

The return value of these functions should be changed to Ref<Array>.
Comment 6 Zan Dobersek 2015-04-06 01:57:52 PDT 
Comment on attachment 250152 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=250152&action=review

>> Source/WebKit2/Shared/API/APIArray.h:54
>>      static PassRefPtr<Array> create();
> 
> The return value of these functions should be changed to Ref<Array>.

That would make sense, but it's a bigger task since a lot of callers already operate on a PassRefPtr<> or are leaking or accessing the stored object through a pointer. Something for a separate bug.
Comment 7 Zan Dobersek 2015-04-06 02:03:59 PDT 
Created attachment 250198 [details]
Patch for landing
Comment 8 Zan Dobersek 2015-04-06 02:55:59 PDT 
Comment on attachment 250198 [details]
Patch for landing

Clearing flags on attachment: 250198

Committed r182388: <http://trac.webkit.org/changeset/182388>
Comment 9 Zan Dobersek 2015-04-06 02:56:07 PDT 
All reviewed patches have been landed.  Closing bug.