Factor out GamepadCommon base class from Gamepad and WebKitGamepad

Factor out GamepadCommon base class from Gamepad and WebKitGamepad

WebKitGamepad is a legacy interface for Gamepad, and hence there's a lot of duplicated code.
This reduces the duplication by C++ inheritance from a base class.

(Also cleanup for consistency and brevity.)

There isn't much code duplication for WebKitGamepadList/GamepadList,
so no benefit for a base class there, but tweaks.

Notes:
* Use C++ inheritance only; IDL files are unchanged
(*implementation* is inherited, *interface* is not)

* Inherit from an abstract GamepadCommon class
(needed for ScriptWrappable::init in constructor, avoids duplicate members and virtual methods)

Followup to:
Added non-prefixed navigator.getGamepads() with updated API
https://codereview.chromium.org/183313003/

BUG=344556
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169194

[Nils Barth (inactive), 2014-03-13 06:10:42]

Here's a pure C++ proposal; how does it look?

Relevant tests pass: gamepad/, specifically:
gamepad/gamepad-api.html
gamepad/gamepad-polling-access.html

[haraken, 2014-03-13 06:29:39]

Overall, the approach looks good.

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:32: ScriptWrappable::init(this);

As discussed offline, we don't want to call ScriptWrappable::init() twice.

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepadList.h (left):

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepadList.h:27: void trace(Visitor*);

You cannot remove trace(). The following trace() is necessary:

virtual void trace(Visitor* visitor) OVERRIDE {
  visitor->trace(m_items);
  GamepadList::trace(visitor);
}

[Nils Barth (inactive), 2014-03-13 06:42:20]

Thanks!
(A question about trace().)

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:32: ScriptWrappable::init(this);
On 2014/03/13 06:29:39, haraken wrote:
> As discussed offline, we don't want to call ScriptWrappable::init() twice.

kouhei will elaborate;
sounds like simplest/best answer is to in fact have an abstract
base class, and just call ScriptWrappable::init() in the concrete
derived classes.

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepadList.h (left):

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepadList.h:27: void trace(Visitor*);
On 2014/03/13 06:29:39, haraken wrote:
> You cannot remove trace(). The following trace() is necessary:
> 
> virtual void trace(Visitor* visitor) OVERRIDE {
>   visitor->trace(m_items);
>   GamepadList::trace(visitor);
> }

? Why is this necessary?
Can't it use trace() in the base class, which is:

void GamepadList::trace(Visitor* visitor)
{
    for (unsigned index = 0; index < blink::WebGamepads::itemsLengthCap;
index++) {
        visitor->trace(m_items[index]);
    }
}

?

[kouhei (in TOK), 2014-03-13 06:47:09]

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepadList.h (left):

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepadList.h:27: void trace(Visitor*);
On 2014/03/13 06:42:21, Nils Barth wrote:
> On 2014/03/13 06:29:39, haraken wrote:
> > You cannot remove trace(). The following trace() is necessary:
> > 
> > virtual void trace(Visitor* visitor) OVERRIDE {
> >   visitor->trace(m_items);
> >   GamepadList::trace(visitor);
> > }
> 
> ? Why is this necessary?
> Can't it use trace() in the base class, which is:
> 
> void GamepadList::trace(Visitor* visitor)
> {
>     for (unsigned index = 0; index < blink::WebGamepads::itemsLengthCap;
> index++) {
>         visitor->trace(m_items[index]);
>     }
> }
> 
> ?

It's an oilpan rule to have trace on every class in the hierarchy.

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepadList.h:35: RefPtrWillBeMember<WebKitGamepad>
m_items[blink::WebGamepads::itemsLengthCap];
Looks like this is hiding GamepadList::m_items???

[Nils Barth (inactive), 2014-03-13 06:58:42]

Thanks for comments!

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepadList.h (left):

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepadList.h:27: void trace(Visitor*);
On 2014/03/13 06:47:09, kouhei wrote:
> It's an oilpan rule to have trace on every class in the hierarchy.

Thanks!
https://docs.google.com/document/d/1y7_0ni0E_kxvrah-QtnreMlzCDKN3QP4BN1Aw7eSL...

I surmise that the standard rule for calling trace on base is:
virtual void trace(Visitor* visitor) OVERRIDE { Base::trace(visitor); }

I'll check once refactored to base class.

https://codereview.chromium.org/195993007/diff/20001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepadList.h:35: RefPtrWillBeMember<WebKitGamepad>
m_items[blink::WebGamepads::itemsLengthCap];
On 2014/03/13 06:47:09, kouhei wrote:
> Looks like this is hiding GamepadList::m_items???

GamepadList::m_items is private,
so this isn't exactly shadowing it, right?
(Dunno the right word; it's a separate implementation.)

But yes, there's duplication here b/c we're not using a common base class:
GamepadList is ignorant of WebKitGamepadList (for simplicity),
and here we just override it.
(This'll go away when we move to a base class.)

[haraken, 2014-03-13 07:08:49]

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/G...
File Source/modules/gamepad/GamepadList.h (right):

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/G...
Source/modules/gamepad/GamepadList.h:58: // WebKitGamepadList items are of type
WebKitGamepad, not Gamepad

Remove this comment. Or move the comment to WebKitGamepad::items.

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:27: void trace(Visitor*) { };

This should be:

virtual void trace(Visitor* visitor) OVERRIDE {
  Gamepad::trace(visitor);
}

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:33: // Instead, factor out a base class
GamepadCommon

We want to handle this in this CL. Calling ScriptWrappable::init() twice sounds
nasty.

On the other hand, it's a shame that we have to introduce XXXCommon just for
ScriptWrappable whenever we unprefix something. Would there be any way to avoid
that? (I have no idea at the moment.)

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepadList.h (left):

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepadList.h:27: void trace(Visitor*);

As I commented, please keep the trace() method and trace m_items.

[Nils Barth (inactive), 2014-03-13 07:36:14]

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:27: void trace(Visitor*) { };
On 2014/03/13 07:08:50, haraken wrote:
> This should be:
> 
> virtual void trace(Visitor* visitor) OVERRIDE {
>   Gamepad::trace(visitor);
> }

Got it; I'll actually factor this into the leaf class.

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:33: // Instead, factor out a base class
GamepadCommon
On 2014/03/13 07:08:50, haraken wrote:
> We want to handle this in this CL. Calling ScriptWrappable::init() twice
sounds
> nasty.

Yes, I'm going to do so in this CL; just put a FIXME for clarity.

> On the other hand, it's a shame that we have to introduce XXXCommon just for
> ScriptWrappable whenever we unprefix something. Would there be any way to
avoid
> that? (I have no idea at the moment.)

I think we'll want to have a common base class in general,
since we'll be changing the interface, not just extending,
so we'll want the differences in the separate interfaces,
and it avoids having virtual methods.
(Discussed with Kouhei; we'll want that for both interfaces here anyway.)

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepadList.h (left):

https://codereview.chromium.org/195993007/diff/40001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepadList.h:27: void trace(Visitor*);
On 2014/03/13 07:08:50, haraken wrote:
> As I commented, please keep the trace() method and trace m_items.

Got it.

[Nils Barth (inactive), 2014-03-13 08:49:52]

PTAL
WebKitGamepad and Gamepad inherit from GamepadCommon,
but WebKitGamepadList/GamepadList do not.

[kouhei (in TOK), 2014-03-13 09:05:10]

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:38: class Gamepad FINAL : public GamepadCommon,
public RefCountedWillBeGarbageCollectedFinalized<Gamepad>, public
ScriptWrappable {
RefCountedWillBeGarbageCollectedFinalized must be derived first in order to get
oilpan gc to work correctly.

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:49: void trace(Visitor* visitor) {
visitor->trace(m_buttons); }
write this in cpp file.

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
File Source/modules/gamepad/GamepadList.h (right):

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
Source/modules/gamepad/GamepadList.h:49: unsigned length() const { return
blink::WebGamepads::itemsLengthCap; }
Is this correct? always return capacity?

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:16: class WebKitGamepad FINAL : public
GamepadCommon, public RefCountedWillBeGarbageCollectedFinalized<WebKitGamepad>,
public ScriptWrappable {
Ditto

[Nils Barth (inactive), 2014-03-13 09:15:14]

Thanks for review Kouhei, revised!

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:38: class Gamepad FINAL : public GamepadCommon,
public RefCountedWillBeGarbageCollectedFinalized<Gamepad>, public
ScriptWrappable {
On 2014/03/13 09:05:11, kouhei wrote:
> RefCountedWillBeGarbageCollectedFinalized must be derived first in order to
get
> oilpan gc to work correctly.

Thanks, got it, done!

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:49: void trace(Visitor* visitor) {
visitor->trace(m_buttons); }
On 2014/03/13 09:05:11, kouhei wrote:
> write this in cpp file.

ok;
is there a rule about this?
(When to inline?)

The rule of thumb I'd been following was: "inline if fits on a line".

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
File Source/modules/gamepad/GamepadList.h (right):

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
Source/modules/gamepad/GamepadList.h:49: unsigned length() const { return
blink::WebGamepads::itemsLengthCap; }
On 2014/03/13 09:05:11, kouhei wrote:
> Is this correct? always return capacity?

Yup (it's always length 4 in fact).
This is just inlined from the .cpp:
https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:16: class WebKitGamepad FINAL : public
GamepadCommon, public RefCountedWillBeGarbageCollectedFinalized<WebKitGamepad>,
public ScriptWrappable {
On 2014/03/13 09:05:11, kouhei wrote:
> Ditto

Done.

[kouhei (in TOK), 2014-03-13 09:17:10]

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:49: void trace(Visitor* visitor) {
visitor->trace(m_buttons); }
On 2014/03/13 09:15:14, Nils Barth wrote:
> On 2014/03/13 09:05:11, kouhei wrote:
> > write this in cpp file.
> 
> ok;
> is there a rule about this?
> (When to inline?)
> 
> The rule of thumb I'd been following was: "inline if fits on a line".

IIRC the oilpan rule for trace() was write in cpp file by default, but may
inline if no cpp file exist.

[Nils Barth (inactive), 2014-03-13 09:19:51]

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/195993007/diff/80001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:49: void trace(Visitor* visitor) {
visitor->trace(m_buttons); }
On 2014/03/13 09:17:10, kouhei wrote:
> IIRC the oilpan rule for trace() was write in cpp file by default, but may
> inline if no cpp file exist.

Thanks for explaining!

[haraken, 2014-03-13 09:33:50]

I'm curious why you add GamepadCommon but you don't add GamepadListCommon.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/Gamepad.h:49: void trace(Visitor*);

Add virtual.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/GamepadCommon.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/GamepadCommon.h:40: const String& id() const { return
m_id; }

Just to confirm: You don't want to add virtual to these methods, right? (If we
add virtual to methods in the Common class, it becomes possible to customize the
implementation in child classes.)

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/GamepadCommon.h:57: 

Shall we add:

virtual void trace(Visitor*) = 0; ?

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/WebKitGamepad.h:22: ~WebKitGamepad() { }

Please keep the empty destructor in the cpp file. (The same comment in other
files in this CL.) Empty destructors in header files can cause #include cycles.
That's why we have empty destructors in cpp files.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/WebKitGamepad.h:29: void trace(Visitor*) { }

Add virtual.

[zerny-chromium, 2014-03-13 09:48:00]

DBCs

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/Gamepad.h:44: ~Gamepad() { }
Nit: keep the impl in the cpp file.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/Gamepad.h:49: void trace(Visitor*);
On 2014/03/13 09:33:50, haraken wrote:
> 
> Add virtual.

Gamepad is final and GCFinalized at that type, so trace does not need to be
virtual.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/GamepadCommon.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/GamepadCommon.h:57: 
On 2014/03/13 09:33:50, haraken wrote:
> 
> Shall we add:
> 
> virtual void trace(Visitor*) = 0; ?

There is no need to trace this base so lets keep it untraced and not add the
virtual trace. (The derived class is final.)

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/GamepadList.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/GamepadList.h:45: ~GamepadList() { }
Nit: keep the impl in the cpp file.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/GamepadList.h:55: RefPtrWillBeMember<Gamepad>
m_items[blink::WebGamepads::itemsLengthCap];
Could we just use a HeapVector here. Maybe replace GamepadList by a vector
completely? Or is the list class an API element?

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/WebKitGamepad.h:29: void trace(Visitor*) { }
On 2014/03/13 09:33:50, haraken wrote:
> 
> Add virtual.

We don't actually need a virtual trace here.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/WebKitGamepadList.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/WebKitGamepadList.h:25: ~WebKitGamepadList() { }
Nit: keep the impl in cpp file

[haraken, 2014-03-13 10:22:35]

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/Gamepad.h:49: void trace(Visitor*);
On 2014/03/13 09:48:00, zerny-chromium wrote:
> On 2014/03/13 09:33:50, haraken wrote:
> > 
> > Add virtual.
> 
> Gamepad is final and GCFinalized at that type, so trace does not need to be
> virtual.

sorry, you're right.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/GamepadCommon.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/GamepadCommon.h:57: 
On 2014/03/13 09:48:00, zerny-chromium wrote:
> On 2014/03/13 09:33:50, haraken wrote:
> > 
> > Shall we add:
> > 
> > virtual void trace(Visitor*) = 0; ?
> 
> There is no need to trace this base so lets keep it untraced and not add the
> virtual trace. (The derived class is final.)

Ditto. Ignore my previous comments about virtual :)

[Nils Barth (inactive), 2014-03-13 10:31:09]

On 2014/03/13 09:33:50, haraken wrote:
> I'm curious why you add GamepadCommon but you don't add GamepadListCommon.

There isn't much code duplication for WebKitGamepadList/GamepadList, 
so no benefit for a base class there.

(We could share 'length', which is inlined anyway.)

In more detail, we could put a bit into the base class, at the cost of adding
|static_cast| everywhere, which would make Bjarne cry.

[haraken, 2014-03-13 10:41:16]

LGTM if you apply comments in #13 and #14 (except for the comments about trace()
methods).

[Nils Barth (inactive), 2014-03-13 12:51:14]

Thanks for the comments, revised!

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/Gamepad.h:44: ~Gamepad() { }
On 2014/03/13 09:48:00, zerny-chromium wrote:
> Nit: keep the impl in the cpp file.

Done.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/GamepadList.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/GamepadList.h:45: ~GamepadList() { }
On 2014/03/13 09:48:00, zerny-chromium wrote:
> Nit: keep the impl in the cpp file.

Done.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/GamepadList.h:55: RefPtrWillBeMember<Gamepad>
m_items[blink::WebGamepads::itemsLengthCap];
On 2014/03/13 09:48:00, zerny-chromium wrote:
> Could we just use a HeapVector here.
? (I'm just refactoring here; how would a HeapVector work?)
(It's currently a 4 element array, FWIW.)

> Maybe replace GamepadList by a vector completely?
> Or is the list class an API element?

GamepadList is an API element (see GamepadList.idl),
so we need it.

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/195993007/diff/100001/Source/modules/gamepad/...
Source/modules/gamepad/WebKitGamepad.h:22: ~WebKitGamepad() { }
On 2014/03/13 09:33:50, haraken wrote:
> 
> Please keep the empty destructor in the cpp file. (The same comment in other
> files in this CL.) Empty destructors in header files can cause #include
cycles.
> That's why we have empty destructors in cpp files.

Done.

[Nils Barth (inactive), 2014-03-13 12:59:06]

On 2014/03/13 10:41:16, haraken wrote:
> LGTM if you apply comments in #13 and #14 (except for the comments about
trace()
> methods).

The only actionable comment (AFAICT) was:
"define ctors/dtors in the impl, not header"
...which is b/c inline ctors/dtors:
1. Break forward declaration, notably if you have a pointer cycle in classes: A
-> B -> A
2. Slow down linking by bloating .o with useless function versions
...per:
http://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...
http://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in....

I've done this (and removed one excess include); anything else?

[haraken, 2014-03-13 14:33:14]

LGTM.

[bajones, 2014-03-13 17:26:53]

On 2014/03/13 14:33:14, haraken wrote:
> LGTM.

LGTM too, FWIW.

[Nils Barth (inactive), 2014-03-14 01:41:06]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-03-14 01:41:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/195993007/160001

[commit-bot: I haz the power, 2014-03-14 02:33:32]

Change committed as 169194