Bug 120637 - Support Vector<Ref<T>>.
Summary: Support Vector<Ref<T>>.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-03 09:37 PDT by Andreas Kling
Modified: 2013-09-04 05:04 PDT (History)
6 users (show)

See Also:


Attachments
Will EWS blend it? (7.14 KB, patch)
2013-09-03 09:45 PDT, Andreas Kling
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (9.75 KB, patch)
2013-09-03 11:28 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2013-09-03 09:37:36 PDT
Support Vector<Ref<T>>.
Comment 1 Andreas Kling 2013-09-03 09:45:36 PDT
Created attachment 210379 [details]
Will EWS blend it?
Comment 2 EFL EWS Bot 2013-09-03 10:17:58 PDT
Comment on attachment 210379 [details]
Will EWS blend it?

Attachment 210379 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1691288
Comment 3 EFL EWS Bot 2013-09-03 10:33:26 PDT
Comment on attachment 210379 [details]
Will EWS blend it?

Attachment 210379 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1696123
Comment 4 Andreas Kling 2013-09-03 11:28:16 PDT
Created attachment 210394 [details]
Patch
Comment 5 Antti Koivisto 2013-09-03 11:50:28 PDT
Comment on attachment 210394 [details]
Patch

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

ok

> Source/WebCore/page/Page.cpp:1201
> +
> +    return views;
>  }

With our current vector implementation do we avoid doing vector copy here?

> Source/WebCore/page/Page.cpp:1210
> +    auto views = pluginViews();

It is bit non-obvious what the return type is. Would something like pluginViewsAsVector be helpful?
Comment 6 Andreas Kling 2013-09-03 12:06:52 PDT
(In reply to comment #5)
> (From update of attachment 210394 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210394&action=review
> 
> ok
> 
> > Source/WebCore/page/Page.cpp:1201
> > +
> > +    return views;
> >  }
> 
> With our current vector implementation do we avoid doing vector copy here?

Yep, according to Anders who added a move constructor to Vector.
Comment 7 WebKit Commit Bot 2013-09-03 12:29:21 PDT
Comment on attachment 210394 [details]
Patch

Clearing flags on attachment: 210394

Committed r154997: <http://trac.webkit.org/changeset/154997>
Comment 8 WebKit Commit Bot 2013-09-03 12:29:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Mikhail Pozdnyakov 2013-09-04 05:02:01 PDT
Comment on attachment 210394 [details]
Patch

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

> Source/WTF/wtf/Ref.h:36
> +    Ref(const T& object) : m_ptr(&const_cast<T&>(object)) { m_ptr->ref(); }

Having only "Ref(T& object)" ctor hado enforced that Ref class can be produced only from non-temporary object. Now Ref can be produced from temporaries, is it desired behaviour?
Comment 10 Mikhail Pozdnyakov 2013-09-04 05:04:41 PDT
Comment on attachment 210394 [details]
Patch

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

> Source/WTF/ChangeLog:12
> +        think of a good name for that, and it'd be nice if append() would "just work."

how about overloaded append() accepting non const reference?