mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1655896 -Use TextChangeData.MergeWith instead. r=geckoview-reviewers,agi 

deleteSurroundingText may create multiple text transactions. This test
failure occurs when multiple text transactions are dispatched to Gecko.

GeckoEditableSupport records text changes by Gecko then it notifies Java
of text changes. And it merges old text changes with new text changes by
AddIMETextChange to reduce JNI/Binder calls if possible.

This issue is that AddIMETextChange creates incorrect text change range
by merging text change ranges.

Actually, TextChangeData already has merge function now, so we should use it
instead of own merge function in GeckoEditableSupport.

Differential Revision: https://phabricator.services.mozilla.com/D111207
This commit is contained in:
Makoto Kato 2021-04-12 03:14:24 +00:00
Родитель d9f6cded53
Коммит 4232c2a5ef
3 изменённых файлов: 40 добавлений и 116 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

1
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/TextInputDelegateTest.kt
Просмотреть файл

@ -459,7 +459,6 @@ class TextInputDelegateTest : BaseSessionTest() {
}
// Test deleteSurroundingText
@Ignore // disable test for frequent failures in bug 1655896
@WithDisplay(width = 512, height = 512) // Child process updates require having a display.
@Test fun inputConnection_deleteSurroundingText() {
setupContent("foobarfoo")

131
widget/android/GeckoEditableSupport.cpp
Просмотреть файл

@ -579,78 +579,17 @@ void GeckoEditableSupport::SendIMEDummyKeyEvent(nsIWidget* aWidget,
mDispatcher->DispatchKeyboardEvent(msg, event, status);
}
void GeckoEditableSupport::AddIMETextChange(const IMETextChange& aChange) {
mIMETextChanges.AppendElement(aChange);
void GeckoEditableSupport::AddIMETextChange(
const IMENotification::TextChangeDataBase& aChange) {
mIMEPendingTextChange.MergeWith(aChange);
// We may not be in the middle of flushing,
// in which case this flag is meaningless.
mIMETextChangedDuringFlush = true;
// Now that we added a new range we need to go back and
// update all the ranges before that.
// Ranges that have offsets which follow this new range
// need to be updated to reflect new offsets
const int32_t delta = aChange.mNewEnd - aChange.mOldEnd;
for (int32_t i = mIMETextChanges.Length() - 2; i >= 0; i--) {
IMETextChange& previousChange = mIMETextChanges[i];
if (previousChange.mStart > aChange.mOldEnd) {
previousChange.mStart += delta;
previousChange.mOldEnd += delta;
previousChange.mNewEnd += delta;
}
}
// Now go through all ranges to merge any ranges that are connected
// srcIndex is the index of the range to merge from
// dstIndex is the index of the range to potentially merge into
int32_t srcIndex = mIMETextChanges.Length() - 1;
int32_t dstIndex = srcIndex;
while (--dstIndex >= 0) {
IMETextChange& src = mIMETextChanges[srcIndex];
IMETextChange& dst = mIMETextChanges[dstIndex];
// When merging a more recent change into an older
// change, we need to compare recent change's (start, oldEnd)
// range to the older change's (start, newEnd)
if (src.mOldEnd < dst.mStart || dst.mNewEnd < src.mStart) {
// No overlap between ranges
continue;
}
if (src.mStart == dst.mStart && src.mNewEnd == dst.mNewEnd) {
// Same range. Adjust old end offset.
dst.mOldEnd = std::min(src.mOldEnd, dst.mOldEnd);
} else {
// When merging two ranges, there are generally four posibilities:
// [----(----]----), (----[----]----),
// [----(----)----], (----[----)----]
// where [----] is the first range and (----) is the second range
// As seen above, the start of the merged range is always the lesser
// of the two start offsets. OldEnd and NewEnd then need to be
// adjusted separately depending on the case. In any case, the change
// in text length of the merged range should be the sum of text length
// changes of the two original ranges, i.e.,
// newNewEnd - newOldEnd == newEnd1 - oldEnd1 + newEnd2 - oldEnd2
dst.mStart = std::min(dst.mStart, src.mStart);
if (src.mOldEnd < dst.mNewEnd) {
// New range overlaps or is within previous range; merge
dst.mNewEnd += src.mNewEnd - src.mOldEnd;
} else { // src.mOldEnd >= dst.mNewEnd
// New range overlaps previous range; merge
dst.mOldEnd += src.mOldEnd - dst.mNewEnd;
dst.mNewEnd = src.mNewEnd;
}
}
// src merged to dst; delete src.
mIMETextChanges.RemoveElementAt(srcIndex);
// Any ranges that we skip over between src and dst are not mergeable
// so we can safely continue the merge starting at dst
srcIndex = dstIndex;
}
}
void GeckoEditableSupport::PostFlushIMEChanges() {
if (!mIMETextChanges.IsEmpty() || mIMESelectionChanged) {
if (mIMEPendingTextChange.IsValid() || mIMESelectionChanged) {
// Already posted
return;
}
@ -680,13 +619,16 @@ void GeckoEditableSupport::FlushIMEChanges(FlushChangesFlag aFlags) {
NS_ENSURE_TRUE_VOID(widget);
struct TextRecord {
TextRecord() : start(-1), oldEnd(-1), newEnd(-1) {}
bool IsValid() const { return start >= 0; }
nsString text;
int32_t start;
int32_t oldEnd;
int32_t newEnd;
};
AutoTArray<TextRecord, 4> textTransaction;
textTransaction.SetCapacity(mIMETextChanges.Length());
TextRecord textTransaction;
nsEventStatus status = nsEventStatus_eIgnore;
mIMETextChangedDuringFlush = false;
@ -708,29 +650,35 @@ void GeckoEditableSupport::FlushIMEChanges(FlushChangesFlag aFlags) {
return true;
};
for (const IMETextChange& change : mIMETextChanges) {
if (change.mStart == change.mOldEnd && change.mStart == change.mNewEnd) {
continue;
}
nsString insertedString;
if (mIMEPendingTextChange.IsValid() &&
(mIMEPendingTextChange.mStartOffset !=
mIMEPendingTextChange.mRemovedEndOffset ||
mIMEPendingTextChange.mStartOffset !=
mIMEPendingTextChange.mAddedEndOffset)) {
WidgetQueryContentEvent queryTextContentEvent(true, eQueryTextContent,
widget);
if (change.mNewEnd != change.mStart) {
if (mIMEPendingTextChange.mAddedEndOffset !=
mIMEPendingTextChange.mStartOffset) {
queryTextContentEvent.InitForQueryTextContent(
change.mStart, change.mNewEnd - change.mStart);
mIMEPendingTextChange.mStartOffset,
mIMEPendingTextChange.mAddedEndOffset -
mIMEPendingTextChange.mStartOffset);
widget->DispatchEvent(&queryTextContentEvent, status);
if (shouldAbort(NS_WARN_IF(queryTextContentEvent.Failed()))) {
return;
}
insertedString = queryTextContentEvent.mReply->DataRef();
textTransaction.text = queryTextContentEvent.mReply->DataRef();
}
textTransaction.AppendElement(TextRecord{insertedString, change.mStart,
change.mOldEnd, change.mNewEnd});
textTransaction.start =
static_cast<int32_t>(mIMEPendingTextChange.mStartOffset);
textTransaction.oldEnd =
static_cast<int32_t>(mIMEPendingTextChange.mRemovedEndOffset);
textTransaction.newEnd =
static_cast<int32_t>(mIMEPendingTextChange.mAddedEndOffset);
}
int32_t selStart = -1;
@ -750,14 +698,12 @@ void GeckoEditableSupport::FlushIMEChanges(FlushChangesFlag aFlags) {
selEnd = static_cast<int32_t>(
querySelectedTextEvent.mReply->SelectionEndOffset());
if (aFlags == FLUSH_FLAG_RECOVER) {
if (aFlags == FLUSH_FLAG_RECOVER && textTransaction.IsValid()) {
// Sometimes we get out-of-bounds selection during recovery.
// Limit the offsets so we don't crash.
for (const TextRecord& record : textTransaction) {
const int32_t end = record.start + record.text.Length();
selStart = std::min(selStart, end);
selEnd = std::min(selEnd, end);
}
const int32_t end = textTransaction.start + textTransaction.text.Length();
selStart = std::min(selStart, end);
selEnd = std::min(selEnd, end);
}
}
@ -784,11 +730,11 @@ void GeckoEditableSupport::FlushIMEChanges(FlushChangesFlag aFlags) {
};
// Commit the text change and selection change transaction.
mIMETextChanges.Clear();
mIMEPendingTextChange.Clear();
for (const TextRecord& record : textTransaction) {
mEditable->OnTextChange(record.text, record.start, record.oldEnd,
record.newEnd);
if (textTransaction.IsValid()) {
mEditable->OnTextChange(textTransaction.text, textTransaction.start,
textTransaction.oldEnd, textTransaction.newEnd);
if (flushOnException()) {
return;
}
@ -815,7 +761,7 @@ void GeckoEditableSupport::FlushIMEText(FlushChangesFlag aFlags) {
"Cannot synchronize Java text with Gecko text");
// Notify Java of the newly focused content
mIMETextChanges.Clear();
mIMEPendingTextChange.Clear();
mIMESelectionChanged = true;
// Use 'INT32_MAX / 2' here because subsequent text changes might combine
@ -1005,9 +951,8 @@ bool GeckoEditableSupport::DoReplaceText(int32_t aStart, int32_t aEnd,
* the NS_COMPOSITION_CHANGE event does not generate a text
* change notification. However, the Java side still expects
* one, so we manually generate a notification. */
IMETextChange dummyChange;
dummyChange.mStart = aStart;
dummyChange.mOldEnd = dummyChange.mNewEnd = aEnd;
IMENotification::TextChangeData dummyChange(aStart, aEnd, aEnd, false,
false);
PostFlushIMEChanges();
mIMESelectionChanged = true;
AddIMETextChange(dummyChange);
@ -1393,7 +1338,7 @@ nsresult GeckoEditableSupport::NotifyIME(
/* Make sure Java's selection is up-to-date */
PostFlushIMEChanges();
mIMESelectionChanged = true;
AddIMETextChange(IMETextChange(aNotification));
AddIMETextChange(aNotification.mTextChangeData);
break;
}

24
widget/android/GeckoEditableSupport.h
Просмотреть файл

@ -46,26 +46,6 @@ class GeckoEditableSupport final
using EditableClient = java::SessionTextInput::EditableClient;
using EditableListener = java::SessionTextInput::EditableListener;
struct IMETextChange final {
int32_t mStart, mOldEnd, mNewEnd;
IMETextChange() : mStart(-1), mOldEnd(-1), mNewEnd(-1) {}
explicit IMETextChange(const IMENotification& aIMENotification)
: mStart(aIMENotification.mTextChangeData.mStartOffset),
mOldEnd(aIMENotification.mTextChangeData.mRemovedEndOffset),
mNewEnd(aIMENotification.mTextChangeData.mAddedEndOffset) {
MOZ_ASSERT(aIMENotification.mMessage == NOTIFY_IME_OF_TEXT_CHANGE,
"IMETextChange initialized with wrong notification");
MOZ_ASSERT(aIMENotification.mTextChangeData.IsValid(),
"The text change notification isn't initialized");
MOZ_ASSERT(aIMENotification.mTextChangeData.IsInInt32Range(),
"The text change notification is out of range");
}
bool IsEmpty() const { return mStart < 0; }
};
enum FlushChangesFlag {
// Not retrying.
FLUSH_FLAG_NONE,
@ -84,7 +64,7 @@ class GeckoEditableSupport final
bool mEditableAttached;
InputContext mInputContext;
AutoTArray<UniquePtr<mozilla::WidgetEvent>, 4> mIMEKeyEvents;
AutoTArray<IMETextChange, 4> mIMETextChanges;
IMENotification::TextChangeData mIMEPendingTextChange;
RefPtr<TextRangeArray> mIMERanges;
RefPtr<Runnable> mDisposeRunnable;
int32_t mIMEMaskEventsCount; // Mask events when > 0.
@ -113,7 +93,7 @@ class GeckoEditableSupport final
RefPtr<TextComposition> GetComposition() const;
bool RemoveComposition(RemoveCompositionFlag aFlag = COMMIT_IME_COMPOSITION);
void SendIMEDummyKeyEvent(nsIWidget* aWidget, EventMessage msg);
void AddIMETextChange(const IMETextChange& aChange);
void AddIMETextChange(const IMENotification::TextChangeDataBase& aChange);
void PostFlushIMEChanges();
void FlushIMEChanges(FlushChangesFlag aFlags = FLUSH_FLAG_NONE);
void FlushIMEText(FlushChangesFlag aFlags = FLUSH_FLAG_NONE);