Reland "Adding unittests to AudioConferenceMixer."

Reland "Adding unittests to AudioConferenceMixer."

Previous code review, see
https://codereview.chromium.org/1257583011/

Did not pass Mac bots since vector(size_t n) needs copy ctor before C++11.

BUG=

Committed: https://crrev.com/9743d076acfc3a7ccf4527ccc5c2ecd6ab2c7733
Cr-Commit-Position: refs/heads/master@{#9851}

[minyue-webrtc, 2015-09-01 09:49:39]

Patchset #5 (id:80001) has been deleted

[minyue-webrtc, 2015-09-01 09:49:44]

Patchset #4 (id:60001) has been deleted

[minyue-webrtc, 2015-09-01 09:49:48]

Patchset #3 (id:40001) has been deleted

[minyue-webrtc, 2015-09-01 09:49:56]

Patchset #2 (id:20001) has been deleted

[minyue-webrtc, 2015-09-01 09:55:32]

Patchset #2 (id:100001) has been deleted

[minyue-webrtc, 2015-09-01 09:59:38]

minyue@webrtc.org changed reviewers:
+ andrew@webrtc.org

[minyue-webrtc, 2015-09-01 10:01:54]

Hi,

A previous reviewed CL met a problem. Please take a look at the fix.

[ajm, 2015-09-01 16:42:54]

ajm@chromium.org changed reviewers:
+ ajm@chromium.org

[ajm, 2015-09-01 16:42:54]

The change is fine, so lgtm, but I'm curious about the error. I see here:
http://en.cppreference.com/w/cpp/container/vector/vector

that prior to C++11, vector should have required a MockMixerParticipant copy
constructor. (As of C++11, the vector(size_t) constructor results in
default-inserted instances). But I can't repro the error in a toy example:
https://paste.googleplex.com/6174816130301952

Would you mind posting the error you got?

Not that you should add a copy constructor just for this test, but the style
guide is much more lenient on it now BTW:
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Copyable_Mov...

[minyue-webrtc, 2015-09-01 22:36:06]

On 2015/09/01 16:42:54, ajm wrote:
> The change is fine, so lgtm, but I'm curious about the error. I see here:
> http://en.cppreference.com/w/cpp/container/vector/vector
> 
> that prior to C++11, vector should have required a MockMixerParticipant copy
> constructor. (As of C++11, the vector(size_t) constructor results in
> default-inserted instances). But I can't repro the error in a toy example:
> https://paste.googleplex.com/6174816130301952

Good that you did the experiment.

We need to delete copy ctor, not sure if
void operator=(const Foo&);
deletes it or defines none action, (or deletes assignment)

To delete default copy ctor, you may need to add static member or something. I
do not have it on top of my head. But you may check.


> 
> Would you mind posting the error you got?
> 
> Not that you should add a copy constructor just for this test, but the style
> guide is much more lenient on it now BTW:
>
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Copyable_Mov...

[Andrew MacDonald, 2015-09-01 22:43:54]

On 2015/09/01 22:36:06, minyue-webrtc wrote:
> On 2015/09/01 16:42:54, ajm wrote:
> > The change is fine, so lgtm, but I'm curious about the error. I see here:
> > http://en.cppreference.com/w/cpp/container/vector/vector
> > 
> > that prior to C++11, vector should have required a MockMixerParticipant copy
> > constructor. (As of C++11, the vector(size_t) constructor results in
> > default-inserted instances). But I can't repro the error in a toy example:
> > https://paste.googleplex.com/6174816130301952
> 
> Good that you did the experiment.
> 
> We need to delete copy ctor, not sure if
> void operator=(const Foo&);
> deletes it or defines none action, (or deletes assignment)
> 
> To delete default copy ctor, you may need to add static member or something. I
> do not have it on top of my head. But you may check.

Deleting of implicitly generated methods is a C++11 feature. We already know the
vector(size_t) constructor will not require a Foo copy constructor in C++11, so
I want to compile with C++98. I used the trick of making the copy constructor
(and copy assignment operator) private, which is exactly what
DISALLOW_COPY_AND_ASSIGN does:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...

[minyue-webrtc, 2015-09-01 23:32:39]

On 2015/09/01 22:43:54, Andrew MacDonald wrote:
> On 2015/09/01 22:36:06, minyue-webrtc wrote:
> > On 2015/09/01 16:42:54, ajm wrote:
> > > The change is fine, so lgtm, but I'm curious about the error. I see here:
> > > http://en.cppreference.com/w/cpp/container/vector/vector
> > > 
> > > that prior to C++11, vector should have required a MockMixerParticipant
copy
> > > constructor. (As of C++11, the vector(size_t) constructor results in
> > > default-inserted instances). But I can't repro the error in a toy example:
> > > https://paste.googleplex.com/6174816130301952
> > 
> > Good that you did the experiment.
> > 
> > We need to delete copy ctor, not sure if
> > void operator=(const Foo&);
> > deletes it or defines none action, (or deletes assignment)
> > 
> > To delete default copy ctor, you may need to add static member or something.
I
> > do not have it on top of my head. But you may check.
> 
> Deleting of implicitly generated methods is a C++11 feature. We already know
the
> vector(size_t) constructor will not require a Foo copy constructor in C++11,
so
> I want to compile with C++98. I used the trick of making the copy constructor
> (and copy assignment operator) private, which is exactly what
> DISALLOW_COPY_AND_ASSIGN does:
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...

see error message here
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/9273/ste...

[ajm, 2015-09-02 05:20:54]

On 2015/09/01 23:32:39, minyue-webrtc wrote:
> see error message here
>
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/9273/ste...

Thanks. Still can't repro with my toy example, so I'll have to be satisfied with
that.

[minyue-webrtc, 2015-09-02 08:35:22]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2015-09-02 08:35:30]

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

[commit-bot: I haz the power, 2015-09-02 08:35:31]

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

[commit-bot: I haz the power, 2015-09-02 08:35:32]

No L-G-T-M from a valid reviewer yet. Only full committers are accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[minyue-webrtc, 2015-09-02 11:22:30]

On 2015/09/02 05:20:54, ajm wrote:
> On 2015/09/01 23:32:39, minyue-webrtc wrote:
> > see error message here
> >
>
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/9273/ste...
> 
> Thanks. Still can't repro with my toy example, so I'll have to be satisfied
with
> that.

I could not reproduce it either with llvm, but can easily repo within ninja, on
your snippet.

[Andrew MacDonald, 2015-09-02 17:10:12]

andrew@webrtc.org changed reviewers:
- ajm@chromium.org

[Andrew MacDonald, 2015-09-02 17:12:05]

lgtm from my webrtc account (I hope).

Minyue, there always seems to be something weird going on with your webrtc CLs:
the emails arrive from codereview.chromium.org, which is easy to miss, and
results in reviewers using their chromium.org instead of webrtc.org accounts.
You should look into it.

[minyue-webrtc, 2015-09-03 09:40:54]

On 2015/09/02 17:12:05, Andrew MacDonald wrote:
> lgtm from my webrtc account (I hope).
> 
> Minyue, there always seems to be something weird going on with your webrtc
CLs:
> the emails arrive from http://codereview.chromium.org, which is easy to miss,
and
> results in reviewers using their http://chromium.org instead of
http://webrtc.org accounts.
> You should look into it.

Now I see the problem,
http://codereview.chromium.org and http://codereview.webrtc.org are the same,
but
The added link in the email depends on the URL from where I clicked the "send"
button.

[minyue-webrtc, 2015-09-03 13:28:38]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2015-09-03 13:28:42]

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

[commit-bot: I haz the power, 2015-09-03 15:28:50]

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

[commit-bot: I haz the power, 2015-09-03 15:28:51]

Try jobs failed on following builders:
  android on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[minyue-webrtc, 2015-09-03 20:16:00]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2015-09-03 20:16:11]

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

[commit-bot: I haz the power, 2015-09-03 20:17:20]

Committed patchset #2 (id:120001)

[commit-bot: I haz the power, 2015-09-03 20:17:30]

Patchset 2 (id:??) landed as
https://crrev.com/9743d076acfc3a7ccf4527ccc5c2ecd6ab2c7733
Cr-Commit-Position: refs/heads/master@{#9851}