diff --git a/garnet/bin/ui/ime/BUILD.gn b/garnet/bin/ui/ime/BUILD.gn index 91ab54fbbe1341274c63189f6040aaac21ff5789..8f7cce9dd1b340b6fe2b6d596e54dd9da9212d83 100644 --- a/garnet/bin/ui/ime/BUILD.gn +++ b/garnet/bin/ui/ime/BUILD.gn @@ -10,6 +10,7 @@ rustc_binary("ime") { edition = "2018" deps = [ + "//garnet/lib/ui/text/common:text_common", "//garnet/public/lib/fidl/rust/fidl", "//garnet/public/rust/fuchsia-async", "//garnet/public/rust/fuchsia-component", diff --git a/garnet/bin/ui/ime/src/legacy_ime/handler.rs b/garnet/bin/ui/ime/src/legacy_ime/handler.rs index 9f907cf19a09799499f49c67ca6dd7a14aebd1c4..41555ca06d8b4fb20e43581be2945f4916064a9b 100644 --- a/garnet/bin/ui/ime/src/legacy_ime/handler.rs +++ b/garnet/bin/ui/ime/src/legacy_ime/handler.rs @@ -64,7 +64,7 @@ impl Ime { let control_handle = stream.control_handle(); { let mut state = await!(self_clone.0.lock()); - let res = control_handle.send_on_update(&mut state.as_text_field_state()); + let res = control_handle.send_on_update(state.as_text_field_state().into()); if let Err(e) = res { fx_log_err!("{}", e); } else { diff --git a/garnet/bin/ui/ime/src/legacy_ime/state.rs b/garnet/bin/ui/ime/src/legacy_ime/state.rs index 2a0fec135e41732f6023de036307b26dad9047f5..918cc90c0dedf4e7fa88949e903c2fa978e2b179 100644 --- a/garnet/bin/ui/ime/src/legacy_ime/state.rs +++ b/garnet/bin/ui/ime/src/legacy_ime/state.rs @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +use text_common::text_field_state::TextFieldState; use crate::fidl_helpers::clone_state; use crate::ime_service::ImeService; use crate::index_convert as idx; @@ -129,9 +130,9 @@ impl ImeState { ) { self.revision += 1; self.text_points = HashMap::new(); - let mut state = self.as_text_field_state(); + let state = self.as_text_field_state(); if let Some(input_method) = &self.input_method { - if let Err(e) = input_method.send_on_update(&mut state) { + if let Err(e) = input_method.send_on_update(state.into()) { fx_log_err!("error when sending update to TextField listener: {}", e); } } @@ -156,8 +157,8 @@ impl ImeState { } /// Converts the current self.text_state (the IME API v1 representation of the text field's state) - /// into the v2 representation txt::TextFieldState. - pub fn as_text_field_state(&mut self) -> txt::TextFieldState { + /// into the v2 representation TextFieldState. + pub fn as_text_field_state(&mut self) -> TextFieldState { let anchor_first = self.text_state.selection.base < self.text_state.selection.extent; let composition = if self.text_state.composing.start < 0 || self.text_state.composing.end < 0 @@ -171,7 +172,7 @@ impl ImeState { } else { txt::Range { start: end, end: start } }; - Some(Box::new(text_range)) + Some(text_range) }; let selection = txt::Selection { range: txt::Range { @@ -193,7 +194,7 @@ impl ImeState { }, affinity: txt::Affinity::Upstream, }; - txt::TextFieldState { + TextFieldState { document: txt::Range { start: self.new_point(0), end: self.new_point(self.text_state.text.len()), diff --git a/garnet/bin/ui/text/default-hardware-ime/BUILD.gn b/garnet/bin/ui/text/default-hardware-ime/BUILD.gn index cb97858def8a706d8dabb7b7a8708bf9e69182b1..1560157b886121f8ee282a5dac4264f88634a052 100644 --- a/garnet/bin/ui/text/default-hardware-ime/BUILD.gn +++ b/garnet/bin/ui/text/default-hardware-ime/BUILD.gn @@ -9,6 +9,7 @@ rustc_binary("bin") { name = "default-hardware-ime" edition = "2018" deps = [ + "//garnet/lib/ui/text/common:text_common", "//garnet/public/lib/fidl/rust/fidl", "//garnet/public/rust/fuchsia-async", "//garnet/public/rust/fuchsia-component", diff --git a/garnet/bin/ui/text/default-hardware-ime/src/main.rs b/garnet/bin/ui/text/default-hardware-ime/src/main.rs index bf5e69964d3952df2b36f88f614c21216a251841..d443d791b326d7c9c6f0c545167ea477f0f369a1 100644 --- a/garnet/bin/ui/text/default-hardware-ime/src/main.rs +++ b/garnet/bin/ui/text/default-hardware-ime/src/main.rs @@ -14,8 +14,10 @@ use futures::lock::Mutex; use futures::prelude::*; use serde_json::{self as json, Map, Value}; use std::collections::HashMap; +use std::convert::TryInto; use std::fs; use std::sync::Arc; +use text_common::text_field_state::TextFieldState; const DEFAULT_LAYOUT_PATH: &'static str = "/pkg/data/us.json"; @@ -68,7 +70,7 @@ impl DefaultHardwareIme { while let Some(evt) = await!(evt_stream.next()) { match evt { Ok(txt::TextFieldEvent::OnUpdate { state }) => { - await!(this.0.lock()).on_update(state); + await!(this.0.lock()).on_update(state.try_into().unwrap()); } Err(e) => { fx_log_err!( @@ -84,7 +86,7 @@ impl DefaultHardwareIme { } impl DefaultHardwareImeState { - fn on_update(&mut self, state: txt::TextFieldState) { + fn on_update(&mut self, state: TextFieldState) { self.last_selection = Some(state.selection); self.last_revision = Some(state.revision); } diff --git a/garnet/bin/ui/text/test_suite/BUILD.gn b/garnet/bin/ui/text/test_suite/BUILD.gn index f2908b9c82946f60b59fd91695cdd86a7de69eb1..43526746be8588f9df6f2a3bd14664aa5d5ff091 100644 --- a/garnet/bin/ui/text/test_suite/BUILD.gn +++ b/garnet/bin/ui/text/test_suite/BUILD.gn @@ -12,6 +12,7 @@ rustc_binary("test_suite") { edition = "2018" deps = [ + "//garnet/lib/ui/text/common:text_common", "//garnet/public/lib/fidl/rust/fidl", "//garnet/public/rust/fuchsia-async", "//garnet/public/rust/fuchsia-component", diff --git a/garnet/bin/ui/text/test_suite/src/main.rs b/garnet/bin/ui/text/test_suite/src/main.rs index 3961739babb24a9b9753324dee22432f25e5b4e6..a9587f1265abbb8ff344f9f9cd9246a4d5c8e0c4 100644 --- a/garnet/bin/ui/text/test_suite/src/main.rs +++ b/garnet/bin/ui/text/test_suite/src/main.rs @@ -46,8 +46,7 @@ fn main() -> Result<(), Error> { let mut executor = fuchsia_async::Executor::new() .context("Creating fuchsia_async executor for text tests failed")?; let mut fs = ServiceFs::new(); - fs.dir("public") - .add_fidl_service(bind_text_tester); + fs.dir("public").add_fidl_service(bind_text_tester); fs.take_and_serve_directory_handle()?; executor.run_singlethreaded(fs.collect::<()>()); Ok(()) diff --git a/garnet/bin/ui/text/test_suite/src/test_helpers.rs b/garnet/bin/ui/text/test_suite/src/test_helpers.rs index 9bc2de63150a2a0f52829add692a5d12d27fdd24..87045de699b815d867af0443d766ae8ffae15d12 100644 --- a/garnet/bin/ui/text/test_suite/src/test_helpers.rs +++ b/garnet/bin/ui/text/test_suite/src/test_helpers.rs @@ -2,15 +2,17 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +use text_common::text_field_state::TextFieldState; use failure::{bail, err_msg, Error, ResultExt}; use fidl_fuchsia_ui_text as txt; use fuchsia_async::TimeoutExt; use futures::prelude::*; use std::collections::HashSet; +use std::convert::TryInto; pub struct TextFieldWrapper { proxy: txt::TextFieldProxy, - last_state: txt::TextFieldState, + last_state: TextFieldState, defunct_point_ids: HashSet<u64>, current_point_ids: HashSet<u64>, } @@ -37,38 +39,15 @@ impl TextFieldWrapper { /// of the editing methods on the TextFieldWrapper, or if making calls on the proxy directly, /// call `await!(text_field_wrapper.wait_for_update())` after you expect a new state update from /// the TextField. - pub fn state(&self) -> txt::TextFieldState { - fn clone_range(range: &txt::Range) -> txt::Range { - txt::Range { - start: txt::Position { id: range.start.id }, - end: txt::Position { id: range.end.id }, - } - } - txt::TextFieldState { - document: clone_range(&self.last_state.document), - selection: txt::Selection { - range: clone_range(&self.last_state.selection.range), - anchor: self.last_state.selection.anchor, - affinity: self.last_state.selection.affinity, - }, - composition: self.last_state.composition.as_ref().map(|v| Box::new(clone_range(v))), - composition_highlight: self - .last_state - .composition_highlight - .as_ref() - .map(|v| Box::new(clone_range(v))), - dead_key_highlight: self - .last_state - .dead_key_highlight - .as_ref() - .map(|v| Box::new(clone_range(v))), - revision: self.last_state.revision, - } + pub fn state(&self) -> TextFieldState { + self.last_state.clone() } /// Waits for an on_update event from the TextFieldProxy, and updates the last state tracked /// by TextFieldWrapper. Edit functions on TextFieldWrapper itself already call this; only - /// use it if you're doing something with the TextFieldProxy directly. + /// use it if you're doing something with the TextFieldProxy directly. This also validates + /// that document, selection, and revision are all set on last_state, so these fields can be + /// unwrapped in other parts of the code. pub async fn wait_for_update(&mut self) -> Result<(), Error> { self.defunct_point_ids = &self.defunct_point_ids | &all_point_ids_for_state(&self.last_state); @@ -229,7 +208,7 @@ impl TextFieldWrapper { } } -async fn get_update(text_field: &txt::TextFieldProxy) -> Result<txt::TextFieldState, Error> { +async fn get_update(text_field: &txt::TextFieldProxy) -> Result<TextFieldState, Error> { let mut stream = text_field.take_event_stream(); let msg_future = stream .try_next() @@ -237,11 +216,11 @@ async fn get_update(text_field: &txt::TextFieldProxy) -> Result<txt::TextFieldSt .on_timeout(*crate::TEST_TIMEOUT, || Err(err_msg("Waiting for on_update event timed out"))); let msg = await!(msg_future)?.ok_or(err_msg("TextMgr event stream unexpectedly closed"))?; match msg { - txt::TextFieldEvent::OnUpdate { state, .. } => Ok(state), + txt::TextFieldEvent::OnUpdate { state, .. } => Ok(state.try_into()?), } } -fn all_point_ids_for_state(state: &txt::TextFieldState) -> HashSet<u64> { +fn all_point_ids_for_state(state: &TextFieldState) -> HashSet<u64> { let mut point_ids = HashSet::new(); let mut point_ids_for_range = |range: &txt::Range| { point_ids.insert(range.start.id); @@ -261,8 +240,8 @@ mod test { fn default_range(n: u64) -> txt::Range { txt::Range { start: txt::Position { id: n }, end: txt::Position { id: n + 1 } } } - fn default_state(n: u64) -> txt::TextFieldState { - txt::TextFieldState { + fn default_state(n: u64) -> TextFieldState { + TextFieldState { document: default_range(n), selection: txt::Selection { range: default_range(n + 2), @@ -284,7 +263,7 @@ mod test { let (mut stream, control_handle) = server_end .into_stream_and_control_handle() .expect("Should have created stream and control handle"); - control_handle.send_on_update(&mut default_state(0)).expect("Should have sent update"); + control_handle.send_on_update(default_state(0).into()).expect("Should have sent update"); fuchsia_async::spawn( async { let mut wrapper = await!(TextFieldWrapper::new(proxy)) @@ -327,20 +306,20 @@ mod test { let (_stream, control_handle) = server_end .into_stream_and_control_handle() .expect("Should have created stream and control handle"); - control_handle.send_on_update(&mut default_state(0)).expect("Should have sent update"); + control_handle.send_on_update(default_state(0).into()).expect("Should have sent update"); let mut wrapper = await!(TextFieldWrapper::new(proxy)).expect("Should have created text field wrapper"); // send a valid update and make sure it works as expected let mut state = default_state(10); - control_handle.send_on_update(&mut state).expect("Should have sent update"); + control_handle.send_on_update(state.clone().into()).expect("Should have sent update"); let res = await!(wrapper.wait_for_update()); assert!(res.is_ok()); assert_eq!(wrapper.state().document.start.id, 10); // send an update with the same points but an incremented revision state.revision += 1; - control_handle.send_on_update(&mut state).expect("Should have sent update"); + control_handle.send_on_update(state.into()).expect("Should have sent update"); let res = await!(wrapper.wait_for_update()); assert!(res.is_err()); // should fail since some points were reused } diff --git a/garnet/lib/ui/text/common/BUILD.gn b/garnet/lib/ui/text/common/BUILD.gn new file mode 100644 index 0000000000000000000000000000000000000000..1bfb2df1dd7f1c5206019b5e930e4df3d22c6aee --- /dev/null +++ b/garnet/lib/ui/text/common/BUILD.gn @@ -0,0 +1,28 @@ +# Copyright 2019 The Fuchsia Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import("//build/rust/rustc_library.gni") +import("//build/test/test_package.gni") + +rustc_library("text_common") { + with_unit_tests = true + edition = "2018" + deps = [ + "//garnet/public/lib/fidl/rust/fidl", + "//garnet/public/rust/fuchsia-async", + "//garnet/public/rust/fuchsia-component", + "//garnet/public/rust/fuchsia-syslog", + "//garnet/public/rust/fuchsia-zircon", + "//sdk/fidl/fuchsia.ui.input:fuchsia.ui.input-rustc", + "//sdk/fidl/fuchsia.ui.text:fuchsia.ui.text-rustc", + "//sdk/fidl/fuchsia.ui.text.testing:fuchsia.ui.text.testing-rustc", + "//third_party/rust_crates:failure", + "//third_party/rust_crates:futures-preview", + "//third_party/rust_crates:lazy_static", + "//third_party/rust_crates:parking_lot", + "//third_party/rust_crates:pin-utils", + "//third_party/rust_crates:regex", + "//third_party/rust_crates:unicode-segmentation", + ] +} \ No newline at end of file diff --git a/garnet/lib/ui/text/common/src/lib.rs b/garnet/lib/ui/text/common/src/lib.rs new file mode 100644 index 0000000000000000000000000000000000000000..f1fa0f05213d87a389f7e4926edd5964b08b1103 --- /dev/null +++ b/garnet/lib/ui/text/common/src/lib.rs @@ -0,0 +1,5 @@ +// Copyright 2019 The Fuchsia Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +pub mod text_field_state; diff --git a/garnet/lib/ui/text/common/src/text_field_state.rs b/garnet/lib/ui/text/common/src/text_field_state.rs new file mode 100644 index 0000000000000000000000000000000000000000..dfc7ce0529b1a080c7598919f812e65e899a77b1 --- /dev/null +++ b/garnet/lib/ui/text/common/src/text_field_state.rs @@ -0,0 +1,105 @@ +// Copyright 2019 The Fuchsia Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +use core::convert::TryFrom; +use failure::{err_msg, Error}; +use fidl_fuchsia_ui_text as txt; + +/// A version of txt::TextFieldState that does not have mandatory fields wrapped in Options. +/// It also implements Clone. +pub struct TextFieldState { + pub document: txt::Range, + pub selection: txt::Selection, + pub revision: u64, + pub composition: Option<txt::Range>, + pub composition_highlight: Option<txt::Range>, + pub dead_key_highlight: Option<txt::Range>, +} + +impl Clone for TextFieldState { + fn clone(&self) -> Self { + TextFieldState { + document: clone_range(&self.document), + selection: txt::Selection { + range: clone_range(&self.selection.range), + anchor: self.selection.anchor, + affinity: self.selection.affinity, + }, + revision: self.revision, + composition: self.composition.as_ref().map(clone_range), + composition_highlight: self.composition_highlight.as_ref().map(clone_range), + dead_key_highlight: self.dead_key_highlight.as_ref().map(clone_range), + } + } +} + +impl TryFrom<txt::TextFieldState> for TextFieldState { + type Error = Error; + fn try_from(state: txt::TextFieldState) -> Result<Self, Self::Error> { + let txt::TextFieldState { + revision, + selection, + document, + composition, + composition_highlight, + dead_key_highlight, + } = state; + let document = match document { + Some(v) => v, + None => { + return Err(err_msg(format!("Expected document field to be set on TextFieldState"))) + } + }; + let selection = match selection { + Some(v) => v, + None => { + return Err(err_msg(format!( + "Expected selection field to be set on TextFieldState" + ))) + } + }; + let revision = match revision { + Some(v) => v, + None => { + return Err(err_msg(format!("Expected revision field to be set on TextFieldState"))) + } + }; + Ok(TextFieldState { + document, + selection, + revision, + composition, + composition_highlight, + dead_key_highlight, + }) + } +} + +impl Into<txt::TextFieldState> for TextFieldState { + fn into(self) -> txt::TextFieldState { + let TextFieldState { + revision, + selection, + document, + composition, + composition_highlight, + dead_key_highlight, + } = self; + txt::TextFieldState { + document: Some(document), + selection: Some(selection), + revision: Some(revision), + composition, + composition_highlight, + dead_key_highlight, + } + } +} + +fn clone_range(range: &txt::Range) -> txt::Range { + txt::Range { + start: txt::Position { id: range.start.id }, + end: txt::Position { id: range.end.id }, + } +} diff --git a/sdk/fidl/fuchsia.ui.text/text_field.fidl b/sdk/fidl/fuchsia.ui.text/text_field.fidl index c2eae84c6da41fc2e80a055498a32dcf0c1b9550..1a334c28d2eddada2fa05394ee89b9ca2f271fb4 100644 --- a/sdk/fidl/fuchsia.ui.text/text_field.fidl +++ b/sdk/fidl/fuchsia.ui.text/text_field.fidl @@ -7,23 +7,23 @@ library fuchsia.ui.text; /// Lists the Positions for selection and other related ranges, at a particular /// revision number. Any time the revision number is incremented, all these Positions /// become invalid, and a new TextFieldState is sent through OnUpdate. -struct TextFieldState { - /// The start and end of the entire text field. - Range document; +table TextFieldState { + /// (required) The start and end of the entire text field. + 1: Range document; - /// The currently selected range of text. - Selection selection; + /// (required) The currently selected range of text. + 2: Selection selection; /// The range that indicates the text that is being composed, or currently /// receiving suggestions from the keyboard. It should be displayed in some /// distinct way, such as underlined. - Range? composition; + 3: Range composition; /// Some keyboards, notably Japanese, give the user buttons to highlight just a /// subset of the composition string for suggestions. It must be equal to or a subset /// of the composition range. If the composition range changes, the TextField may /// discard this and require the keyboard to create a new one. - Range? composition_highlight; + 4: Range composition_highlight; /// A dead key is a key combination you press before another key to add diacritical /// marks, accents, or other changes to the second key. After the first key, a @@ -31,11 +31,11 @@ struct TextFieldState { /// range is that highlighted character. If the selection moves away from this /// highlight range, or if the contents of the highlight range change, the TextField /// may discard this and require the keyboard to create a new one. - Range? dead_key_highlight; + 5: Range dead_key_highlight; - /// This number is increased any time content in the text field is changed, if the - /// selection is changed, or if anything else about the state is changed. - uint64 revision; + /// (required) This number is increased any time content in the text field is changed, + /// if the selection is changed, or if anything else about the state is changed. + 6: uint64 revision; }; /// Indicates errors that can occur with various TextField methods. Until FIDL supports