[ozone/wayland] Ozone Wayland IME integration [chromium/src : master]

Jani Hautakangas (Gerrit)

unread,

Sep 13, 2018, 5:46:17 PM9/13/18

to Shu Chen, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Robert Kroeger, Maksim Sisov, Antonio Gomes

Jani Hautakangas would like Shu Chen to review this change.

View Change

[ozone/wayland] Ozone Wayland IME integration

Introduce Ozone Wayland IME integration.

Implements Ozone Wayland specific LinuxInputMethodContext
and adds possibility to integrate it with various Wayland
text input protocols.

Patch also introduces new SetSurroundingText function to
LinuxInputMethodContext that is internally called by
InputMethodAuraLinux on caret bounds change.

This CL:
- Implements Ozone wayland specific Linux input method
  context interface and integrates it with zwp_text_input_v1 protocol.
- Introduces new runtime flag --enable-wayland-ime to
  enable this feature (by default false).
- Makes Wayland IME working with InputMethodAuraLinux.
- Adds surrounding text support (kudos to msi...@igalia.com).

Bug: 791920
Change-Id: I168d4a266bff6e62acce69edf943469e05e575ac
---
M chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc
M chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.h
M ui/base/BUILD.gn
M ui/base/ime/input_method_auralinux.cc
M ui/base/ime/input_method_auralinux.h
M ui/base/ime/input_method_auralinux_unittest.cc
M ui/base/ime/input_method_factory.cc
M ui/base/ime/linux/fake_input_method_context.cc
M ui/base/ime/linux/fake_input_method_context.h
M ui/base/ime/linux/linux_input_method_context.h
M ui/ozone/platform/wayland/BUILD.gn
M ui/ozone/platform/wayland/DEPS
M ui/ozone/platform/wayland/fake_server.cc
M ui/ozone/platform/wayland/fake_server.h
M ui/ozone/platform/wayland/ozone_platform_wayland.cc
M ui/ozone/platform/wayland/wayland_connection.cc
M ui/ozone/platform/wayland/wayland_connection.h
A ui/ozone/platform/wayland/wayland_input_method_context.cc
A ui/ozone/platform/wayland/wayland_input_method_context.h
A ui/ozone/platform/wayland/wayland_input_method_context_factory.cc
A ui/ozone/platform/wayland/wayland_input_method_context_factory.h
A ui/ozone/platform/wayland/wayland_input_method_context_unittest.cc
M ui/ozone/platform/wayland/wayland_object.cc
M ui/ozone/platform/wayland/wayland_object.h
M ui/ozone/platform/wayland/wayland_window.cc
M ui/ozone/platform/wayland/wayland_window.h
A ui/ozone/platform/wayland/zwp_text_input_wrapper.h
A ui/ozone/platform/wayland/zwp_text_input_wrapper_v1.cc
A ui/ozone/platform/wayland/zwp_text_input_wrapper_v1.h
M ui/ozone/public/ozone_switches.cc
M ui/ozone/public/ozone_switches.h
31 files changed, 1,120 insertions(+), 5 deletions(-)

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Jani Hautakangas (Gerrit)

unread,

Sep 13, 2018, 5:47:07 PM9/13/18

to nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Shu Chen, chromium...@chromium.org, Kalyan Kondapally, James Su, Robert Kroeger, Maksim Sisov, Antonio Gomes, Commit Bot

Patch Set 11:
(9 comments)
Can you explain how this works (perhaps in the patch description.) The patch looks largely good but I'm not particularly expert with Chrome IME. You'll need to eventually add a reviewer from ui/base/ime/OWNERS for ui/base/ime/*

done

View Change

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Jani Hautakangas (Gerrit)

unread,

Sep 13, 2018, 5:55:36 PM9/13/18

to nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Shu Chen, chromium...@chromium.org, Kalyan Kondapally, James Su, Robert Kroeger, Maksim Sisov, Antonio Gomes, Commit Bot

Patch Set 11:
(9 comments)
Can you explain how this works (perhaps in the patch description.) The patch looks largely good but I'm not particularly expert with Chrome IME. You'll need to eventually add a reviewer from ui/base/ime/OWNERS for ui/base/ime/*

Added reviewer from ui/base/ime/OWNERS

View Change

9 comments:

File ui/base/ime/input_method_auralinux.cc:
- Patch Set #11, Line 320: #if !defined(USE_OZONE)
  auralinux is not CrOS?
  nope
File ui/base/ime/input_method_auralinux_unittest.cc:
- Patch Set #11, Line 128: void SetSurroundingText(const base::string16& text,
  You have added a new method to the tested class. […]
  Added unit tests
File ui/ozone/platform/wayland/DEPS:
- Patch Set #11, Line 3: "+ui/base/ime/composition_text.h",
  I'm not sure about this. I need to think about it.
  linux_input_method_context.h uses it in its interface. Won't compile without this dep
File ui/ozone/platform/wayland/fake_server.cc:
- Patch Set #11, Line 31: const uint32_t kXdgShellVersion = 1;
  this isn't in alphabetic order
  fixed
File ui/ozone/platform/wayland/wayland_input_method_context.h:
- Patch Set #11, Line 36: // ui::ZWPTextInputWrapperClient
  these are callbacks? If so from where? can you add method comments?
  They are from ZWPTextInputWrapper. I think it is self explanatory
- Patch Set #11, Line 42: private:
  There are multiple sites have a copy of this reference. […]
  added TODO. This approach is used all around wayland backend currently. Probably best to fix them all together at once at some point
File ui/ozone/platform/wayland/wayland_input_method_context.cc:
- Patch Set #11, Line 39: : connection_(connection), text_input_(nullptr), delegate_(delegate) {
  Can you split this into a separate Init method? Inquiry about state makes it harder to write a unit […]
  removed this env var and added new constructor param for this
File ui/ozone/platform/wayland/wayland_input_method_context_unittest.cc:
- Patch Set #11, Line 60: input_method_context_delegate_ =
  I'd like you to test the code when the variable is not set. It needs to do something reasonable.
  this env var is not needed nor used anymore
File ui/ozone/platform/wayland/zwp_text_input_wrapper.h:
- Patch Set #11, Line 33: // A wrapper around different versions of wayland text input protocols.
  Can you add a bit more about why they exist and why they're needed in a comment?
  Added more details to comments

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Shu Chen (Gerrit)

unread,

Sep 14, 2018, 1:57:16 AM9/14/18

to Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Robert Kroeger, Maksim Sisov, Antonio Gomes, Commit Bot

lgtm for ui/base/ime & c/b/ui/libgtkui

Patch set 13:Code-Review +1

View Change

1 comment:

File ui/ozone/platform/wayland/wayland_input_method_context.cc:
- Patch Set #13, Line 46: avaible
  "avaible" is a possible misspelling of "available".
  Please Fix

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Jani Hautakangas (Gerrit)

unread,

Sep 14, 2018, 4:24:57 AM9/14/18

to nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Robert Kroeger, Maksim Sisov, Antonio Gomes, Commit Bot

Patch Set 13: Code-Review+1
(1 comment)

lgtm for ui/base/ime & c/b/ui/libgtkui

View Change

1 comment:

File ui/ozone/platform/wayland/wayland_input_method_context.cc:
- Patch Set #13, Line 46: availab
  > "avaible" is a possible misspelling of "available". […]
  fixed

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Maksim Sisov (Gerrit)

unread,

Sep 14, 2018, 6:50:30 AM9/14/18

to Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Robert Kroeger, Antonio Gomes, Commit Bot

View Change

17 comments:

File ui/base/ime/input_method_auralinux.cc:
- Patch Set #14, Line 320: #if !defined(USE_OZONE)
  please add a comment why it's needed.
  and as long as it was me, who did it, here is a comment then -
  bluring and triggering focus results in disappearing and popping up text input panel, which is annoying. it happens when a user clicks within the same text he/she types. Maybe, we should add a bool to the OzonePlatform::PlatformProperties. Something like, bool ime_requires_refocusing. And set it to false by default. Then, when we implement an ime for other platforms (x11, for example), it can be set to true for that specific platform and bluring and refocusing will work then and workaround an issue, which exists with that platform.
- Patch Set #14, Line 370: if (GetTextInputClient()) {
  Will it be possible to combine those conditions to
  if (composition_.text.empty() && GetTextInputClient()) ?
File ui/base/ime/linux/fake_input_method_context.cc:
- Patch Set #14, Line 32: {}
  nit: fix brackets. Move one of these below like
  ) {
  }
File ui/ozone/platform/wayland/fake_server.cc:
- Patch Set #14, Line 30: const uint32_t kTextInputManagerVersion = 1;
  can you please fix those to constexpr instead? thanks!
- Patch Set #14, Line 533:
  remove space
File ui/ozone/platform/wayland/wayland_connection.h:
- Patch Set #14, Line 85: GetCurrentKeyboardFocusedWindow
  is it possible to avoid this and just use a focused window method above instead?
- Patch Set #14, Line 100: DispatchUiEvent
  is it possible to keep this in the private and make a callback (base::BindRepeating()) instead, which will be passed to WaylandInputMethodContext?
File ui/ozone/platform/wayland/wayland_input_method_context.cc:
- Patch Set #14, Line 104: Handle selection range
  is it still valid?
- Patch Set #14, Line 105:
  remove space?
File ui/ozone/platform/wayland/wayland_window.h:
- Patch Set #14, Line 60: bool has_keyboard_focus() { return has_keyboard_focus_; }
  const
File ui/ozone/platform/wayland/wayland_window.cc:
- Patch Set #14, Line 201: set_keyboard_focus(true);
  please add a comment why it's needed.
File ui/ozone/platform/wayland/zwp_text_input_wrapper.h:
- Patch Set #14, Line 22: ZWPTextInputWrapperClient
  can you comment this interface as well?
File ui/ozone/platform/wayland/zwp_text_input_wrapper_v1.h:
- Patch Set #14, Line 24: ZWPTextInputWrapperV1
  explicit?
File ui/ozone/platform/wayland/zwp_text_input_wrapper_v1.cc:
- Patch Set #14, Line 18: 0
  nullptr?
- Patch Set #14, Line 93: zwp_text_input_v1_listener
  this is not needed in the cc file
- Patch Set #14, Line 132:
  remove space
- Patch Set #14, Line 197: // Not implemented
  NOTIMPLEMENTED_LOG_ONCE?

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Maksim Sisov (Gerrit)

unread,

Sep 17, 2018, 6:20:08 AM9/17/18

to Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Robert Kroeger, Antonio Gomes, Commit Bot

Robert, do you have any other questions regarding this?

View Change

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Robert Kroeger (Gerrit)

unread,

Sep 18, 2018, 2:37:35 PM9/18/18

to Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Maksim Sisov, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes, Commit Bot

This seems reasonable. But please address my remaining comments and those of Maksim.

View Change

4 comments:

File ui/ozone/platform/wayland/wayland_input_method_context.h:

- Patch Set #11, Line 36: // ui::ZWPTextInputWrapperClient

- They are from ZWPTextInputWrapper. […]
  Indeed. It's unnecessary here.

- Patch Set #11, Line 42: private:

- added TODO. This approach is used all around wayland backend currently. […]
  It is. That doesn't necessarily mean that it should be. But yes, it should be sorted out in a future CL.
File ui/ozone/platform/wayland/wayland_input_method_context_unittest.cc:

- Patch Set #11, Line 60: input_method_context_delegate_ =

- this env var is not needed nor used anymore
  Ack
File ui/ozone/platform/wayland/zwp_text_input_wrapper.h:
- Patch Set #14, Line 26: virtual void OnPreeditString(const std::string& text,
  Please add some more details. If someone needs to re-implement this class someday, more detail would make that easier.

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Jani Hautakangas (Gerrit)

unread,

Sep 19, 2018, 2:44:56 PM9/19/18

to nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Robert Kroeger, Maksim Sisov, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes, Commit Bot

Patch Set 14:
(17 comments)

Fixed the issues pointed out

View Change

18 comments:

File ui/base/ime/input_method_auralinux.cc:
- Patch Set #14, Line 320: is_sync_mode_ = false
  please add a comment why it's needed. […]
  This logic is specific to GTK implementations so I moved these calls to X11InputMethodContextImplGtk::Reset()
- Patch Set #14, Line 370: if (is_sync_mode_) {
  Will it be possible to combine those conditions to […]
  Done
File ui/base/ime/linux/fake_input_method_context.cc:

- Patch Set #14, Line 32: {

- nit: fix brackets. Move one of these below like […]
  Done
File ui/ozone/platform/wayland/fake_server.cc:
- Patch Set #14, Line 30: constexpr uint32_t kTextInputManagerVersion = 1;

- can you please fix those to constexpr instead? thanks!

- Done

- Patch Set #14, Line 533:
  remove space

- Patch Set #14, Line 85: GetCurrentKeyboardFocusedWindow
  is it possible to avoid this and just use a focused window method above instead?

- GetCurrentFocusedWindow tracks pointer focus which might be different from keyboard focus

- Patch Set #14, Line 100:

- is it possible to keep this in the private and make a callback (base::BindRepeating()) instead, whic […]
  Done
File ui/ozone/platform/wayland/wayland_input_method_context.cc:
- Patch Set #14, Line 104: nge& selection_range)
  is it still valid?
  not valid anymore
- Patch Set #14, Line 105: if (text_input_)
  remove space?
  Done
File ui/ozone/platform/wayland/wayland_window.h:
- Patch Set #14, Line 60: bool has_keyboard_focus() const { return has_keyboard_focus_; }
  const
  Done
File ui/ozone/platform/wayland/wayland_window.cc:

- Patch Set #14, Line 201: set_keyboard_focus(true);
  please add a comment why it's needed.

- Done
File ui/ozone/platform/wayland/zwp_text_input_wrapper.h:
- Patch Set #14, Line 22: ent interface which handl

- can you comment this interface as well?

- Done

- Patch Set #14, Line 26:

- Please add some more details. […]
  Done
File ui/ozone/platform/wayland/zwp_text_input_wrapper_v1.h:
- Patch Set #14, Line 24: explicit ZWPTextInput
  explicit?
  Done
File ui/ozone/platform/wayland/zwp_text_input_wrapper_v1.cc:
- Patch Set #14, Line 18: n
  nullptr?
  Done
- Patch Set #14, Line 93: d ZWPTextInputWrapperV1::O

- this is not needed in the cc file

- Done
- Patch Set #14, Line 132: uint32_t length,
  remove space
  Done
- Patch Set #14, Line 197: } // namespace ui
  NOTIMPLEMENTED_LOG_ONCE?
  Done

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Jani Hautakangas (Gerrit)

unread,

Sep 19, 2018, 2:45:24 PM9/19/18

to nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Robert Kroeger, Maksim Sisov, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes, Commit Bot

Patch Set 14:
(4 comments)

This seems reasonable. But please address my remaining comments and those of Maksim.

Done

View Change

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Maksim Sisov (Gerrit)

unread,

Sep 20, 2018, 2:35:57 PM9/20/18

to Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Robert Kroeger, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes, Commit Bot

I guess, it's lgtm. Robert, wdyt?

View Change

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Robert Kroeger (Gerrit)

unread,

Sep 24, 2018, 11:34:09 AM9/24/18

to Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Maksim Sisov, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes, Commit Bot

lgtm. please fix nits before landing.

Patch set 15:Code-Review +1

View Change

2 comments:

File chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.h:
- Patch Set #15, Line 85: // - true if supports table-based input methods
  nit: if it
- Patch Set #15, Line 86: // - false if supports multiple, loadable input methods
  nit: if it

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Maksim Sisov (Gerrit)

unread,

Sep 24, 2018, 11:54:44 AM9/24/18

to Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Robert Kroeger, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes, Commit Bot

Forgot to +1 :)

Patch set 15:Code-Review +1

View Change

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Jani Hautakangas (Gerrit)

unread,

Sep 25, 2018, 2:46:24 PM9/25/18

to nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Maksim Sisov, Robert Kroeger, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes, Commit Bot

Patch Set 15: Code-Review+1
(2 comments)

lgtm. please fix nits before landing.

fixed

View Change

2 comments:

File chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.h:
- Patch Set #15, Line 85: // - true if it supports table-based input methods
  nit: if it
  Done
- Patch Set #15, Line 86: // - false if it supports multiple, loadable input methods
  nit: if it
  Done

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Thomas Anderson (Gerrit)

unread,

Sep 25, 2018, 3:04:29 PM9/25/18

to Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Yuki Shiino, Maksim Sisov, Robert Kroeger, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes, Commit Bot

libgtkui lgtm

Patch set 16:Code-Review +1

View Change

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Jani Hautakangas (Gerrit)

unread,

Sep 25, 2018, 4:21:40 PM9/25/18

to nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Thomas Anderson, Yuki Shiino, Maksim Sisov, Robert Kroeger, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes, Commit Bot

Patch set 16:Commit-Queue +2

View Change

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Commit Bot (Gerrit)

unread,

Sep 25, 2018, 4:26:43 PM9/25/18

to Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Thomas Anderson, Yuki Shiino, Maksim Sisov, Robert Kroeger, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes

Commit Bot merged this change.

View Change

Approvals:
  Robert Kroeger: Looks good to me
  Shu Chen: Looks good to me
  Thomas Anderson: Looks good to me
  Maksim Sisov: Looks good to me
  Jani Hautakangas: Commit

[ozone/wayland] Ozone Wayland IME integration

Introduce Ozone Wayland IME integration.

Implements Ozone Wayland specific LinuxInputMethodContext
and adds possibility to integrate it with various Wayland
text input protocols.

Patch also introduces new SetSurroundingText function to
LinuxInputMethodContext that is internally called by
InputMethodAuraLinux on caret bounds change.

This CL:
- Implements Ozone wayland specific Linux input method
  context interface and integrates it with zwp_text_input_v1 protocol.
- Introduces new runtime flag --enable-wayland-ime to
  enable this feature (by default false).
- Makes Wayland IME working with InputMethodAuraLinux.
- Adds surrounding text support (kudos to msi...@igalia.com).

Bug: 791920
Change-Id: I168d4a266bff6e62acce69edf943469e05e575ac

Reviewed-on: https://chromium-review.googlesource.com/1193872
Reviewed-by: Thomas Anderson <thomasa...@chromium.org>
Reviewed-by: Robert Kroeger <rjkr...@chromium.org>
Reviewed-by: Maksim Sisov <msi...@igalia.com>
Reviewed-by: Shu Chen <shu...@chromium.org>
Commit-Queue: Jani Hautakangas <jani.hau...@lge.com>
Cr-Commit-Position: refs/heads/master@{#594082}

31 files changed, 1,180 insertions(+), 16 deletions(-)

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Yuki Shiino (Gerrit)

unread,

Sep 26, 2018, 1:46:58 AM9/26/18

to Commit Bot, Jani Hautakangas, nona+...@chromium.org, ozone-...@chromium.org, shuche...@chromium.org, yhanad...@chromium.org, yusuke...@chromium.org, Yuki Shiino, Thomas Anderson, Maksim Sisov, Robert Kroeger, Shu Chen, Tricium, chromium...@chromium.org, Kalyan Kondapally, James Su, Antonio Gomes

shuchen@ is the primary owner of IME, so please wait for the review.

ui/base/ime/ LGTM.

I'd recommend to split this patch into two: one for making the existing Aura/Linux IME code ready for Ozone Wayland IME, and another for the rest. It would be easier to observe that there is no regression for existing platforms.

Patch set 17:Code-Review +1

View Change

2 comments:

File ui/base/ime/input_method_auralinux.cc:
- Patch Set #17, Line 360: index >= 0 ? 0U : static_cast<uint32_t>(-1 * index)
  Would you use abs(index)?
File ui/base/ime/input_method_auralinux_unittest.cc:
- Patch Set #17, Line 205: base::string16 surrounding_text;
  s/text_range/text_range_/
  No need to follow the wrong style of |composition_text|, which should be |conposition_text_|.

To view, visit change 1193872. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward