summaryrefslogtreecommitdiffstats
path: root/content
diff options
context:
space:
mode:
authorb.kelemen@samsung.com <b.kelemen@samsung.com@0039d316-1c4b-4281-b951-d872f2087c98>2014-07-02 21:58:04 +0000
committerb.kelemen@samsung.com <b.kelemen@samsung.com@0039d316-1c4b-4281-b951-d872f2087c98>2014-07-02 21:58:04 +0000
commitaa16b184102b427f560d1c614f707b0e49e9afc4 (patch)
tree55b60a17cc5111956334ad1f9cf43933adde7a0d /content
parent8155b50793c5e051231042c2223f7c321619f9fc (diff)
downloadchromium_src-aa16b184102b427f560d1c614f707b0e49e9afc4.zip
chromium_src-aa16b184102b427f560d1c614f707b0e49e9afc4.tar.gz
chromium_src-aa16b184102b427f560d1c614f707b0e49e9afc4.tar.bz2
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 Review URL: https://codereview.chromium.org/362123002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@281098 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
-rw-r--r--content/browser/gamepad/gamepad_provider.cc17
-rw-r--r--content/browser/gamepad/gamepad_provider_unittest.cc14
-rw-r--r--content/browser/gamepad/gamepad_service.cc17
-rw-r--r--content/browser/gamepad/gamepad_service.h3
-rw-r--r--content/browser/gamepad/gamepad_service_unittest.cc132
-rw-r--r--content/browser/gamepad/gamepad_test_helpers.cc9
-rw-r--r--content/browser/gamepad/gamepad_test_helpers.h5
-rw-r--r--content/browser/renderer_host/pepper/pepper_gamepad_host_unittest.cc14
-rw-r--r--content/content_tests.gypi1
9 files changed, 181 insertions, 31 deletions
diff --git a/content/browser/gamepad/gamepad_provider.cc b/content/browser/gamepad/gamepad_provider.cc
index 16976d9..caa4cc8 100644
--- a/content/browser/gamepad/gamepad_provider.cc
+++ b/content/browser/gamepad/gamepad_provider.cc
@@ -175,8 +175,7 @@ bool GamepadProvider::PadState::Match(const WebGamepad& pad) const {
}
void GamepadProvider::PadState::SetPad(const WebGamepad& pad) {
- DCHECK(pad.connected);
- connected_ = true;
+ connected_ = pad.connected;
axes_length_ = pad.axesLength;
buttons_length_ = pad.buttonsLength;
memcpy(id_, pad.id, arraysize(id_));
@@ -230,8 +229,6 @@ void GamepadProvider::DoPoll() {
hwbuf->sequence.WriteEnd();
}
- CheckForUserGesture();
-
if (ever_had_user_gesture_) {
for (unsigned i = 0; i < WebGamepads::itemsLengthCap; ++i) {
WebGamepad& pad = hwbuf->buffer.items[i];
@@ -249,6 +246,8 @@ void GamepadProvider::DoPoll() {
}
}
+ CheckForUserGesture();
+
// Schedule our next interval of polling.
ScheduleDoPoll();
}
@@ -308,7 +307,9 @@ void GamepadProvider::CheckForUserGesture() {
if (user_gesture_observers_.empty() && ever_had_user_gesture_)
return;
- if (GamepadsHaveUserGesture(SharedMemoryAsHardwareBuffer()->buffer)) {
+ bool had_gesture_before = ever_had_user_gesture_;
+ const WebGamepads& pads = SharedMemoryAsHardwareBuffer()->buffer;
+ if (GamepadsHaveUserGesture(pads)) {
ever_had_user_gesture_ = true;
for (size_t i = 0; i < user_gesture_observers_.size(); i++) {
user_gesture_observers_[i].message_loop->PostTask(FROM_HERE,
@@ -316,6 +317,12 @@ void GamepadProvider::CheckForUserGesture() {
}
user_gesture_observers_.clear();
}
+ if (!had_gesture_before && ever_had_user_gesture_) {
+ // Initialize pad_states_ for the first time.
+ for (size_t i = 0; i < WebGamepads::itemsLengthCap; ++i) {
+ pad_states_.get()[i].SetPad(pads.items[i]);
+ }
+ }
}
} // namespace content
diff --git a/content/browser/gamepad/gamepad_provider_unittest.cc b/content/browser/gamepad/gamepad_provider_unittest.cc
index adfef0c..e4f6571 100644
--- a/content/browser/gamepad/gamepad_provider_unittest.cc
+++ b/content/browser/gamepad/gamepad_provider_unittest.cc
@@ -131,26 +131,16 @@ TEST_F(GamepadProviderTest, UserGesture) {
GamepadProvider* provider = CreateProvider(no_button_data);
provider->Resume();
- // Register for a user gesture and make sure the provider reads it twice
- // see below for why).
provider->RegisterForUserGesture(listener.GetClosure());
- mock_data_fetcher_->WaitForDataRead();
- mock_data_fetcher_->WaitForDataRead();
+ mock_data_fetcher_->WaitForDataReadAndCallbacksIssued();
// It should not have issued our callback.
message_loop().RunUntilIdle();
EXPECT_FALSE(listener.has_user_gesture());
// Set a button down and wait for it to be read twice.
- //
- // We wait for two reads before calling RunAllPending because the provider
- // will read the data on the background thread (setting the event) and *then*
- // will issue the callback on our thread. Waiting for it to read twice
- // ensures that it was able to issue callbacks for the first read (if it
- // issued one) before we try to check for it.
mock_data_fetcher_->SetTestData(button_down_data);
- mock_data_fetcher_->WaitForDataRead();
- mock_data_fetcher_->WaitForDataRead();
+ mock_data_fetcher_->WaitForDataReadAndCallbacksIssued();
// It should have issued our callback.
message_loop().RunUntilIdle();
diff --git a/content/browser/gamepad/gamepad_service.cc b/content/browser/gamepad/gamepad_service.cc
index a2a83b3..42b1e10 100644
--- a/content/browser/gamepad/gamepad_service.cc
+++ b/content/browser/gamepad/gamepad_service.cc
@@ -15,24 +15,36 @@
namespace content {
+namespace {
+GamepadService* g_gamepad_service = 0;
+}
+
GamepadService::GamepadService()
: num_active_consumers_(0),
gesture_callback_pending_(false) {
+ SetInstance();
}
GamepadService::GamepadService(scoped_ptr<GamepadDataFetcher> fetcher)
: provider_(new GamepadProvider(fetcher.Pass())),
num_active_consumers_(0),
gesture_callback_pending_(false) {
+ SetInstance();
thread_checker_.DetachFromThread();
}
GamepadService::~GamepadService() {
}
+void GamepadService::SetInstance() {
+ CHECK(!g_gamepad_service);
+ g_gamepad_service = this;
+}
+
GamepadService* GamepadService::GetInstance() {
- return Singleton<GamepadService,
- LeakySingletonTraits<GamepadService> >::get();
+ if (!g_gamepad_service)
+ g_gamepad_service = new GamepadService;
+ return g_gamepad_service;
}
void GamepadService::ConsumerBecameActive(GamepadConsumer* consumer) {
@@ -46,6 +58,7 @@ void GamepadService::ConsumerBecameActive(GamepadConsumer* consumer) {
insert_result.first->is_active = true;
if (!insert_result.first->did_observe_user_gesture &&
!gesture_callback_pending_) {
+ gesture_callback_pending_ = true;
provider_->RegisterForUserGesture(
base::Bind(&GamepadService::OnUserGesture,
base::Unretained(this)));
diff --git a/content/browser/gamepad/gamepad_service.h b/content/browser/gamepad/gamepad_service.h
index 6008188..8d68068 100644
--- a/content/browser/gamepad/gamepad_service.h
+++ b/content/browser/gamepad/gamepad_service.h
@@ -82,6 +82,7 @@ class CONTENT_EXPORT GamepadService {
private:
friend struct DefaultSingletonTraits<GamepadService>;
friend class GamepadServiceTestConstructor;
+ friend class GamepadServiceTest;
GamepadService();
@@ -91,6 +92,8 @@ class CONTENT_EXPORT GamepadService {
virtual ~GamepadService();
+ void SetInstance();
+
void OnUserGesture();
struct ConsumerInfo {
diff --git a/content/browser/gamepad/gamepad_service_unittest.cc b/content/browser/gamepad/gamepad_service_unittest.cc
new file mode 100644
index 0000000..ff527f5
--- /dev/null
+++ b/content/browser/gamepad/gamepad_service_unittest.cc
@@ -0,0 +1,132 @@
+// 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.
+
+#include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
+#include "content/browser/gamepad/gamepad_consumer.h"
+#include "content/browser/gamepad/gamepad_service.h"
+#include "content/browser/gamepad/gamepad_test_helpers.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+static const int kNumberOfGamepads = blink::WebGamepads::itemsLengthCap;
+}
+
+using blink::WebGamepads;
+
+class ConnectionListener : public GamepadConsumer {
+ public:
+ ConnectionListener() {
+ ClearCounters();
+ }
+
+ virtual void OnGamepadConnected(
+ unsigned index,
+ const blink::WebGamepad& gamepad) OVERRIDE {
+ connected_counter_++;
+ }
+ virtual void OnGamepadDisconnected(
+ unsigned index,
+ const blink::WebGamepad& gamepad) OVERRIDE {
+ disconnected_counter_++;
+ }
+
+ void ClearCounters() {
+ connected_counter_ = 0;
+ disconnected_counter_ = 0;
+ }
+
+ int connected_counter() const { return connected_counter_; }
+ int disconnected_counter() const { return disconnected_counter_; }
+
+ private:
+ int connected_counter_;
+ int disconnected_counter_;
+};
+
+class GamepadServiceTest : public testing::Test {
+ protected:
+ GamepadServiceTest();
+ virtual ~GamepadServiceTest() OVERRIDE;
+
+ void SetPadsConnected(bool connected);
+ void WaitForData();
+
+ int GetConnectedCounter() const {
+ return connection_listener_->connected_counter();
+ }
+ int GetDisconnectedCounter() const {
+ return connection_listener_->disconnected_counter();
+ }
+
+ virtual void SetUp() OVERRIDE;
+
+ private:
+ MockGamepadDataFetcher* fetcher_;
+ GamepadService* service_;
+ scoped_ptr<ConnectionListener> connection_listener_;
+ TestBrowserThreadBundle browser_thread_;
+ WebGamepads test_data_;
+
+ DISALLOW_COPY_AND_ASSIGN(GamepadServiceTest);
+};
+
+GamepadServiceTest::GamepadServiceTest()
+ : browser_thread_(TestBrowserThreadBundle::IO_MAINLOOP) {
+ memset(&test_data_, 0, sizeof(test_data_));
+
+ // Set it so that we have user gesture.
+ test_data_.items[0].buttonsLength = 1;
+ test_data_.items[0].buttons[0].value = 1.f;
+ test_data_.items[0].buttons[0].pressed = true;
+}
+
+GamepadServiceTest::~GamepadServiceTest() {
+ delete service_;
+}
+
+void GamepadServiceTest::SetUp() {
+ fetcher_ = new MockGamepadDataFetcher(test_data_);
+ service_ = new GamepadService(scoped_ptr<GamepadDataFetcher>(fetcher_));
+ connection_listener_.reset((new ConnectionListener));
+ service_->ConsumerBecameActive(connection_listener_.get());
+}
+
+void GamepadServiceTest::SetPadsConnected(bool connected) {
+ for (int i = 0; i < kNumberOfGamepads; ++i) {
+ test_data_.items[i].connected = connected;
+ }
+ fetcher_->SetTestData(test_data_);
+}
+
+void GamepadServiceTest::WaitForData() {
+ connection_listener_->ClearCounters();
+ fetcher_->WaitForDataReadAndCallbacksIssued();
+ base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(GamepadServiceTest, ConnectionsTest) {
+ WaitForData();
+ EXPECT_EQ(0, GetConnectedCounter());
+ EXPECT_EQ(0, GetDisconnectedCounter());
+
+ SetPadsConnected(true);
+ WaitForData();
+ EXPECT_EQ(kNumberOfGamepads, GetConnectedCounter());
+ EXPECT_EQ(0, GetDisconnectedCounter());
+
+ SetPadsConnected(false);
+ WaitForData();
+ EXPECT_EQ(0, GetConnectedCounter());
+ EXPECT_EQ(kNumberOfGamepads, GetDisconnectedCounter());
+
+ WaitForData();
+ EXPECT_EQ(0, GetConnectedCounter());
+ EXPECT_EQ(0, GetDisconnectedCounter());
+}
+
+} // namespace content
diff --git a/content/browser/gamepad/gamepad_test_helpers.cc b/content/browser/gamepad/gamepad_test_helpers.cc
index 8837c8e..fc79dcb 100644
--- a/content/browser/gamepad/gamepad_test_helpers.cc
+++ b/content/browser/gamepad/gamepad_test_helpers.cc
@@ -30,6 +30,15 @@ void MockGamepadDataFetcher::WaitForDataRead() {
return read_data_.Wait();
}
+void MockGamepadDataFetcher::WaitForDataReadAndCallbacksIssued() {
+ // The provider will read the data on the background thread (setting the
+ // event) and *then* will issue the callback on the client thread. Waiting for
+ // it to read twice is a simple way to ensure that it was able to issue
+ // callbacks for the first read (if it issued one).
+ WaitForDataRead();
+ WaitForDataRead();
+}
+
void MockGamepadDataFetcher::SetTestData(const blink::WebGamepads& new_data) {
base::AutoLock lock(lock_);
test_data_ = new_data;
diff --git a/content/browser/gamepad/gamepad_test_helpers.h b/content/browser/gamepad/gamepad_test_helpers.h
index 2aad5753..bf88258 100644
--- a/content/browser/gamepad/gamepad_test_helpers.h
+++ b/content/browser/gamepad/gamepad_test_helpers.h
@@ -33,6 +33,11 @@ class MockGamepadDataFetcher : public GamepadDataFetcher {
// fetcher on the background thread.
void WaitForDataRead();
+ // Blocks the current thread until the GamepadProvider reads from this
+ // fetcher on the background thread and issued all callbacks related to the
+ // read on the client's thread.
+ void WaitForDataReadAndCallbacksIssued();
+
// Updates the test data.
void SetTestData(const blink::WebGamepads& new_data);
diff --git a/content/browser/renderer_host/pepper/pepper_gamepad_host_unittest.cc b/content/browser/renderer_host/pepper/pepper_gamepad_host_unittest.cc
index f86c944..38bc0e2 100644
--- a/content/browser/renderer_host/pepper/pepper_gamepad_host_unittest.cc
+++ b/content/browser/renderer_host/pepper/pepper_gamepad_host_unittest.cc
@@ -140,29 +140,19 @@ TEST_F(PepperGamepadHostTest, WaitForReply) {
gamepad_host.OnResourceMessageReceived(
PpapiHostMsg_Gamepad_RequestMemory(), &context));
- // Wait for the gamepad background thread to read twice to make sure we
- // don't get a message yet (see below for why).
MockGamepadDataFetcher* fetcher = service_->data_fetcher();
- fetcher->WaitForDataRead();
- fetcher->WaitForDataRead();
+ fetcher->WaitForDataReadAndCallbacksIssued();
// It should not have sent the callback message.
service_->message_loop().RunUntilIdle();
EXPECT_EQ(0u, sink().message_count());
// Set a button down and wait for it to be read twice.
- //
- // We wait for two reads before calling RunAllPending because the provider
- // will read the data on the background thread (setting the event) and *then*
- // will issue the callback on our thread. Waiting for it to read twice
- // ensures that it was able to issue callbacks for the first read (if it
- // issued one) before we try to check for it.
blink::WebGamepads button_down_data = default_data;
button_down_data.items[0].buttons[0].value = 1.f;
button_down_data.items[0].buttons[0].pressed = true;
fetcher->SetTestData(button_down_data);
- fetcher->WaitForDataRead();
- fetcher->WaitForDataRead();
+ fetcher->WaitForDataReadAndCallbacksIssued();
// It should have sent a callback.
service_->message_loop().RunUntilIdle();
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index 5d2ea4d..17d78c3 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -471,6 +471,7 @@
'browser/frame_host/render_widget_host_view_child_frame_unittest.cc',
'browser/frame_host/render_widget_host_view_guest_unittest.cc',
'browser/gamepad/gamepad_provider_unittest.cc',
+ 'browser/gamepad/gamepad_service_unittest.cc',
'browser/gamepad/gamepad_test_helpers.cc',
'browser/gamepad/gamepad_test_helpers.h',
'browser/geolocation/geolocation_provider_unittest.cc',
generated by cgit v1.1 at 2025-04-01 06:12:07 +0000