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

5 files changed, 126 insertions, 8 deletions

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<IBusProperty> IBusPropertyList;
 // A interface to handle the engine client method call.
 class CHROMEOS_EXPORT IBusEngineHandlerInterface {
  public:
+  typedef base::Callback<void (bool consumed)> 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 <string>
 #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