Factor out GamepadCommon base class from Gamepad and WebKitGamepad

Source/modules/gamepad/WebKitGamepadList.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef WebKitGamepadList_h
 #define WebKitGamepadList_h
 
 #include "bindings/v8/ScriptWrappable.h"
-#include "heap/Handle.h"
+#include "modules/gamepad/GamepadList.h"
 #include "modules/gamepad/WebKitGamepad.h"
-#include "public/platform/WebGamepads.h"
 #include "wtf/PassRefPtr.h"
-#include "wtf/RefCounted.h"
-#include "wtf/Vector.h"
 
 namespace WebCore {
 
-class WebKitGamepadList : public RefCountedWillBeGarbageCollectedFinalized<WebKitGamepadList>, public ScriptWrappable {
+// Inherit from concrete GamepadList class (instead of factoring out an abstract
+// GamepadListCommon class) so GamepadList is simple (unfactored) and low-cruft.
+class WebKitGamepadList : public GamepadList {
 public:
     static PassRefPtrWillBeRawPtr<WebKitGamepadList> create() { return adoptRefWillBeNoop(new WebKitGamepadList); }
-    ~WebKitGamepadList();
 
     void set(unsigned index, PassRefPtrWillBeRawPtr<WebKitGamepad>);
     WebKitGamepad* item(unsigned index);
-    unsigned length() const;
-
-    void trace(Visitor*);

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

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:21]

? 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]

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

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

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.

 
 private:
     WebKitGamepadList()
     {
         ScriptWrappable::init(this);
     }
 
     RefPtrWillBeMember<WebKitGamepad> m_items[blink::WebGamepads::itemsLengthCap];

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

Looks like this is hiding GamepadList::m_items???

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

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.)

 };
 
 } // namespace WebCore
 
 #endif // WebKitGamepadList_h