Make the Vector copy constructor for mismatched inline sizes explicit.

Make the Vector copy constructor for mismatched inline sizes explicit.

This exposes a bunch of existing places where an implicit conversion was
occurring. These have been updated to explicit conversions.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=157657

[Chris Evans, 2013-09-11 07:58:40]

Uhh, the CSSParser-in.cpp one is kind of sad. The source type is
Vector<CSSProperty, 256> so we're obviously expecting a ton of these and a copy
may be visible in benchmarks?

So let me know if you want me to attempt a repair in this CL.

Also, your C++ is better than mine, so you might want to confirm my belief that
the C++11 rvalue-based move constructor and move assignment don't save us in
these cases.

[jsbell, 2013-09-11 15:44:02]

https://codereview.chromium.org/23903041/diff/1/Source/modules/indexeddb/IDBO...
File Source/modules/indexeddb/IDBObjectStore.cpp (right):

https://codereview.chromium.org/23903041/diff/1/Source/modules/indexeddb/IDBO...
Source/modules/indexeddb/IDBObjectStore.cpp:340:
Vector<IDBObjectStore::IndexKeys, 1> indexKeysList;
Given that this just results in copies for passing in as const references
(below), I don't see any benefit in constructing with an inline capacity here.
We should switch this to just Vector<IDBObjectStore::IndexKeys> and revert the
two changes below.

[abarth-chromium, 2013-09-11 18:16:41]

LGTM

This CL is great.  It's exposing a bunch of badness.  Let's land this CL and fix
the badness in a followup CL.

https://codereview.chromium.org/23903041/diff/1/Source/core/css/CSSValueList.cpp
File Source/core/css/CSSValueList.cpp (right):

https://codereview.chromium.org/23903041/diff/1/Source/core/css/CSSValueList....
Source/core/css/CSSValueList.cpp:130: return m_valueListSeparator ==
other.m_valueListSeparator &&
compareCSSValueVector<CSSValue>(Vector<RefPtr<CSSValue> >(m_values),
Vector<RefPtr<CSSValue> >(other.m_values));
This has got to be wrong.  Can you add a FIXME comment about avoiding this copy?

https://codereview.chromium.org/23903041/diff/1/Source/core/html/track/WebVTT...
File Source/core/html/track/WebVTTTokenizer.cpp (right):

https://codereview.chromium.org/23903041/diff/1/Source/core/html/track/WebVTT...
Source/core/html/track/WebVTTTokenizer.cpp:87: ||
vectorEqualsString<UChar>(Vector<UChar, 32>(m_token->characters()),
emptyString()))
This has got to be wrong too.  I don't think we need to fix these all in this
CL, but we should flag them with a FIXME comment that we can grep for and fix in
a followup CL.

[Chris Evans, 2013-09-11 20:13:56]

On 2013/09/11 18:16:41, abarth wrote:
> LGTM
> 
> This CL is great.  It's exposing a bunch of badness.  Let's land this CL and
fix
> the badness in a followup CL.

Ok. I'll do the change suggested by @jsbell, add the FIXMEs. And also if you
don't object, I'll quickly change the CSSParser-in.cpp called function to just
be a Vector<T, 256> because the two places I changed represent the only call
sites and I think CSS property parsing might be hot.

> 
>
https://codereview.chromium.org/23903041/diff/1/Source/core/css/CSSValueList.cpp
> File Source/core/css/CSSValueList.cpp (right):
> 
>
https://codereview.chromium.org/23903041/diff/1/Source/core/css/CSSValueList....
> Source/core/css/CSSValueList.cpp:130: return m_valueListSeparator ==
> other.m_valueListSeparator &&
> compareCSSValueVector<CSSValue>(Vector<RefPtr<CSSValue> >(m_values),
> Vector<RefPtr<CSSValue> >(other.m_values));
> This has got to be wrong.  Can you add a FIXME comment about avoiding this
copy?
> 
>
https://codereview.chromium.org/23903041/diff/1/Source/core/html/track/WebVTT...
> File Source/core/html/track/WebVTTTokenizer.cpp (right):
> 
>
https://codereview.chromium.org/23903041/diff/1/Source/core/html/track/WebVTT...
> Source/core/html/track/WebVTTTokenizer.cpp:87: ||
> vectorEqualsString<UChar>(Vector<UChar, 32>(m_token->characters()),
> emptyString()))
> This has got to be wrong too.  I don't think we need to fix these all in this
> CL, but we should flag them with a FIXME comment that we can grep for and fix
in
> a followup CL.

[esprehn, 2013-09-11 20:43:40]

LGTM, I don't think C++ can save us here since the types don't match so the
compiler can't know which inline capacity to use when generating the code at the
call sites.

[commit-bot: I haz the power, 2013-09-12 04:09:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cevans@chromium.org/23903041/10001

[commit-bot: I haz the power, 2013-09-12 04:14:37]

Change committed as 157657

[Chris Evans, 2013-09-12 04:18:13]

On 2013/09/12 04:14:37, I haz the power (commit-bot) wrote:
> Change committed as 157657

Hmm. I was expecting the cq to re-run the *_layout debug testing because they
came up red last time. (I'm pretty sure they're red on account of flake and
already-known issues but still....?!)

Is there anyone we like to cc: when the cq has possibly misbehaved?

In terms of the CL, I'll be keeping an eye on the tree to make sure it's ok.