From 8e6587ecde3178a57de40dd38a61dec0eb38b537 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Thu, 26 Nov 2020 12:37:42 +0000 Subject: [PATCH] Bug 1678553 - part 3: Make `TSFTextStore::Content` store the latest composition range with `Maybe` and new class which stores start and end offsets r=m_kato They are not always set. Therefore, it should be managed with `Maybe`. And this creates a new class `StartAndEndOffsets` and it's similar to `OffsetAndData` class so that similar API makes the code easier to read. Differential Revision: https://phabricator.services.mozilla.com/D97949 --- widget/EventForwards.h | 2 + widget/IMEData.h | 66 +++++++++++++++++++++++ widget/windows/TSFTextStore.cpp | 96 +++++++++++++++------------------ widget/windows/TSFTextStore.h | 27 +++------- 4 files changed, 119 insertions(+), 72 deletions(-) diff --git a/widget/EventForwards.h b/widget/EventForwards.h index e2dd9179f47d..cfb90940efba 100644 --- a/widget/EventForwards.h +++ b/widget/EventForwards.h @@ -462,6 +462,8 @@ enum class TextRangeType : RawTextRangeType; // IMEData.h +template +class StartAndEndOffsets; template class OffsetAndData; diff --git a/widget/IMEData.h b/widget/IMEData.h index 2a148c5fee16..a6e8784dd5c5 100644 --- a/widget/IMEData.h +++ b/widget/IMEData.h @@ -36,6 +36,69 @@ class MOZ_STACK_CLASS PrintStringDetail : public nsAutoCString { static nsCString PrintCharData(char32_t aChar); }; +// StartAndEndOffsets represents a range in flat-text. +template +class StartAndEndOffsets { + protected: + static IntType MaxOffset() { return std::numeric_limits::max(); } + + public: + StartAndEndOffsets() = delete; + explicit StartAndEndOffsets(IntType aStartOffset, IntType aEndOffset) + : mStartOffset(aStartOffset), + mEndOffset(aStartOffset <= aEndOffset ? aEndOffset : aStartOffset) { + MOZ_ASSERT(aStartOffset <= mEndOffset); + } + + IntType StartOffset() const { return mStartOffset; } + IntType Length() const { return mEndOffset - mStartOffset; } + IntType EndOffset() const { return mEndOffset; } + + bool IsOffsetInRange(IntType aOffset) const { + return aOffset >= mStartOffset && aOffset < mEndOffset; + } + bool IsOffsetInRangeOrEndOffset(IntType aOffset) const { + return aOffset >= mStartOffset && aOffset <= mEndOffset; + } + + void MoveTo(IntType aNewStartOffset) { + auto delta = static_cast(mStartOffset) - aNewStartOffset; + mStartOffset += delta; + mEndOffset += delta; + } + void SetOffsetAndLength(IntType aNewOffset, IntType aNewLength) { + mStartOffset = aNewOffset; + CheckedInt endOffset(aNewOffset + aNewLength); + mEndOffset = endOffset.isValid() ? endOffset.value() : MaxOffset(); + } + void SetEndOffset(IntType aEndOffset) { + MOZ_ASSERT(mStartOffset <= aEndOffset); + mEndOffset = std::max(aEndOffset, mStartOffset); + } + void SetStartAndEndOffsets(IntType aStartOffset, IntType aEndOffset) { + MOZ_ASSERT(aStartOffset <= aEndOffset); + mStartOffset = aStartOffset; + mEndOffset = aStartOffset <= aEndOffset ? aEndOffset : aStartOffset; + } + void SetLength(IntType aNewLength) { + CheckedInt endOffset(mStartOffset + aNewLength); + mEndOffset = endOffset.isValid() ? endOffset.value() : MaxOffset(); + } + + friend std::ostream& operator<<( + std::ostream& aStream, + const StartAndEndOffsets& aStartAndEndOffsets) { + aStream << "{ mStartOffset=" << aStartAndEndOffsets.mStartOffset + << ", mEndOffset=" << aStartAndEndOffsets.mEndOffset + << ", Length()=" << aStartAndEndOffsets.Length() << " }"; + return aStream; + } + + private: + IntType mStartOffset; + IntType mEndOffset; +}; + // OffsetAndData class is designed for storing composition string and its // start offset. Length() and EndOffset() return only valid length or // offset. I.e., if the string is too long for inserting at the offset, @@ -58,6 +121,9 @@ class OffsetAndData { return endOffset.isValid() ? mData.Length() : MaxOffset() - mOffset; } IntType EndOffset() const { return mOffset + Length(); } + StartAndEndOffsets CreateStartAndEndOffsets() const { + return StartAndEndOffsets(StartOffset(), EndOffset()); + } const nsString& DataRef() const { // In strictly speaking, we should return substring which may be shrunken // for rounding to the max offset. However, it's unrealistic edge case, diff --git a/widget/windows/TSFTextStore.cpp b/widget/windows/TSFTextStore.cpp index 6ab132f4d3c4..6f67bf152ff8 100644 --- a/widget/windows/TSFTextStore.cpp +++ b/widget/windows/TSFTextStore.cpp @@ -4341,36 +4341,27 @@ TSFTextStore::GetACPFromPoint(TsViewCookie vcView, const POINT* pt, STDMETHODIMP TSFTextStore::GetTextExt(TsViewCookie vcView, LONG acpStart, LONG acpEnd, RECT* prc, BOOL* pfClipped) { - MOZ_LOG( - sTextStoreLog, LogLevel::Info, - ("0x%p TSFTextStore::GetTextExt(vcView=%ld, " - "acpStart=%ld, acpEnd=%ld, prc=0x%p, pfClipped=0x%p), " - "IsHandlingCompositionInParent()=%s, " - "IsHandlingCompositionInContent()=%s, " - "mContentForTSF={ MinOffsetOfLayoutChanged()=%u, " - "LatestCompositionStartOffset()=%d, LatestCompositionEndOffset()=%d, " - "mSelection={ acpStart=%ld, acpEnd=%ld, style.ase=%s, " - "style.fInterimChar=%s } " - "}, mComposition=%s, " - "mDeferNotifyingTSF=%s, mWaitingQueryLayout=%s, " - "IMEHandler::IsA11yHandlingNativeCaret()=%s", - this, vcView, acpStart, acpEnd, prc, pfClipped, - GetBoolName(IsHandlingCompositionInParent()), - GetBoolName(IsHandlingCompositionInContent()), - mContentForTSF.MinOffsetOfLayoutChanged(), - mContentForTSF.HasOrHadComposition() - ? mContentForTSF.LatestCompositionStartOffset() - : -1, - mContentForTSF.HasOrHadComposition() - ? mContentForTSF.LatestCompositionEndOffset() - : -1, - mContentForTSF.Selection().StartOffset(), - mContentForTSF.Selection().EndOffset(), - GetActiveSelEndName(mContentForTSF.Selection().ActiveSelEnd()), - GetBoolName(mContentForTSF.Selection().IsInterimChar()), - ToString(mComposition).c_str(), GetBoolName(mDeferNotifyingTSF), - GetBoolName(mWaitingQueryLayout), - GetBoolName(IMEHandler::IsA11yHandlingNativeCaret()))); + MOZ_LOG(sTextStoreLog, LogLevel::Info, + ("0x%p TSFTextStore::GetTextExt(vcView=%ld, " + "acpStart=%ld, acpEnd=%ld, prc=0x%p, pfClipped=0x%p), " + "IsHandlingCompositionInParent()=%s, " + "IsHandlingCompositionInContent()=%s, mContentForTSF=%s, " + "mSelection={ acpStart=%ld, acpEnd=%ld, style.ase=%s, " + "style.fInterimChar=%s } " + "}, mComposition=%s, " + "mDeferNotifyingTSF=%s, mWaitingQueryLayout=%s, " + "IMEHandler::IsA11yHandlingNativeCaret()=%s", + this, vcView, acpStart, acpEnd, prc, pfClipped, + GetBoolName(IsHandlingCompositionInParent()), + GetBoolName(IsHandlingCompositionInContent()), + mozilla::ToString(mContentForTSF).c_str(), + mContentForTSF.Selection().StartOffset(), + mContentForTSF.Selection().EndOffset(), + GetActiveSelEndName(mContentForTSF.Selection().ActiveSelEnd()), + GetBoolName(mContentForTSF.Selection().IsInterimChar()), + ToString(mComposition).c_str(), GetBoolName(mDeferNotifyingTSF), + GetBoolName(mWaitingQueryLayout), + GetBoolName(IMEHandler::IsA11yHandlingNativeCaret()))); if (!IsReadLocked()) { MOZ_LOG(sTextStoreLog, LogLevel::Error, @@ -4471,7 +4462,7 @@ TSFTextStore::GetTextExt(TsViewCookie vcView, LONG acpStart, LONG acpEnd, // yet, ContentCacheInParent is still open for relative offset query from // the latest composition. options.mRelativeToInsertionPoint = true; - startOffset -= mContentForTSF.LatestCompositionStartOffset(); + startOffset -= mContentForTSF.LatestCompositionRange()->StartOffset(); } else if (!CanAccessActualContentDirectly()) { // If TSF/TIP cannot access actual content directly, there may be pending // text and/or selection changes which have not been notified TSF yet. @@ -4588,10 +4579,10 @@ bool TSFTextStore::MaybeHackNoErrorLayoutBugs(LONG& aACPStart, LONG& aACPEnd) { MOZ_ASSERT(!mComposition.IsComposing() || mComposition.StartOffset() == - mContentForTSF.LatestCompositionStartOffset()); + mContentForTSF.LatestCompositionRange()->StartOffset()); MOZ_ASSERT(!mComposition.IsComposing() || mComposition.EndOffset() == - mContentForTSF.LatestCompositionEndOffset()); + mContentForTSF.LatestCompositionRange()->EndOffset()); // If TSF does not have the bug, we need to hack only with a few TIPs. static const bool sAlllowToStopHackingIfFine = @@ -4708,10 +4699,10 @@ bool TSFTextStore::MaybeHackNoErrorLayoutBugs(LONG& aACPStart, LONG& aACPEnd) { } // If the range is in the composition string, we should return rectangle // in it as far as possible. - if (aACPStart < mContentForTSF.LatestCompositionStartOffset() || - aACPStart > mContentForTSF.LatestCompositionEndOffset() || - aACPEnd < mContentForTSF.LatestCompositionStartOffset() || - aACPEnd > mContentForTSF.LatestCompositionEndOffset()) { + if (!mContentForTSF.LatestCompositionRange()->IsOffsetInRangeOrEndOffset( + aACPStart) || + !mContentForTSF.LatestCompositionRange()->IsOffsetInRangeOrEndOffset( + aACPEnd)) { return false; } break; @@ -4725,10 +4716,10 @@ bool TSFTextStore::MaybeHackNoErrorLayoutBugs(LONG& aACPStart, LONG& aACPEnd) { DoNotReturnNoLayoutErrorToJapanist10OfCompositionString()) { return false; } - if (aACPStart < mContentForTSF.LatestCompositionStartOffset() || - aACPStart > mContentForTSF.LatestCompositionEndOffset() || - aACPEnd < mContentForTSF.LatestCompositionStartOffset() || - aACPEnd > mContentForTSF.LatestCompositionEndOffset()) { + if (!mContentForTSF.LatestCompositionRange()->IsOffsetInRangeOrEndOffset( + aACPStart) || + !mContentForTSF.LatestCompositionRange()->IsOffsetInRangeOrEndOffset( + aACPEnd)) { return false; } break; @@ -4742,7 +4733,7 @@ bool TSFTextStore::MaybeHackNoErrorLayoutBugs(LONG& aACPStart, LONG& aACPEnd) { if (!TSFPrefs::DoNotReturnNoLayoutErrorToFreeChangJie()) { return false; } - aACPEnd = mContentForTSF.LatestCompositionStartOffset(); + aACPEnd = mContentForTSF.LatestCompositionRange()->StartOffset(); aACPStart = std::min(aACPStart, aACPEnd); break; // Some Traditional Chinese TIPs of Microsoft don't show candidate window @@ -4757,7 +4748,7 @@ bool TSFTextStore::MaybeHackNoErrorLayoutBugs(LONG& aACPStart, LONG& aACPEnd) { !TSFPrefs::DoNotReturnNoLayoutErrorToMSTraditionalTIP()) { return false; } - aACPEnd = mContentForTSF.LatestCompositionStartOffset(); + aACPEnd = mContentForTSF.LatestCompositionRange()->StartOffset(); aACPStart = std::min(aACPStart, aACPEnd); break; // Some Simplified Chinese TIPs of Microsoft don't show candidate window @@ -4774,7 +4765,7 @@ bool TSFTextStore::MaybeHackNoErrorLayoutBugs(LONG& aACPStart, LONG& aACPEnd) { !TSFPrefs::DoNotReturnNoLayoutErrorToMSSimplifiedTIP()) { return false; } - aACPEnd = mContentForTSF.LatestCompositionStartOffset(); + aACPEnd = mContentForTSF.LatestCompositionRange()->StartOffset(); aACPStart = std::min(aACPStart, aACPEnd); break; default: @@ -4794,14 +4785,15 @@ bool TSFTextStore::MaybeHackNoErrorLayoutBugs(LONG& aACPStart, LONG& aACPEnd) { static_cast(mContentForTSF.MinOffsetOfLayoutChanged()); LONG lastUnmodifiedOffset = std::max(firstModifiedOffset - 1, 0); if (mContentForTSF.IsLayoutChangedAt(aACPStart)) { - if (aACPStart >= mContentForTSF.LatestCompositionStartOffset()) { + if (aACPStart >= mContentForTSF.LatestCompositionRange()->StartOffset()) { // If mContentForTSF has last composition string and current // composition string, we can assume that ContentCacheInParent has // cached rects of composition string at least length of current // composition string. Otherwise, we can assume that rect for // first character of composition string is stored since it was // selection start or caret position. - LONG maxCachedOffset = mContentForTSF.LatestCompositionEndOffset(); + LONG maxCachedOffset = + mContentForTSF.LatestCompositionRange()->EndOffset(); if (mContentForTSF.LastComposition().isSome()) { maxCachedOffset = std::min( maxCachedOffset, mContentForTSF.LastComposition()->EndOffset()); @@ -4831,8 +4823,8 @@ bool TSFTextStore::MaybeHackNoErrorLayoutBugs(LONG& aACPStart, LONG& aACPEnd) { // should keep using the original aACPEnd. Otherwise, we should set // aACPEnd to larger value of aACPStart and lastUnmodifiedOffset. else if (mContentForTSF.IsLayoutChangedAt(aACPEnd) && - (aACPEnd < mContentForTSF.LatestCompositionStartOffset() || - aACPEnd > mContentForTSF.LatestCompositionEndOffset())) { + !mContentForTSF.LatestCompositionRange()->IsOffsetInRangeOrEndOffset( + aACPEnd)) { aACPEnd = std::max(aACPStart, lastUnmodifiedOffset); } @@ -7122,7 +7114,7 @@ void TSFTextStore::Content::ReplaceTextWith(LONG aStart, LONG aLength, mMinTextModifiedOffset = firstDifferentOffset = mComposition.EndOffset(); } - mLatestCompositionEndOffset = mComposition.EndOffset(); + mLatestCompositionRange = Some(mComposition.CreateStartAndEndOffsets()); MOZ_LOG( sTextStoreLog, LogLevel::Debug, ("0x%p TSFTextStore::Content::ReplaceTextWith(aStart=%d, " @@ -7159,8 +7151,7 @@ void TSFTextStore::Content::StartComposition( aCompositionView, aCompStart.mSelectionStart, GetSubstring(static_cast(aCompStart.mSelectionStart), static_cast(aCompStart.mSelectionLength))); - mLatestCompositionStartOffset = mComposition.StartOffset(); - mLatestCompositionEndOffset = mComposition.EndOffset(); + mLatestCompositionRange = Some(mComposition.CreateStartAndEndOffsets()); if (!aPreserveSelection) { // XXX Do we need to set a new writing-mode here when setting a new // selection? Currently, we just preserve the existing value. @@ -7188,8 +7179,7 @@ void TSFTextStore::Content::RestoreCommittedComposition( // Restore the committed string as composing string. mComposition.Start(aCompositionView, aCanceledCompositionEnd.mSelectionStart, aCanceledCompositionEnd.mData); - mLatestCompositionStartOffset = mComposition.StartOffset(); - mLatestCompositionEndOffset = mComposition.EndOffset(); + mLatestCompositionRange = Some(mComposition.CreateStartAndEndOffsets()); } void TSFTextStore::Content::EndComposition(const PendingAction& aCompEnd) { diff --git a/widget/windows/TSFTextStore.h b/widget/windows/TSFTextStore.h index d346057d7a7e..28bf56aa45e4 100644 --- a/widget/windows/TSFTextStore.h +++ b/widget/windows/TSFTextStore.h @@ -825,7 +825,7 @@ class TSFTextStore final : public ITextStoreACP, mLastComposition.reset(); } mMinTextModifiedOffset = NOT_MODIFIED; - mLatestCompositionStartOffset = mLatestCompositionEndOffset = LONG_MAX; + mLatestCompositionRange.reset(); mInitialized = true; } @@ -883,15 +883,9 @@ class TSFTextStore final : public ITextStoreACP, MOZ_ASSERT(mInitialized); return mMinTextModifiedOffset; } - LONG LatestCompositionStartOffset() const { + const Maybe>& LatestCompositionRange() const { MOZ_ASSERT(mInitialized); - MOZ_ASSERT(HasOrHadComposition()); - return mLatestCompositionStartOffset; - } - LONG LatestCompositionEndOffset() const { - MOZ_ASSERT(mInitialized); - MOZ_ASSERT(HasOrHadComposition()); - return mLatestCompositionEndOffset; + return mLatestCompositionRange; } // Returns true if layout of the character at the aOffset has not been @@ -910,8 +904,7 @@ class TSFTextStore final : public ITextStoreACP, } bool HasOrHadComposition() const { - return mInitialized && mLatestCompositionStartOffset != LONG_MAX && - mLatestCompositionEndOffset != LONG_MAX; + return mLatestCompositionRange.isSome(); } TSFTextStore::Composition& Composition() { return mComposition; } @@ -924,10 +917,8 @@ class TSFTextStore final : public ITextStoreACP, PrintStringDetail::kMaxLengthForEditor) .get() << ", mLastComposition=" << aContent.mLastComposition - << ", mLatestCompositionStartOffset=" - << aContent.mLatestCompositionStartOffset - << ", mLatestCompositionEndOffset=" - << aContent.mLatestCompositionEndOffset + << ", mLatestCompositionRange=" + << aContent.mLatestCompositionRange << ", mMinTextModifiedOffset=" << aContent.mMinTextModifiedOffset << ", mInitialized=" << aContent.mInitialized << " }"; return aStream; @@ -944,10 +935,8 @@ class TSFTextStore final : public ITextStoreACP, TSFTextStore::Composition& mComposition; TSFTextStore::Selection& mSelection; - // The latest composition's start and end offset. If composition hasn't - // been started since this instance is initialized, they are LONG_MAX. - LONG mLatestCompositionStartOffset; - LONG mLatestCompositionEndOffset; + // The latest composition's start and end offset. + Maybe> mLatestCompositionRange; // The minimum offset of modified part of the text. enum : uint32_t { NOT_MODIFIED = UINT32_MAX };