RESOLVED FIXED 134670
PlatformGamepad should have an inherent index
https://bugs.webkit.org/show_bug.cgi?id=134670
Summary PlatformGamepad should have an inherent index
Brady Eidson
Reported 2014-07-06 17:09:11 PDT
PlatformGamepad should have an inherent index. This will simplify a lot of things from here on out.
Attachments
Patch v1 (14.65 KB, patch)
2014-07-06 17:15 PDT, Brady Eidson
darin: review+
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (532.77 KB, application/zip)
2014-07-06 19:21 PDT, Build Bot
no flags
Patch for landing (15.18 KB, patch)
2014-07-07 18:13 PDT, Brady Eidson
no flags
Brady Eidson
Comment 1 2014-07-06 17:15:21 PDT
Created attachment 234470 [details] Patch v1
Build Bot
Comment 2 2014-07-06 19:21:20 PDT
Comment on attachment 234470 [details] Patch v1 Attachment 234470 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6598039766564864 New failing tests: media/W3C/video/src/src_reflects_attribute_not_source_elements.html
Build Bot
Comment 3 2014-07-06 19:21:22 PDT
Created attachment 234475 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Brady Eidson
Comment 4 2014-07-06 21:43:25 PDT
Ummmmm yah, that's not related.
Brady Eidson
Comment 5 2014-07-06 22:35:52 PDT
That failure is a flake that's been hitting some set of bots for awhile, definitely not related. This can be reviewed.
Darin Adler
Comment 6 2014-07-07 10:36:16 PDT
Comment on attachment 234470 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=234470&action=review > Source/WebCore/Modules/gamepad/Gamepad.h:60 > + Gamepad(const PlatformGamepad&); Should mark this explicit. > Source/WebCore/Modules/gamepad/GamepadManager.cpp:86 > unsigned size = platformGamepads.size(); > for (unsigned i = 0; i < size; ++i) { > if (platformGamepads[i]) > - navigator->gamepadConnected(i); > + navigator->gamepadConnected(*platformGamepads[i]); > } Should write this as a modern for loop: for (auto& gamepad : GamepadProvider::shared().platformGamepads()) { if (gamepad) navigator->gamepadConnected(*gamepad); } > Source/WebCore/platform/PlatformGamepad.h:48 > + PlatformGamepad(unsigned index) Should mark this explicit. > Source/WebCore/platform/mac/HIDGamepadProvider.cpp:240 > +std::unique_ptr<HIDGamepad> HIDGamepadProvider::removeGamepadForDevice(IOHIDDeviceRef device) Extra space here after the ">" character. > Source/WebCore/platform/mac/HIDGamepadProvider.cpp:249 > for (unsigned i = 0; i < m_gamepadVector.size(); ++i) { > - if (m_gamepadVector[i] == result.first.get()) { > - result.second = i; > + if (m_gamepadVector[i] == result.get()) { > m_gamepadVector[i] = nullptr; > break; > } Could write this with Vector::find: auto i = m_gamepadVector.find(result.get()); if (i != notFound) m_gamepadVector[i] = nullptr;
Brady Eidson
Comment 7 2014-07-07 18:13:01 PDT
Created attachment 234530 [details] Patch for landing
WebKit Commit Bot
Comment 8 2014-07-07 18:52:59 PDT
Comment on attachment 234530 [details] Patch for landing Clearing flags on attachment: 234530 Committed r170869: <http://trac.webkit.org/changeset/170869>
WebKit Commit Bot
Comment 9 2014-07-07 18:53:02 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.