Commit 8e69907d authored by Yash Malik's avatar Yash Malik Committed by Commit Bot

VR: Request text state correctly

https://chromium-review.googlesource.com/c/chromium/src/+/1012989 corrected the
case when we failed to request text state when the indices matched that of the
stale pending_text_state by clearing pending_text_state. As a result, we didn't
request the text state when the external forces (e.g. JS) cleared the text input
state, and consequently keyboard's knowledge of the text state and the actual
text state would be out of sync.

Bug: 835368
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: I24d28af51e3508cf9fc861585fd6fa2f2256bf01
Reviewed-on: https://chromium-review.googlesource.com/1022612Reviewed-by: default avatarIan Vollick <vollick@chromium.org>
Commit-Queue: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552707}
parent d27270ac
...@@ -190,11 +190,13 @@ void ContentInputDelegate::OnWebInputIndicesChanged( ...@@ -190,11 +190,13 @@ void ContentInputDelegate::OnWebInputIndicesChanged(
// view. This is also how android IME works (it only requests text state when // view. This is also how android IME works (it only requests text state when
// the indices actually change). // the indices actually change).
i = pending_text_input_info_; i = pending_text_input_info_;
if (i.selection_start == selection_start && if (pending_text_request_state_ != kNoPendingRequest &&
i.selection_start == selection_start &&
i.selection_end == selection_end && i.selection_end == selection_end &&
i.composition_start == composition_start && i.composition_start == composition_start &&
i.composition_end == composition_end) { i.composition_end == composition_end) {
pending_text_input_info_ = TextInputInfo(); pending_text_input_info_ = TextInputInfo();
pending_text_request_state_ = kNoPendingRequest;
return; return;
} }
...@@ -204,11 +206,13 @@ void ContentInputDelegate::OnWebInputIndicesChanged( ...@@ -204,11 +206,13 @@ void ContentInputDelegate::OnWebInputIndicesChanged(
pending_text_input_info_.composition_start = composition_start; pending_text_input_info_.composition_start = composition_start;
pending_text_input_info_.composition_end = composition_end; pending_text_input_info_.composition_end = composition_end;
update_state_callbacks_.emplace(std::move(callback)); update_state_callbacks_.emplace(std::move(callback));
pending_text_request_state_ = kRequested;
content_->RequestWebInputText(base::BindOnce( content_->RequestWebInputText(base::BindOnce(
&ContentInputDelegate::OnWebInputTextChanged, base::Unretained(this))); &ContentInputDelegate::OnWebInputTextChanged, base::Unretained(this)));
} }
void ContentInputDelegate::ClearTextInputState() { void ContentInputDelegate::ClearTextInputState() {
pending_text_request_state_ = kNoPendingRequest;
pending_text_input_info_ = TextInputInfo(); pending_text_input_info_ = TextInputInfo();
last_keyboard_edit_ = EditedText(); last_keyboard_edit_ = EditedText();
} }
...@@ -217,6 +221,7 @@ void ContentInputDelegate::OnWebInputTextChanged(const base::string16& text) { ...@@ -217,6 +221,7 @@ void ContentInputDelegate::OnWebInputTextChanged(const base::string16& text) {
pending_text_input_info_.text = text; pending_text_input_info_.text = text;
DCHECK(!update_state_callbacks_.empty()); DCHECK(!update_state_callbacks_.empty());
pending_text_request_state_ = kResponseReceived;
auto update_state_callback = std::move(update_state_callbacks_.front()); auto update_state_callback = std::move(update_state_callbacks_.front());
update_state_callbacks_.pop(); update_state_callbacks_.pop();
base::ResetAndReturn(&update_state_callback).Run(pending_text_input_info_); base::ResetAndReturn(&update_state_callback).Run(pending_text_input_info_);
......
...@@ -109,6 +109,11 @@ class ContentInputDelegate { ...@@ -109,6 +109,11 @@ class ContentInputDelegate {
void ClearTextInputState(); void ClearTextInputState();
private: private:
enum TextRequestState {
kNoPendingRequest,
kRequested,
kResponseReceived,
};
void UpdateGesture(const gfx::PointF& normalized_content_hit_point, void UpdateGesture(const gfx::PointF& normalized_content_hit_point,
blink::WebGestureEvent& gesture); blink::WebGestureEvent& gesture);
void SendGestureToContent(std::unique_ptr<blink::WebInputEvent> event); void SendGestureToContent(std::unique_ptr<blink::WebInputEvent> event);
...@@ -127,6 +132,7 @@ class ContentInputDelegate { ...@@ -127,6 +132,7 @@ class ContentInputDelegate {
PlatformController* controller_ = nullptr; PlatformController* controller_ = nullptr;
EditedText last_keyboard_edit_; EditedText last_keyboard_edit_;
TextRequestState pending_text_request_state_ = kNoPendingRequest;
TextInputInfo pending_text_input_info_; TextInputInfo pending_text_input_info_;
std::queue<base::OnceCallback<void(const TextInputInfo&)>> std::queue<base::OnceCallback<void(const TextInputInfo&)>>
update_state_callbacks_; update_state_callbacks_;
......
...@@ -306,6 +306,10 @@ TEST_F(ContentElementInputEditingTest, IndicesUpdated) { ...@@ -306,6 +306,10 @@ TEST_F(ContentElementInputEditingTest, IndicesUpdated) {
// request the text state, because for example, it may be for a different text // request the text state, because for example, it may be for a different text
// field. // field.
TEST_F(ContentElementInputEditingTest, PendingRequestStateCleared) { TEST_F(ContentElementInputEditingTest, PendingRequestStateCleared) {
// Initial keyboard state.
content_delegate_->OnWebInputEdited(
EditedText(GetInputInfo("a", 1, 1), TextInputInfo()), false);
content_delegate_->OnWebInputIndicesChanged( content_delegate_->OnWebInputIndicesChanged(
2, 2, -1, -1, base::BindOnce([](const TextInputInfo& info) {})); 2, 2, -1, -1, base::BindOnce([](const TextInputInfo& info) {}));
EXPECT_TRUE(input_forwarder_->text_state_requested()); EXPECT_TRUE(input_forwarder_->text_state_requested());
...@@ -319,7 +323,7 @@ TEST_F(ContentElementInputEditingTest, PendingRequestStateCleared) { ...@@ -319,7 +323,7 @@ TEST_F(ContentElementInputEditingTest, PendingRequestStateCleared) {
input_forwarder_->Reset(); input_forwarder_->Reset();
content_delegate_->OnWebInputIndicesChanged( content_delegate_->OnWebInputIndicesChanged(
2, 2, -1, -1, base::BindOnce([](const TextInputInfo& info) {})); 0, 0, -1, -1, base::BindOnce([](const TextInputInfo& info) {}));
// We should request the current text state this time because the call to // We should request the current text state this time because the call to
// OnWebInputIndicesChanged this time can be for a different reason. // OnWebInputIndicesChanged this time can be for a different reason.
EXPECT_TRUE(input_forwarder_->text_state_requested()); EXPECT_TRUE(input_forwarder_->text_state_requested());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment