Sync Navigator.getGamepads() with the spec

Source/modules/gamepad/NavigatorGamepad.cpp

 /*
  * Copyright (C) 2011, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 76 matching lines @@
         provideTo(navigator, supplementName(), adoptPtrWillBeNoop(supplement));
     }
     return *supplement;
 }
 
 WebKitGamepadList* NavigatorGamepad::webkitGetGamepads(Navigator& navigator)
 {
     return NavigatorGamepad::from(navigator).webkitGamepads();
 }
 
-GamepadList* NavigatorGamepad::getGamepads(Navigator& navigator)
+HeapVector<Member<Gamepad>> NavigatorGamepad::getGamepads(Navigator& navigator)
 {
-    return NavigatorGamepad::from(navigator).gamepads();
+    GamepadList* gamepadList = NavigatorGamepad::from(navigator).gamepads();
+    unsigned length = gamepadList->length();
+    HeapVector<Member<Gamepad>> gamepads(length);
+    unsigned actualLength = 0;
+    for (unsigned i = 0; i < length; ++i) {
+        Gamepad* gamepad = gamepadList->item(i);
+        if (gamepad) {
+            ASSERT(gamepad->index() == i);
+            gamepads[i] = gamepad;

[kbalazs, 2014/12/17 21:53:41]

This should be gamepads[actualLength], no?

It seems to me that if there is a hole in GamepadList - which is legit - you
might lose pads when you shrink.

[philipj_slow, 2014/12/17 22:45:38]

This started out as a loop simply copying gamepads[i] = gamepadList->item(i).
The actualLength is set for any non-null item to shrink the array afterwards to
the minimum length. The modified test shows how a hole in GamepadList is
handled.

[kbalazs, 2014/12/18 00:34:39]

I see this in Vector::shrink:

TypeOperations::destruct(begin() + size, end());

So if you have a hole and than you have a non-null item, you will lose that
item.

[philipj_slow, 2014/12/18 08:29:03]

gamepad-polling-access.html tries exactly this, and the second item isn't lost.

+            actualLength = i + 1;
+        }
+    }
+    gamepads.shrink(actualLength);

[kbalazs, 2014/12/17 21:53:41]

In the common case this would allocate, deallocate and allocate again, right? We
can do it with just one allocation. Actually, with inlineCapacity we can avoid
using any heap at all.

[philipj_slow, 2014/12/17 22:45:38]

shrink() only modifies the size, it doesn't reallocate. Because
GamepadList::length() is always 4, this function will allocate 4 slots and then
shrink the size.

[kbalazs, 2014/12/18 00:34:39]

Correct, it doesn't deallocate but it destroys elements in the shrinked part of
the array.

[philipj_slow, 2014/12/18 08:29:03]

It's Member<Gamepad> being destroyed, so that seems OK.

+    return gamepads;
 }
 
 WebKitGamepadList* NavigatorGamepad::webkitGamepads()
 {
     if (!m_webkitGamepads)
         m_webkitGamepads = WebKitGamepadList::create();
     if (window()) {
         startUpdating();
         sampleGamepads<WebKitGamepad>(m_webkitGamepads.get());
     }
@@ skipping 175 matching lines @@
         if (connectedGamepadChanged || (!oldWasConnected && newIsConnected)) {
             m_pendingEvents.append(newGamepad);
         }
     }
 
     if (!m_pendingEvents.isEmpty())
         m_dispatchOneEventRunner.runAsync();
 }
 
 } // namespace blink