From 5bdb65c9f634b96fef71bfca19e026d98617c9e9 Mon Sep 17 00:00:00 2001 From: "nona@chromium.org" Date: Fri, 27 Jul 2012 06:28:50 +0000 Subject: Revise IBusEngineService. This CL contains - Make ProcessKeyEvent asynchronous and add unittest for asynchronous handling. - Add missing signal (CommitText). - Fix wrong signal name (UpdatePreedit -> UpdatePreeditText). BUG=None TEST=ran chromeos_unittests Review URL: https://chromiumcodereview.appspot.com/10835003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148725 0039d316-1c4b-4281-b951-d872f2087c98 --- chromeos/dbus/ibus/ibus_constants.h | 3 +- chromeos/dbus/ibus/ibus_engine_service.cc | 24 +++++- chromeos/dbus/ibus/ibus_engine_service.h | 8 +- chromeos/dbus/ibus/ibus_engine_service_unittest.cc | 97 +++++++++++++++++++++- chromeos/dbus/ibus/mock_ibus_engine_service.h | 2 + 5 files changed, 126 insertions(+), 8 deletions(-) (limited to 'chromeos') diff --git a/chromeos/dbus/ibus/ibus_constants.h b/chromeos/dbus/ibus/ibus_constants.h index ab11791..6121dde 100644 --- a/chromeos/dbus/ibus/ibus_constants.h +++ b/chromeos/dbus/ibus/ibus_constants.h @@ -55,12 +55,13 @@ const char kProcessKeyEventMethod[] = "ProcessKeyEvent"; const char kCandidateClickedMethod[] = "CandidateClicked"; const char kSetSurroundingTextMethod[] = "SetSurroundingText"; const char kRegisterPropertiesSignal[] = "RegisterProperties"; -const char kUpdatePreeditSignal[] = "UpdatePreedit"; +const char kUpdatePreeditSignal[] = "UpdatePreeditText"; const char kUpdateAuxiliaryTextSignal[] = "UpdateAuxiliaryText"; const char kUpdateLookupTableSignal[] = "UpdateLookupTable"; const char kUpdatePropertySignal[] = "UpdateProperty"; const char kForwardKeyEventSignal[] = "ForwardKeyEvent"; const char kRequireSurroundingTextSignal[] = "RequireSurroundingText"; +const char kCommitTextSignal[] = "CommitText"; } // namespace engine } // namespace ibus diff --git a/chromeos/dbus/ibus/ibus_engine_service.cc b/chromeos/dbus/ibus/ibus_engine_service.cc index 85cace4..dcfee83 100644 --- a/chromeos/dbus/ibus/ibus_engine_service.cc +++ b/chromeos/dbus/ibus/ibus_engine_service.cc @@ -213,6 +213,14 @@ class IBusEngineServiceImpl : public IBusEngineService { exported_object_->SendSignal(&signal); } + virtual void CommitText(const std::string& text) OVERRIDE { + dbus::Signal signal(ibus::engine::kServiceInterface, + ibus::engine::kCommitTextSignal); + dbus::MessageWriter writer(&signal); + ibus::AppendStringAsIBusText(text, &writer); + exported_object_->SendSignal(&signal); + } + private: // Handles FocusIn method call from ibus-daemon. void FocusIn(dbus::MethodCall* method_call, @@ -355,8 +363,18 @@ class IBusEngineServiceImpl : public IBusEngineService { return; } DCHECK(engine_handler_.get()); - bool consume = engine_handler_->ProcessKeyEvent(keysym, keycode, state); - dbus::Response* response = dbus::Response::FromMethodCall(method_call); + engine_handler_->ProcessKeyEvent( + keysym, keycode, state, + base::Bind(&IBusEngineServiceImpl::KeyEventDone, + weak_ptr_factory_.GetWeakPtr(), + base::Unretained( + dbus::Response::FromMethodCall(method_call)), + response_sender)); + } + + void KeyEventDone(dbus::Response* response, + const dbus::ExportedObject::ResponseSender& response_sender, + bool consume) { dbus::MessageWriter writer(response); writer.AppendBool(consume); response_sender.Run(response); @@ -464,6 +482,8 @@ class IBusEngineServiceStubImpl : public IBusEngineService { virtual void ForwardKeyEvent(uint32 keyval, uint32 keycode, uint32 state) OVERRIDE {} virtual void RequireSurroundingText() OVERRIDE {} + virtual void CommitText(const std::string& text) OVERRIDE {} + private: DISALLOW_COPY_AND_ASSIGN(IBusEngineServiceStubImpl); }; diff --git a/chromeos/dbus/ibus/ibus_engine_service.h b/chromeos/dbus/ibus/ibus_engine_service.h index bebdd66..a7a11b4 100644 --- a/chromeos/dbus/ibus/ibus_engine_service.h +++ b/chromeos/dbus/ibus/ibus_engine_service.h @@ -30,6 +30,8 @@ typedef ScopedVector IBusPropertyList; // A interface to handle the engine client method call. class CHROMEOS_EXPORT IBusEngineHandlerInterface { public: + typedef base::Callback KeyEventDoneCallback; + // Following capability mask is introduced from // http://ibus.googlecode.com/svn/docs/ibus-1.4/ibus-ibustypes.html#IBusCapabilite // TODO(nona): Move to ibus_contants and merge one in ui/base/ime/* @@ -87,11 +89,13 @@ class CHROMEOS_EXPORT IBusEngineHandlerInterface { // Called when the key event is received. The |keycode| is raw layout // independent keycode. The |keysym| is result of XLookupString function // which translate |keycode| to keyboard layout dependent symbol value. + // Actual implementation must call |callback| after key event handling. // For example: key press event for 'd' key on us layout and dvorak layout. // keyval keycode state // us layout : 0x64 0x20 0x00 // dvorak layout : 0x65 0x20 0x00 - virtual bool ProcessKeyEvent(uint32 keysym, uint32 keycode, uint32 state) = 0; + virtual void ProcessKeyEvent(uint32 keysym, uint32 keycode, uint32 state, + const KeyEventDoneCallback& callback) = 0; // Called when the candidate in lookup table is clicked. The |index| is 0 // based candidate index in lookup table. The |state| is same value as @@ -149,6 +153,8 @@ class CHROMEOS_EXPORT IBusEngineService { virtual void ForwardKeyEvent(uint32 keyval, uint32 keycode, uint32 state) = 0; // Emits RequireSurroundingText signal. virtual void RequireSurroundingText() = 0; + // Emits CommitText signal. + virtual void CommitText(const std::string& text) = 0; // Factory function, creates a new instance and returns ownership. // For normal usage, access the singleton via DBusThreadManager::Get(). diff --git a/chromeos/dbus/ibus/ibus_engine_service_unittest.cc b/chromeos/dbus/ibus/ibus_engine_service_unittest.cc index 9ae2c4b..3800087 100644 --- a/chromeos/dbus/ibus/ibus_engine_service_unittest.cc +++ b/chromeos/dbus/ibus/ibus_engine_service_unittest.cc @@ -41,8 +41,11 @@ class MockIBusEngineHandler : public IBusEngineHandlerInterface { MOCK_METHOD1(PropertyHide, void(const std::string& property_name)); MOCK_METHOD1(SetCapability, void(IBusCapability capability)); MOCK_METHOD0(Reset, void()); - MOCK_METHOD3(ProcessKeyEvent, bool(uint32 keysym, uint32 keycode, - uint32 state)); + MOCK_METHOD4(ProcessKeyEvent, void( + uint32 keysym, + uint32 keycode, + uint32 state, + const KeyEventDoneCallback& callback)); MOCK_METHOD3(CandidateClicked, void(uint32 index, IBusMouseButton button, uint32 state)); MOCK_METHOD3(SetSurroundingText, void(const std::string& text, @@ -135,6 +138,51 @@ class RegisterPropertiesExpectation { DISALLOW_COPY_AND_ASSIGN(RegisterPropertiesExpectation); }; +// Used for mocking ProcessKeyEventHandler. +class ProcessKeyEventHandler { + public: + explicit ProcessKeyEventHandler(bool expected_value) + : expected_value_(expected_value) { + } + + void ProcessKeyEvent( + uint32 keysym, + uint32 keycode, + uint32 state, + const IBusEngineHandlerInterface::KeyEventDoneCallback& callback) { + callback.Run(expected_value_); + } + + private: + bool expected_value_; + + DISALLOW_COPY_AND_ASSIGN(ProcessKeyEventHandler); +}; + +// Used for mocking asynchronous ProcessKeyEventHandler. +class DelayProcessKeyEventHandler { + public: + DelayProcessKeyEventHandler(bool expected_value, + MessageLoop* message_loop) + : expected_value_(expected_value), + message_loop_(message_loop) { + } + + void ProcessKeyEvent( + uint32 keysym, + uint32 keycode, + uint32 state, + const IBusEngineHandlerInterface::KeyEventDoneCallback& callback) { + message_loop_->PostTask(FROM_HERE, base::Bind(callback, expected_value_)); + } + + private: + bool expected_value_; + MessageLoop* message_loop_; + + DISALLOW_COPY_AND_ASSIGN(DelayProcessKeyEventHandler); +}; + // Used for UpdatePreedit signal message evaluation. class UpdatePreeditExpectation { public: @@ -705,8 +753,46 @@ TEST_F(IBusEngineServiceTest, ProcessKeyEventTest) { const uint32 kState = 0x00; const bool kResult = true; - EXPECT_CALL(*engine_handler_, ProcessKeyEvent(kKeySym, kKeyCode, kState)) - .WillOnce(Return(kResult)); + ProcessKeyEventHandler handler(kResult); + EXPECT_CALL(*engine_handler_, ProcessKeyEvent(kKeySym, kKeyCode, kState, _)) + .WillOnce(Invoke(&handler, + &ProcessKeyEventHandler::ProcessKeyEvent)); + MockResponseSender response_sender; + BoolResponseExpectation response_expectation(kSerialNo, kResult); + EXPECT_CALL(response_sender, Run(_)) + .WillOnce(Invoke(&response_expectation, + &BoolResponseExpectation::Evaluate)); + + // Create method call; + dbus::MethodCall method_call(ibus::engine::kServiceInterface, + ibus::engine::kProcessKeyEventMethod); + method_call.SetSerial(kSerialNo); + dbus::MessageWriter writer(&method_call); + writer.AppendUint32(kKeySym); + writer.AppendUint32(kKeyCode); + writer.AppendUint32(kState); + + // Call exported function. + EXPECT_NE(method_callback_map_.find(ibus::engine::kProcessKeyEventMethod), + method_callback_map_.end()); + method_callback_map_[ibus::engine::kProcessKeyEventMethod].Run( + &method_call, + base::Bind(&MockResponseSender::Run, + base::Unretained(&response_sender))); +} + +TEST_F(IBusEngineServiceTest, DelayProcessKeyEventTest) { + // Set expectations. + const uint32 kSerialNo = 1; + const uint32 kKeySym = 0x64; + const uint32 kKeyCode = 0x20; + const uint32 kState = 0x00; + const bool kResult = true; + + DelayProcessKeyEventHandler handler(kResult, &message_loop_); + EXPECT_CALL(*engine_handler_, ProcessKeyEvent(kKeySym, kKeyCode, kState, _)) + .WillOnce(Invoke(&handler, + &DelayProcessKeyEventHandler::ProcessKeyEvent)); MockResponseSender response_sender; BoolResponseExpectation response_expectation(kSerialNo, kResult); EXPECT_CALL(response_sender, Run(_)) @@ -729,6 +815,9 @@ TEST_F(IBusEngineServiceTest, ProcessKeyEventTest) { &method_call, base::Bind(&MockResponseSender::Run, base::Unretained(&response_sender))); + + // Call KeyEventDone callback. + message_loop_.RunAllPending(); } TEST_F(IBusEngineServiceTest, CandidateClickedTest) { diff --git a/chromeos/dbus/ibus/mock_ibus_engine_service.h b/chromeos/dbus/ibus/mock_ibus_engine_service.h index 2b6c5c8..f64aeba 100644 --- a/chromeos/dbus/ibus/mock_ibus_engine_service.h +++ b/chromeos/dbus/ibus/mock_ibus_engine_service.h @@ -5,6 +5,7 @@ #ifndef CHROMEOS_DBUS_IBUS_MOCK_IBUS_ENGINE_SERVICE_H_ #define CHROMEOS_DBUS_IBUS_MOCK_IBUS_ENGINE_SERVICE_H_ +#include #include "chromeos/dbus/ibus/ibus_engine_service.h" namespace chromeos { @@ -30,6 +31,7 @@ class MockIBusEngineService : public IBusEngineService { virtual void ForwardKeyEvent(uint32 keyval, uint32 keycode, uint32 state) OVERRIDE; virtual void RequireSurroundingText() OVERRIDE; + virtual void CommitText(const std::string& text) OVERRIDE; }; } // namespace chromeos -- cgit v1.1