Mark template class RefCountedObject as final.

Mark template class RefCountedObject as final.

RefCountedObject should only ever be used as a leaf class in the
inheritance tree.

With final, the compiler can identify places where this template is
used incorrectly; this cl tries to fix them all.

BUG=webrtc:8270

[nisse-webrtc, 2017-09-01 12:51:23]

nisse@webrtc.org changed reviewers:
+ kwiberg@webrtc.org, tommi@webrtc.org

[nisse-webrtc, 2017-09-01 12:51:24]

When working on cl https://codereview.webrtc.org/3010883002/, we noticed that
RefCountedObject is used incorrectly in a couple of places. With incorrect use,
we get several layers of the RefCountedObject template around the real class.

As far as I understand, each layer of wrapping gets its own ref_count_ member
variable, although only the outermost is used. With no warnings from the
compiler. The change in the other cl had the side effect of turning invalid uses
into compilation errors, "direct base
'rtc::RefCountedBase' is inaccessible due to ambiguity" 

Hence, let's try to fix those errors first, in this cl.

[tommi, 2017-09-01 14:57:28]

On 2017/09/01 12:51:24, nisse-webrtc wrote:
> When working on cl https://codereview.webrtc.org/3010883002/, we noticed that
> RefCountedObject is used incorrectly in a couple of places. With incorrect
use,
> we get several layers of the RefCountedObject template around the real class.
> 
> As far as I understand, each layer of wrapping gets its own ref_count_ member
> variable, although only the outermost is used. With no warnings from the
> compiler. The change in the other cl had the side effect of turning invalid
uses
> into compilation errors, "direct base
> 'rtc::RefCountedBase' is inaccessible due to ambiguity" 
> 
> Hence, let's try to fix those errors first, in this cl.

Thumbs up on fixing this.  Are the bot errors unrelated?

[kwiberg-webrtc, 2017-09-03 01:47:55]

lgtm!

[nisse-webrtc, 2017-09-04 07:31:37]

On 2017/09/01 14:57:28, tommi wrote:

> Thumbs up on fixing this.  Are the bot errors unrelated?

I'm investigating. I think at least the chromium error indicates misuse in the
chromium code, which has to be fixed before landing this cl.

[nisse-webrtc, 2017-09-18 08:28:57]

Downstream projects hopefully fixed by now.

https://codereview.webrtc.org/3007973002/diff/20001/pc/rtpsenderreceiver_unit...
File pc/rtpsenderreceiver_unittest.cc (right):

https://codereview.webrtc.org/3007973002/diff/20001/pc/rtpsenderreceiver_unit...
pc/rtpsenderreceiver_unittest.cc:137:
std::vector<std::string>({local_stream_->label()}),
The compiler template magic failed without explicit construction of this vector,
it said it could infer the argument for the argument 
|{local_stream->label()}|.

Is there any nice way to fix that, and make RefCountedObject forward initializer
lists to the constructor of the underlying type?

[kwiberg-webrtc, 2017-09-18 09:13:51]

https://codereview.webrtc.org/3007973002/diff/20001/pc/rtpsenderreceiver_unit...
File pc/rtpsenderreceiver_unittest.cc (right):

https://codereview.webrtc.org/3007973002/diff/20001/pc/rtpsenderreceiver_unit...
pc/rtpsenderreceiver_unittest.cc:137:
std::vector<std::string>({local_stream_->label()}),
On 2017/09/18 08:28:57, nisse-webrtc wrote:
> The compiler template magic failed without explicit construction of this
vector,
> it said it could infer the argument for the argument 
> |{local_stream->label()}|.
> 
> Is there any nice way to fix that, and make RefCountedObject forward
initializer
> lists to the constructor of the underlying type?

Not that I know of. See e.g.
https://stackoverflow.com/questions/28370970/forwarding-initializer-list-expr...

[nisse-webrtc, 2017-09-18 09:20:05]

https://codereview.webrtc.org/3007973002/diff/20001/pc/rtpsenderreceiver_unit...
File pc/rtpsenderreceiver_unittest.cc (right):

https://codereview.webrtc.org/3007973002/diff/20001/pc/rtpsenderreceiver_unit...
pc/rtpsenderreceiver_unittest.cc:137:
std::vector<std::string>({local_stream_->label()}),
On 2017/09/18 09:13:50, kwiberg-webrtc wrote:
> On 2017/09/18 08:28:57, nisse-webrtc wrote:
> > The compiler template magic failed without explicit construction of this
> vector,
> > it said it could infer the argument for the argument 
> > |{local_stream->label()}|.
> > 
> > Is there any nice way to fix that, and make RefCountedObject forward
> initializer
> > lists to the constructor of the underlying type?
> 
> Not that I know of. See e.g.
>
https://stackoverflow.com/questions/28370970/forwarding-initializer-list-expr...

Hmm. From there, it looks like it might work to write
std::initializer_list(local_stream_->label()) instead of std::vector.... But
that wouldn't improve readability, I'm afraid, even f it works.

[nisse-webrtc, 2017-09-18 13:50:46]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-09-18 13:50:56]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-09-18 13:55:46]

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

[commit-bot: I haz the power, 2017-09-18 13:55:47]

Dry run: Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/21818)

[nisse-webrtc, 2017-09-22 07:38:59]

Description was changed from

==========
Mark template class RefCountedObject as final.

RefCountedObject should only ever be used as a leaf class in the
inheritance tree.

With final, the compiler can identify places where this template is
used incorrectly; this cl tries to fix them all.

BUG=
==========

to

==========
Mark template class RefCountedObject as final.

RefCountedObject should only ever be used as a leaf class in the
inheritance tree.

With final, the compiler can identify places where this template is
used incorrectly; this cl tries to fix them all.

BUG=webrtc:8270
==========

[nisse-webrtc, 2017-09-22 07:41:00]

Another set of downstream fixes are now landed. I'd like to get this landed, as
soon as the linux_internal bot is green.

[kwiberg-webrtc, 2017-09-22 08:37:18]

On 2017/09/22 07:41:00, nisse-webrtc wrote:
> Another set of downstream fixes are now landed. I'd like to get this landed,
as
> soon as the linux_internal bot is green.

stgm

[kwiberg-webrtc, 2017-09-22 08:38:28]

On 2017/09/22 08:37:18, kwiberg-webrtc wrote:
> On 2017/09/22 07:41:00, nisse-webrtc wrote:
> > Another set of downstream fixes are now landed. I'd like to get this landed,
> as
> > soon as the linux_internal bot is green.
> 
> stgm

Sorry, that should've been "gtmis" (good to me, it sounds). Accidental Yoda
speech is hard to get right...

[nisse-webrtc, 2017-09-22 09:28:04]

On 2017/09/22 08:38:28, kwiberg-webrtc wrote:
> On 2017/09/22 08:37:18, kwiberg-webrtc wrote:
> > On 2017/09/22 07:41:00, nisse-webrtc wrote:
> > > Another set of downstream fixes are now landed. I'd like to get this
landed,
> > as
> > > soon as the linux_internal bot is green.
> > 
> > stgm
> 
> Sorry, that should've been "gtmis" (good to me, it sounds). Accidental Yoda
> speech is hard to get right...

Discussed offline. Current plan is to land RefCountedBase first, so that
application code with concrete refcounted classes can be fixed to use that
instead of RefCountedObject.