Clean up redundant virtual specifiers for chromecast android

Clean up redundant virtual specifiers for chromecast android

BUG=None

Committed: https://crrev.com/5bb8ae3e2c5270d6aa939056d2de24eb3b37f2ef
Cr-Commit-Position: refs/heads/master@{#316660}

[kjoswiak, 2015-02-12 19:13:03]

kjoswiak@chromium.org changed reviewers:
+ gunsch@chromium.org

[kjoswiak, 2015-02-12 19:13:04]

Also, is there any preference between virtual / override for dtors? I believe
I've seen both

[kjoswiak, 2015-02-12 19:59:38]

kjoswiak@chromium.org changed reviewers:
+ byungchul@chromium.org, lcwu@chromium.org

[gunsch, 2015-02-14 00:08:49]

On 2015/02/12 19:13:04, kjoswiak wrote:
> Also, is there any preference between virtual / override for dtors? I believe
> I've seen both

It depends if the dtor is overriding another dtor or not.

In this CL, all three classes have parent classes whose dtors are being
overridden. So all three should be changed from virtual to override.

But in general, if you write a class that might be overridden, you must declare
the dtor virtual so that the subclass's dtor gets called correctly.

[byungchul, 2015-02-14 04:40:51]

On 2015/02/14 00:08:49, gunsch wrote:
> On 2015/02/12 19:13:04, kjoswiak wrote:
> > Also, is there any preference between virtual / override for dtors? I
believe
> > I've seen both
> 
> It depends if the dtor is overriding another dtor or not.
> 
> In this CL, all three classes have parent classes whose dtors are being
> overridden. So all three should be changed from virtual to override.
> 
> But in general, if you write a class that might be overridden, you must
declare
> the dtor virtual so that the subclass's dtor gets called correctly.

You need a virtual dtor in base class when you delete objects with base class
reference. Otherwise, delete cannot find proper dtor and subclasses dtors are
not called.
But, if subclasses don't have member vars to destroy, or you delete objects with
subclass reference, you don't need a virtual class in base class.
Once base class defines dtor virtual, all subclasses' dtors are virtual
automatically.

[byungchul, 2015-02-14 04:45:48]

On 2015/02/14 04:40:51, byungchul wrote:
> On 2015/02/14 00:08:49, gunsch wrote:
> > On 2015/02/12 19:13:04, kjoswiak wrote:
> > > Also, is there any preference between virtual / override for dtors? I
> believe
> > > I've seen both
> > 
> > It depends if the dtor is overriding another dtor or not.
> > 
> > In this CL, all three classes have parent classes whose dtors are being
> > overridden. So all three should be changed from virtual to override.
> > 
> > But in general, if you write a class that might be overridden, you must
> declare
> > the dtor virtual so that the subclass's dtor gets called correctly.
> 
> You need a virtual dtor in base class when you delete objects with base class
> reference. Otherwise, delete cannot find proper dtor and subclasses dtors are
> not called.
> But, if subclasses don't have member vars to destroy, or you delete objects
with
> subclass reference, you don't need a virtual class in base class.
> Once base class defines dtor virtual, all subclasses' dtors are virtual
> automatically.

content::WebContentsDelegate has a protected virtual destructor which is a bit
non-sense. Usually, destructor of base class should be "public & virtual" or
"protected & non-virtual".

[byungchul, 2015-02-14 04:45:59]

lgtm

[kjoswiak, 2015-02-17 19:13:40]

The CQ bit was checked by kjoswiak@chromium.org

[commit-bot: I haz the power, 2015-02-17 19:14:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/917303002/20001

[commit-bot: I haz the power, 2015-02-17 19:14:43]

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

[commit-bot: I haz the power, 2015-02-17 19:14:43]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM 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.

[gunsch, 2015-02-17 19:51:31]

lgtm

[kjoswiak, 2015-02-17 21:20:51]

The CQ bit was checked by kjoswiak@chromium.org

[commit-bot: I haz the power, 2015-02-17 21:21:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/917303002/20001

[commit-bot: I haz the power, 2015-02-17 21:44:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-17 21:45:57]

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