Even more virtual method deinlining.

Even more virtual method deinlining.

BUG=none
TEST=compiles

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=69041

[Elliot Glaysher, 2010-12-09 20:15:15]

Boring patch seeks rubberstamp.

[Peter Kasting, 2010-12-09 20:46:45]

I mentioned to Elliot that I'm not comfortable with some of the deinlining
that's happening here.

[Elliot Glaysher, 2010-12-09 22:54:19]

On 2010/12/09 20:46:45, Peter Kasting wrote:
> I mentioned to Elliot that I'm not comfortable with some of the deinlining
> that's happening here.

1) This patch shaves 1032k off our .a files*, making it as worthwhile thing to
do. This is only slightly smaller than the average impact of my virtual
destructor patches. r68746, my previous patch that I committed this morning, was
548k off, which is still better than my least impactful destructor patch.

I remember you mentioning infobar_delegate.{h,cc} by name in our conversation;
the size difference from trunk with just the changes to those two files are
296k.

* Locally built Linux Debug.

2) I would restate my argument that such method definitions (not declarations)
are part of the implementation and not the interface and don't belong in the
header.

[I'm not clear on exactly what you're arguing against, so I'm providing two
additional points, which may not address your arguments.]
4) (In case you're arguing about virtual accessors) You mentioned that it's
easier to not have to search through the implementation file, but I'd argue that
you can more reasonably rely on the method being defined in the implementation
file if we mandate this. For example, look at
chrome/browser/autofill/credit_card.h. I don't think I would have caught the
method definition among all those declarations.

5) (In case you're arguing against the removal of the default implementations,
as in infobar_delegate.h) According to Ben, we're not supposed to be supplying
default implementations anyway, but do because we're lazy. It's even more
important (per the results above)  to deinline in this case because the methods
are emitted in each .o whenever objects of their subclasses are instantiated.

For example, other than ~InfoBarDelegate, the heaviest symbols from this class
is a three way tie between ::AsCrashedExtensionInfoBarDelegate(),
::AsExtensionInfoBarDelegate(), and ::AsTranslateInfoBarDelegate() which are
emitted in 20 different object files. The other methods trail off, with
::AsAlertInfoBarDelegate() being emitted in 6 files and ::GetIcon() being
emitted only twice.

[Peter Kasting, 2010-12-09 23:13:39]

I withdraw my objections.  I disagree in passing that "we're not supposed to
supply default implementations anyway" when it's stated in such a generalized
fashion.  Default implementations frequently make sense and sometimes don't.

[Elliot Glaysher, 2010-12-13 19:28:21]

SUPER PING PPL.

[James Hawkins, 2010-12-13 19:35:10]

LGTM