Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(188)

Issue 362123002: Gamepad: don't notify about connected pads twice (Closed)

Created:
6 years, 5 months ago by kbalazs
Modified:
6 years, 5 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Gamepad: don't notify about connected pads twice Currently the first time we observe user gesture we call GamepadConsumer::OnGamepadConnected once because of the gesture and once because of the state change of the pad. This CL fixes that by reordering the gesture check and the state tests in GamepadProvider the state change will be ignored by GamepadService because did_observe_user_gesture is still false for the consumer. Also it seems like I forget to set gesture_callback_pending_ to true when appropriate in my former CL's, fixed it. Added unit test for connections and made some refactoring related to unittests. BUG=344556 R=bajones@chromium.org,scottmg@chromium.org TBR=dmichael@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281098

Patch Set 1 #

Patch Set 2 : lol, gesture_callback_pending_ was still at the wrong place #

Patch Set 3 : c++11 fix #

Patch Set 4 : more build fix (kNumberOfGamepads) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+181 lines, -31 lines) Patch
M content/browser/gamepad/gamepad_provider.cc View 5 chunks +12 lines, -5 lines 0 comments Download
M content/browser/gamepad/gamepad_provider_unittest.cc View 1 chunk +2 lines, -12 lines 0 comments Download
M content/browser/gamepad/gamepad_service.h View 2 chunks +3 lines, -0 lines 0 comments Download
M content/browser/gamepad/gamepad_service.cc View 1 2 chunks +15 lines, -2 lines 0 comments Download
A content/browser/gamepad/gamepad_service_unittest.cc View 1 2 3 1 chunk +132 lines, -0 lines 0 comments Download
M content/browser/gamepad/gamepad_test_helpers.h View 1 chunk +5 lines, -0 lines 0 comments Download
M content/browser/gamepad/gamepad_test_helpers.cc View 1 chunk +9 lines, -0 lines 0 comments Download
M content/browser/renderer_host/pepper/pepper_gamepad_host_unittest.cc View 1 chunk +2 lines, -12 lines 0 comments Download
M content/content_tests.gypi View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 18 (0 generated)
kbalazs
6 years, 5 months ago (2014-07-01 20:09:16 UTC) #1
bajones
On 2014/07/01 20:09:16, kbalazs wrote: content/browser/gamepad LGTM
6 years, 5 months ago (2014-07-01 20:59:26 UTC) #2
dmichael (off chromium)
pepper lgtm
6 years, 5 months ago (2014-07-01 21:02:07 UTC) #3
kbalazs
The CQ bit was checked by b.kelemen@samsung.com
6 years, 5 months ago (2014-07-01 21:17:30 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/362123002/20001
6 years, 5 months ago (2014-07-01 21:18:08 UTC) #5
kbalazs
The CQ bit was unchecked by b.kelemen@samsung.com
6 years, 5 months ago (2014-07-01 21:31:28 UTC) #6
kbalazs
The CQ bit was checked by b.kelemen@samsung.com
6 years, 5 months ago (2014-07-01 21:32:53 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/362123002/20001
6 years, 5 months ago (2014-07-01 21:34:24 UTC) #8
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_clang_dbg on tryserver.chromium ...
6 years, 5 months ago (2014-07-01 22:51:06 UTC) #9
kbalazs
The CQ bit was checked by b.kelemen@samsung.com
6 years, 5 months ago (2014-07-01 22:55:55 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/362123002/40001
6 years, 5 months ago (2014-07-01 22:56:01 UTC) #11
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: ios_dbg_simulator on tryserver.chromium ...
6 years, 5 months ago (2014-07-02 01:18:35 UTC) #12
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 5 months ago (2014-07-02 01:31:15 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/builds/47785)
6 years, 5 months ago (2014-07-02 01:31:16 UTC) #14
kbalazs
The CQ bit was checked by b.kelemen@samsung.com
6 years, 5 months ago (2014-07-02 17:31:19 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/362123002/60001
6 years, 5 months ago (2014-07-02 17:32:11 UTC) #16
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_rel on tryserver.chromium ...
6 years, 5 months ago (2014-07-02 19:55:02 UTC) #17
commit-bot: I haz the power
6 years, 5 months ago (2014-07-02 21:58:16 UTC) #18
Message was sent while issue was closed.
Change committed as 281098

Powered by Google App Engine
This is Rietveld 408576698