From ae3aaca336589f540ec09c0903bb32a6eb0c66b0 Mon Sep 17 00:00:00 2001 From: "nona@chromium.org" Date: Thu, 24 Jan 2013 07:20:59 +0000 Subject: Introduce bypass logic for SetCursorLocation message. This CL is second try of https://codereview.chromium.org/12017010/ Current Chrome does not emit SetCursorLocation message to ibus-daemon because both candidate window and input context is in same process. It was achieved by injecting IBusClient into ui::InputMethodIBus, but we are now able to bypass them only in chromeos/ directory. With this refactoring, we can remove ui::internal::IBusClient, IBusChromeOSClientImpl and their injection code. This patch introduces bypass code for SetCursorLocation, but not used yet. BUG=None TEST=Manually done on lumpy. Review URL: https://chromiumcodereview.appspot.com/12039019 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@178543 0039d316-1c4b-4281-b951-d872f2087c98 --- chromeos/dbus/dbus_thread_manager.cc | 4 +- chromeos/dbus/ibus/ibus_constants.h | 14 +++++ chromeos/dbus/ibus/ibus_input_context_client.cc | 65 ++++++++++++++++++---- chromeos/dbus/ibus/ibus_input_context_client.h | 21 ++++++- .../ibus/ibus_input_context_client_unittest.cc | 58 ++++++++++--------- chromeos/dbus/ibus/ibus_panel_service.cc | 21 +++++-- chromeos/dbus/ibus/ibus_panel_service.h | 8 ++- chromeos/dbus/ibus/ibus_panel_service_unittest.cc | 11 +++- .../dbus/ibus/mock_ibus_input_context_client.cc | 17 +++++- .../dbus/ibus/mock_ibus_input_context_client.h | 8 ++- 10 files changed, 173 insertions(+), 54 deletions(-) (limited to 'chromeos') diff --git a/chromeos/dbus/dbus_thread_manager.cc b/chromeos/dbus/dbus_thread_manager.cc index 8d7a0d1..da12b79 100644 --- a/chromeos/dbus/dbus_thread_manager.cc +++ b/chromeos/dbus/dbus_thread_manager.cc @@ -185,7 +185,9 @@ class DBusThreadManagerImpl : public DBusThreadManager { ibus_engine_factory_service_.reset( IBusEngineFactoryService::Create(ibus_bus_.get(), client_type)); ibus_panel_service_.reset( - ibus::IBusPanelService::Create(client_type, ibus_bus_.get())); + ibus::IBusPanelService::Create(client_type, + ibus_bus_.get(), + ibus_input_context_client_.get())); ibus_engine_services_.clear(); } diff --git a/chromeos/dbus/ibus/ibus_constants.h b/chromeos/dbus/ibus/ibus_constants.h index f375099..1c4c027 100644 --- a/chromeos/dbus/ibus/ibus_constants.h +++ b/chromeos/dbus/ibus/ibus_constants.h @@ -124,6 +124,20 @@ const char kServiceInterface[] = "org.freedesktop.IBus.Config"; const char kSetValueMethod[] = "SetValue"; } // namespace config +// We can't use ui/gfx/rect.h in chromeos/, so we should use ibus::Rect instead. +struct Rect { + Rect() : x(0), y(0), width(0), height(0) {} + Rect(int x, int y, int width, int height) + : x(x), + y(y), + width(width), + height(height) {} + int x; + int y; + int width; + int height; +}; + } // namespace ibus } // namespace chromeos diff --git a/chromeos/dbus/ibus/ibus_input_context_client.cc b/chromeos/dbus/ibus/ibus_input_context_client.cc index 315736d..678b792 100644 --- a/chromeos/dbus/ibus/ibus_input_context_client.cc +++ b/chromeos/dbus/ibus/ibus_input_context_client.cc @@ -26,6 +26,7 @@ class IBusInputContextClientImpl : public IBusInputContextClient { public: IBusInputContextClientImpl() : proxy_(NULL), + is_xkb_layout_(true), weak_ptr_factory_(this) { } @@ -51,6 +52,18 @@ class IBusInputContextClientImpl : public IBusInputContextClient { } // IBusInputContextClient override. + virtual void SetSetCursorLocationHandler( + const SetCursorLocationHandler& set_cursor_location_handler) OVERRIDE { + DCHECK(!set_cursor_location_handler.is_null()); + set_cursor_location_handler_ = set_cursor_location_handler; + } + + // IBusInputContextClient override. + virtual void UnsetSetCursorLocationHandler() OVERRIDE { + set_cursor_location_handler_.Reset(); + } + + // IBusInputContextClient override. virtual void ResetObjectProxy() OVERRIDE { // Do not delete proxy here, proxy object is managed by dbus::Bus object. proxy_ = NULL; @@ -93,17 +106,10 @@ class IBusInputContextClientImpl : public IBusInputContextClient { } // IBusInputContextClient override. - virtual void SetCursorLocation(int32 x, int32 y, int32 width, - int32 height) OVERRIDE { - dbus::MethodCall method_call(ibus::input_context::kServiceInterface, - ibus::input_context::kSetCursorLocationMethod); - dbus::MessageWriter writer(&method_call); - writer.AppendInt32(x); - writer.AppendInt32(y); - writer.AppendInt32(width); - writer.AppendInt32(height); - CallNoResponseMethod(&method_call, - ibus::input_context::kSetCursorLocationMethod); + virtual void SetCursorLocation(const ibus::Rect& cursor_location, + const ibus::Rect& composition_head) OVERRIDE { + if (!set_cursor_location_handler_.is_null()) + set_cursor_location_handler_.Run(cursor_location, composition_head); } // IBusInputContextClient override. @@ -159,6 +165,17 @@ class IBusInputContextClientImpl : public IBusInputContextClient { CallNoResponseMethod(&method_call, ibus::input_context::kPropertyActivateMethod); } + + // IBusInputContextClient override. + virtual bool IsXKBLayout() OVERRIDE { + return is_xkb_layout_; + } + + // IBusInputContextClient override. + virtual void SetIsXKBLayout(bool is_xkb_layout) OVERRIDE { + is_xkb_layout_ = is_xkb_layout; + } + private: void CallNoResponseMethod(dbus::MethodCall* method_call, const std::string& method_name) { @@ -331,6 +348,11 @@ class IBusInputContextClientImpl : public IBusInputContextClient { // The pointer for input context handler. This can be NULL. IBusInputContextHandlerInterface* handler_; + SetCursorLocationHandler set_cursor_location_handler_; + + // True if the current input method is xkb layout. + bool is_xkb_layout_; + base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(IBusInputContextClientImpl); @@ -355,6 +377,15 @@ class IBusInputContextClientDaemonlessImpl : public IBusInputContextClient { // TODO(nona): Implement this. } + virtual void SetSetCursorLocationHandler( + const SetCursorLocationHandler& set_cursor_location_handler) OVERRIDE { + // TODO(nona): Implement this. + } + + virtual void UnsetSetCursorLocationHandler() OVERRIDE { + // TODO(nona): Implement this. + } + virtual void ResetObjectProxy() OVERRIDE { // TODO(nona): Implement this. } @@ -380,7 +411,8 @@ class IBusInputContextClientDaemonlessImpl : public IBusInputContextClient { // TODO(nona): Implement this. } - virtual void SetCursorLocation(int32 x, int32 y, int32 w, int32 h) OVERRIDE { + virtual void SetCursorLocation(const ibus::Rect& cursor_location, + const ibus::Rect& composition_head) OVERRIDE { // TODO(nona): Implement this. } @@ -405,6 +437,15 @@ class IBusInputContextClientDaemonlessImpl : public IBusInputContextClient { // TODO(nona): Implement this. } + virtual bool IsXKBLayout() OVERRIDE { + // TODO(nona): Implement this. + return true; + } + + virtual void SetIsXKBLayout(bool is_xkb_layout) OVERRIDE { + // TODO(nona): Implement this. + } + private: DISALLOW_COPY_AND_ASSIGN(IBusInputContextClientDaemonlessImpl); }; diff --git a/chromeos/dbus/ibus/ibus_input_context_client.h b/chromeos/dbus/ibus/ibus_input_context_client.h index a55064e..ca3d13a 100644 --- a/chromeos/dbus/ibus/ibus_input_context_client.h +++ b/chromeos/dbus/ibus/ibus_input_context_client.h @@ -43,7 +43,6 @@ class CHROMEOS_EXPORT IBusInputContextHandlerInterface { // Called when the engine request hiding preedit string. virtual void HidePreeditText() = 0; - }; // A class to make the actual DBus calls for IBusInputContext service. @@ -54,6 +53,9 @@ class CHROMEOS_EXPORT IBusInputContextHandlerInterface { // one input context but it is enough for ChromeOS. class CHROMEOS_EXPORT IBusInputContextClient { public: + typedef base::Callback + SetCursorLocationHandler; typedef base::Callback ProcessKeyEventCallback; typedef base::Callback ErrorCallback; @@ -68,6 +70,13 @@ class CHROMEOS_EXPORT IBusInputContextClient { virtual void SetInputContextHandler( IBusInputContextHandlerInterface* handler) = 0; + // Sets SetCursorLocation handler. + virtual void SetSetCursorLocationHandler( + const SetCursorLocationHandler& set_cursor_location_handler) = 0; + + // Unset SetCursorLocation handler. + virtual void UnsetSetCursorLocationHandler() = 0; + // Resets object proxy. If you want to use InputContext again, should call // Initialize function again. virtual void ResetObjectProxy() = 0; @@ -85,8 +94,8 @@ class CHROMEOS_EXPORT IBusInputContextClient { // Invokes Reset method call. virtual void Reset() = 0; // Invokes SetCursorLocation method call. - virtual void SetCursorLocation(int32 x, int32 y, int32 width, - int32 height) = 0; + virtual void SetCursorLocation(const ibus::Rect& cursor_location, + const ibus::Rect& composition_head) = 0; // Invokes ProcessKeyEvent method call. |callback| should not be null. virtual void ProcessKeyEvent(uint32 keyval, uint32 keycode, @@ -105,6 +114,12 @@ class CHROMEOS_EXPORT IBusInputContextClient { virtual void PropertyActivate(const std::string& key, ibus::IBusPropertyState state) = 0; + // Returns true if the current input method is XKB layout. + virtual bool IsXKBLayout() = 0; + + // Sets current input method is XKB layout or not. + virtual void SetIsXKBLayout(bool is_xkb_layout) = 0; + // Factory function, creates a new instance and returns ownership. // For normal usage, access the singleton via DBusThreadManager::Get(). static CHROMEOS_EXPORT IBusInputContextClient* Create( diff --git a/chromeos/dbus/ibus/ibus_input_context_client_unittest.cc b/chromeos/dbus/ibus/ibus_input_context_client_unittest.cc index ff05074..fe3cee0 100644 --- a/chromeos/dbus/ibus/ibus_input_context_client_unittest.cc +++ b/chromeos/dbus/ibus/ibus_input_context_client_unittest.cc @@ -36,6 +36,10 @@ const int32 kCursorHeight = 33; const uint32 kKeyval = 34; const uint32 kKeycode = 35; const uint32 kState = 36; +const int32 kCompositionX = 37; +const int32 kCompositionY = 38; +const int32 kCompositionWidth = 39; +const int32 kCompositionHeight = 40; const bool kIsKeyHandled = false; const char kSurroundingText[] = "Surrounding Text"; const uint32 kCursorPos = 2; @@ -74,7 +78,9 @@ MATCHER_P(IBusTextEq, expected_text, "The expected IBusText does not match") { class IBusInputContextClientTest : public testing::Test { public: - IBusInputContextClientTest() : response_(NULL) {} + IBusInputContextClientTest() + : response_(NULL), + on_set_cursor_location_call_count_(0) {} virtual void SetUp() OVERRIDE { // Create a mock bus. @@ -180,24 +186,9 @@ class IBusInputContextClientTest : public testing::Test { } // Handles SetCursorLocation method call. - void OnSetCursorLocation( - dbus::MethodCall* method_call, - int timeout_ms, - const dbus::ObjectProxy::ResponseCallback& callback, - const dbus::ObjectProxy::ErrorCallback& error_callback) { - EXPECT_EQ(ibus::input_context::kServiceInterface, - method_call->GetInterface()); - EXPECT_EQ(ibus::input_context::kSetCursorLocationMethod, - method_call->GetMember()); - dbus::MessageReader reader(method_call); - int32 x, y, width, height; - EXPECT_TRUE(reader.PopInt32(&x)); - EXPECT_TRUE(reader.PopInt32(&y)); - EXPECT_TRUE(reader.PopInt32(&width)); - EXPECT_TRUE(reader.PopInt32(&height)); - EXPECT_FALSE(reader.HasMoreData()); - - message_loop_.PostTask(FROM_HERE, base::Bind(callback, response_)); + void OnSetCursorLocation(const ibus::Rect& cursor_location, + const ibus::Rect& composition_head) { + ++on_set_cursor_location_call_count_; } // Handles SetCapabilities method call. @@ -324,6 +315,8 @@ class IBusInputContextClientTest : public testing::Test { MessageLoop message_loop_; // The map from signal to signal handler. std::map signal_callback_map_; + // Call count of OnSetCursorLocation. + int on_set_cursor_location_call_count_; private: // Used to implement the mock proxy. @@ -526,18 +519,23 @@ TEST_F(IBusInputContextClientTest, SetCapabilitiesTest) { } TEST_F(IBusInputContextClientTest, SetCursorLocationTest) { - // Set expectations. - EXPECT_CALL(*mock_proxy_, CallMethodWithErrorCallback(_, _, _, _)) - .WillOnce(Invoke(this, - &IBusInputContextClientTest::OnSetCursorLocation)); - // Create response. - scoped_ptr response(dbus::Response::CreateEmpty()); - response_ = response.get(); - + on_set_cursor_location_call_count_ = 0; + client_->SetSetCursorLocationHandler( + base::Bind(&IBusInputContextClientTest::OnSetCursorLocation, + base::Unretained(this))); + const ibus::Rect cursor_location(kCursorX, + kCursorY, + kCursorWidth, + kCursorHeight); + const ibus::Rect composition_location(kCompositionX, + kCompositionY, + kCompositionWidth, + kCompositionHeight); // Call SetCursorLocation. - client_->SetCursorLocation(kCursorX, kCursorY, kCursorWidth, kCursorHeight); - // Run the message loop. - message_loop_.RunUntilIdle(); + client_->SetCursorLocation(cursor_location, composition_location); + + EXPECT_EQ(1, on_set_cursor_location_call_count_); + client_->UnsetSetCursorLocationHandler(); } TEST_F(IBusInputContextClientTest, OnProcessKeyEvent) { diff --git a/chromeos/dbus/ibus/ibus_panel_service.cc b/chromeos/dbus/ibus/ibus_panel_service.cc index 6c90ffe..995bb84 100644 --- a/chromeos/dbus/ibus/ibus_panel_service.cc +++ b/chromeos/dbus/ibus/ibus_panel_service.cc @@ -8,6 +8,7 @@ #include "base/bind.h" #include "base/callback.h" #include "chromeos/dbus/ibus/ibus_constants.h" +#include "chromeos/dbus/ibus/ibus_input_context_client.h" #include "chromeos/dbus/ibus/ibus_lookup_table.h" #include "chromeos/dbus/ibus/ibus_property.h" #include "chromeos/dbus/ibus/ibus_text.h" @@ -23,7 +24,8 @@ namespace ibus { class IBusPanelServiceImpl : public IBusPanelService { public: - explicit IBusPanelServiceImpl(dbus::Bus* bus) + explicit IBusPanelServiceImpl(dbus::Bus* bus, + IBusInputContextClient* input_context) : bus_(bus), candidate_window_handler_(NULL), property_handler_(NULL), @@ -124,6 +126,10 @@ class IBusPanelServiceImpl : public IBusPanelService { ibus::panel::kServiceName, base::Bind(&IBusPanelServiceImpl::OnRequestOwnership, weak_ptr_factory_.GetWeakPtr())); + + input_context->SetSetCursorLocationHandler( + base::Bind(&IBusPanelServiceImpl::SetCursorLocation, + weak_ptr_factory_.GetWeakPtr())); } virtual ~IBusPanelServiceImpl() { @@ -338,6 +344,11 @@ class IBusPanelServiceImpl : public IBusPanelService { response_sender.Run(response); } + void SetCursorLocation(const ibus::Rect& cursor_location, + const ibus::Rect& composition_head) { + // TODO(nona): implement this function. + } + // Handles FocusIn, FocusOut, StateChanged method calls from IBus, and ignores // them. void NoOperation(dbus::MethodCall* method_call, @@ -436,10 +447,12 @@ IBusPanelService::~IBusPanelService() { } // static -IBusPanelService* IBusPanelService::Create(DBusClientImplementationType type, - dbus::Bus* bus) { +IBusPanelService* IBusPanelService::Create( + DBusClientImplementationType type, + dbus::Bus* bus, + IBusInputContextClient* input_context) { if (type == REAL_DBUS_CLIENT_IMPLEMENTATION) { - return new IBusPanelServiceImpl(bus); + return new IBusPanelServiceImpl(bus, input_context); } else { return new IBusPanelServiceDaemonlessImpl(); } diff --git a/chromeos/dbus/ibus/ibus_panel_service.h b/chromeos/dbus/ibus/ibus_panel_service.h index 5fde186..7b32638 100644 --- a/chromeos/dbus/ibus/ibus_panel_service.h +++ b/chromeos/dbus/ibus/ibus_panel_service.h @@ -19,6 +19,7 @@ class ObjectPath; } // namespace dbus namespace chromeos { +class IBusInputContextClient; // TODO(nona): Remove ibus namespace after complete libibus removal. namespace ibus { @@ -54,6 +55,8 @@ class CHROMEOS_EXPORT IBusPanelCandidateWindowHandlerInterface { // Called when the IME hides the preedit text. virtual void HidePreeditText() = 0; + // TODO(nona): Introduce SetCursorLocation function. + protected: IBusPanelCandidateWindowHandlerInterface() {} }; @@ -112,9 +115,12 @@ class CHROMEOS_EXPORT IBusPanelService { // Factory function, creates a new instance and returns ownership. // For normal usage, access the singleton via DBusThreadManager::Get(). + // IBusPanelService does not take an ownership of |input_context|, so caller + // should release it. static CHROMEOS_EXPORT IBusPanelService* Create( DBusClientImplementationType type, - dbus::Bus* bus); + dbus::Bus* bus, + IBusInputContextClient* input_context); protected: // Create() should be used instead. diff --git a/chromeos/dbus/ibus/ibus_panel_service_unittest.cc b/chromeos/dbus/ibus/ibus_panel_service_unittest.cc index 85a59e9..43cc325 100644 --- a/chromeos/dbus/ibus/ibus_panel_service_unittest.cc +++ b/chromeos/dbus/ibus/ibus_panel_service_unittest.cc @@ -9,6 +9,7 @@ #include "base/message_loop.h" #include "base/values.h" #include "chromeos/dbus/ibus/ibus_constants.h" +#include "chromeos/dbus/ibus/ibus_input_context_client.h" #include "chromeos/dbus/ibus/ibus_lookup_table.h" #include "chromeos/dbus/ibus/ibus_property.h" #include "chromeos/dbus/ibus/ibus_text.h" @@ -44,6 +45,8 @@ class MockIBusPanelCandidateWindowHandler uint32 cursor_pos, bool visible) ); MOCK_METHOD0(HidePreeditText, void()); + MOCK_METHOD2(SetCursorLocation, void(const ibus::Rect& cursor_location, + const ibus::Rect& composition_head)); private: DISALLOW_COPY_AND_ASSIGN(MockIBusPanelCandidateWindowHandler); @@ -290,10 +293,14 @@ class IBusPanelServiceTest : public testing::Test { AssertOnOriginThread()) .WillRepeatedly(Return()); + stub_input_context_client_.reset(IBusInputContextClient::Create( + STUB_DBUS_CLIENT_IMPLEMENTATION)); + // Create a service service_.reset(IBusPanelService::Create( REAL_DBUS_CLIENT_IMPLEMENTATION, - mock_bus_.get())); + mock_bus_.get(), + stub_input_context_client_.get())); // Set panel handler. candidate_window_handler_.reset(new MockIBusPanelCandidateWindowHandler()); @@ -310,6 +317,8 @@ class IBusPanelServiceTest : public testing::Test { scoped_ptr candidate_window_handler_; // The mock property handler. Do not free, this is owned by IBusPanelService. scoped_ptr property_handler_; + // The stub input context client. + scoped_ptr stub_input_context_client_; // The mock bus. scoped_refptr mock_bus_; // The mock exported object. diff --git a/chromeos/dbus/ibus/mock_ibus_input_context_client.cc b/chromeos/dbus/ibus/mock_ibus_input_context_client.cc index fbaebb1..6946413 100644 --- a/chromeos/dbus/ibus/mock_ibus_input_context_client.cc +++ b/chromeos/dbus/ibus/mock_ibus_input_context_client.cc @@ -31,6 +31,13 @@ void MockIBusInputContextClient::SetInputContextHandler( IBusInputContextHandlerInterface* handler) { } +void MockIBusInputContextClient::SetSetCursorLocationHandler( + const SetCursorLocationHandler& set_cursor_location_handler) { +} + +void MockIBusInputContextClient::UnsetSetCursorLocationHandler() { +} + void MockIBusInputContextClient::ResetObjectProxy() { reset_object_proxy_call_caount_++; is_initialized_ = false; @@ -60,7 +67,8 @@ void MockIBusInputContextClient::Reset() { } void MockIBusInputContextClient::SetCursorLocation( - int32 x, int32 y, int32 w, int32 h) { + const ibus::Rect& cursor_location, + const ibus::Rect& composition_head) { set_cursor_location_call_count_++; } @@ -90,4 +98,11 @@ void MockIBusInputContextClient::PropertyActivate( ibus::IBusPropertyState state) { } +bool MockIBusInputContextClient::IsXKBLayout() { + return true; +} + +void MockIBusInputContextClient::SetIsXKBLayout(bool is_xkb_layout) { +} + } // namespace chromeos diff --git a/chromeos/dbus/ibus/mock_ibus_input_context_client.h b/chromeos/dbus/ibus/mock_ibus_input_context_client.h index 528db41..5e8199e 100644 --- a/chromeos/dbus/ibus/mock_ibus_input_context_client.h +++ b/chromeos/dbus/ibus/mock_ibus_input_context_client.h @@ -28,13 +28,17 @@ class MockIBusInputContextClient : public IBusInputContextClient { const dbus::ObjectPath& object_path) OVERRIDE; virtual void SetInputContextHandler( IBusInputContextHandlerInterface* handler) OVERRIDE; + virtual void SetSetCursorLocationHandler( + const SetCursorLocationHandler& set_cursor_location_handler) OVERRIDE; + virtual void UnsetSetCursorLocationHandler() OVERRIDE; virtual void ResetObjectProxy() OVERRIDE; virtual bool IsObjectProxyReady() const OVERRIDE; virtual void SetCapabilities(uint32 capabilities) OVERRIDE; virtual void FocusIn() OVERRIDE; virtual void FocusOut() OVERRIDE; virtual void Reset() OVERRIDE; - virtual void SetCursorLocation(int32 x, int32 y, int32 w, int32 h) OVERRIDE; + virtual void SetCursorLocation(const ibus::Rect& cursor_location, + const ibus::Rect& composition_head) OVERRIDE; virtual void ProcessKeyEvent(uint32 keyval, uint32 keycode, uint32 state, @@ -45,6 +49,8 @@ class MockIBusInputContextClient : public IBusInputContextClient { uint32 anchor_pos) OVERRIDE; virtual void PropertyActivate(const std::string& key, ibus::IBusPropertyState state) OVERRIDE; + virtual bool IsXKBLayout() OVERRIDE; + virtual void SetIsXKBLayout(bool is_xkb_layout) OVERRIDE; // Call count of Initialize(). int initialize_call_count() const { return initialize_call_count_; } -- cgit v1.1