components: Add out-of-line copy ctors for complex classes.

components: Add out-of-line copy ctors for complex classes.

This patch adds out of line copy constructors for classes that our
clang-plugin considers heavy. This is an effort to enable copy
constructor checks by default.

BUG=436357
R=jochen@chromium.org, dcheng@chromium.org, thakis@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/b6449d51ce2b1140e763a79b2ca453f3ccedeeac
Cr-Commit-Position: refs/heads/master@{#377717}

[vmpstr, 2016-02-24 00:19:53]

Please take a look.

[Nico, 2016-02-24 01:16:58]

lgtm

[jochen (gone - plz use gerrit), 2016-02-24 08:10:49]

did you double check that those classes can actually be copied, or are there
just forgotten disallow copy&assign macros.

If we add copy ctors, shouldn't we also add operator=?

https://codereview.chromium.org/1728033002/diff/1/components/url_matcher/url_...
File components/url_matcher/url_matcher.h (right):

https://codereview.chromium.org/1728033002/diff/1/components/url_matcher/url_...
components/url_matcher/url_matcher.h:256: URLQueryElementMatcherCondition(const
URLQueryElementMatcherCondition& other);
i wonder whether this class should rather be DISALLOW_COPY_AND_ASSIGN. Is it
really safe to copy the string_pattern_ pointer?

[vmpstr, 2016-02-25 00:20:38]

https://codereview.chromium.org/1728033002/diff/1/components/url_matcher/url_...
File components/url_matcher/url_matcher.h (right):

https://codereview.chromium.org/1728033002/diff/1/components/url_matcher/url_...
components/url_matcher/url_matcher.h:256: URLQueryElementMatcherCondition(const
URLQueryElementMatcherCondition& other);
On 2016/02/24 08:10:49, jochen wrote:
> i wonder whether this class should rather be DISALLOW_COPY_AND_ASSIGN. Is it
> really safe to copy the string_pattern_ pointer?

It's being copied over here:
https://code.google.com/p/chromium/codesearch#chromium/src/components/policy/...

I think this could be a move, but unfortunately since we have an explicit dtor,
it's preventing an implicit move ctor... I already filed crbug.com/589662 for
this.

As it stands now, this patch isn't introducing new copies it's just making
things explicit where they used to be implicit. However, I do agree that this
line can be interpreted as "copies are OK to make". 

Even with a default move ctor added, there's a compilation error here:
https://code.google.com/p/chromium/codesearch#chromium/src/components/url_mat...

since that does do a copy instead of a move.

[jochen (gone - plz use gerrit), 2016-02-25 11:56:59]

lgtm

[vmpstr, 2016-02-25 18:49:06]

Description was changed from

==========
components: Add out-of-line copy ctors for complex classes.

This patch adds out of line copy constructors for classes that our
clang-plugin considers heavy. This is an effort to enable copy
constructor checks by default.

BUG=436357
R=jochen@chromium.org, dcheng@chromium.org, thakis@chromium.org
==========

to

==========
components: Add out-of-line copy ctors for complex classes.

This patch adds out of line copy constructors for classes that our
clang-plugin considers heavy. This is an effort to enable copy
constructor checks by default.

BUG=436357
R=jochen@chromium.org, dcheng@chromium.org, thakis@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[vmpstr, 2016-02-25 18:51:37]

Patchset #2 (id:20001) has been deleted

[vmpstr, 2016-02-25 18:52:55]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2016-02-25 18:57:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1728033002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1728033002/1

[commit-bot: I haz the power, 2016-02-25 19:29:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-25 19:29:09]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[vmpstr, 2016-02-25 22:51:15]

On 2016/02/25 19:29:09, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

dcheng@ could you stamp this (components/printing/common/print_messages.h)

[dcheng, 2016-02-25 22:52:13]

lgtm

[vmpstr, 2016-02-25 22:52:49]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2016-02-25 22:56:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1728033002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1728033002/1

[commit-bot: I haz the power, 2016-02-25 23:55:49]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-02-25 23:56:57]

Description was changed from

==========
components: Add out-of-line copy ctors for complex classes.

This patch adds out of line copy constructors for classes that our
clang-plugin considers heavy. This is an effort to enable copy
constructor checks by default.

BUG=436357
R=jochen@chromium.org, dcheng@chromium.org, thakis@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
components: Add out-of-line copy ctors for complex classes.

This patch adds out of line copy constructors for classes that our
clang-plugin considers heavy. This is an effort to enable copy
constructor checks by default.

BUG=436357
R=jochen@chromium.org, dcheng@chromium.org, thakis@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/b6449d51ce2b1140e763a79b2ca453f3ccedeeac
Cr-Commit-Position: refs/heads/master@{#377717}
==========

[commit-bot: I haz the power, 2016-02-25 23:56:59]

Patchset 1 (id:??) landed as
https://crrev.com/b6449d51ce2b1140e763a79b2ca453f3ccedeeac
Cr-Commit-Position: refs/heads/master@{#377717}