Closed Bug 1718924 Opened 3 years ago Closed 3 years ago

Make `OffsetEntry` in `TextServicesDocument.cpp` clearer

Categories

(Core :: Spelling checker, task, P3)

task

Tracking

()

RESOLVED FIXED
92 Branch
Tracking Status
firefox92 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

Attachments

(18 files)

(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
No description provided.
Severity: -- → N/A
Priority: -- → P3

It seems that it treats mainly a text node in various places, but it's not
guaranteed by any variable declarations. So, first of all, I'd like to make
it clearer.

TextServicesDocument::IsTextNode() isn't necessary because nsINode::IsText()
is enough useful. And AsText() should be zero cost at runtime. So, in blocks
which guarantee specific content node is a text node, this patch appends
AsText() for making the code clearer.

Now, mNode is always a text node, and it may store across "can run script"
boundaries. So, it should be OwningNonNull<Text>.

Depends on D119148

Now, the meaning of OffsetEntry is clear. Therefore, this patch adds comment
explaining the class and its members.

Then, the meaning of TextServicesDocument::mSelStartOffset and
TextServicesDocument::mSelEndOffset becomes clearer since they are used to
create OffsetEntry instances. Therefore, this patch renames them.

Depends on D119155

Now, it stores dom::Text with OwningNonNull. So, once it's leaked, it
wastes a lot of memory spaces. Therefore, we should make mOffsetTable
store UniquePtr<OffsetEntry> instead of OffsetEntry*.

Depends on D119156

There are some methods in TextServicesDocument which work only with
TextServicesDocument::mOffsetTable. Once we move such methods to custom
class of nsTArray<UniquePtr<OffsetEntry>>, we can make TextServicesDocument
simpler.

Depends on D119157

They are indices of OffsetEntryArray. Therefore, they should be managed in it.

Depends on D119161

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/ade9a3381ccb part 1: Get rid of `TextServicesDocument::IsTextNode()` and use `AsText()` if it's safe r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/58608619e491 part 2: Change `OffsetEntry::mNode` to `OwningNonNull<Text>` r=m_kato https://hg.mozilla.org/integration/autoland/rev/3857f3a91edb part 3: Rename `OffsetEntry::mNodeOffset` to `mOffsetInTextNode` r=m_kato
Attachment #9229825 - Attachment description: Bug 1718924 - part 5: Add `OffsetEntry::OffsetInTextNodeIsRangeOrEndOffset()` r=m_kato! → Bug 1718924 - part 5: Add `OffsetEntry::OffsetInTextNodeIsInRangeOrEndOffset()` r=m_kato!
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/3e43427ec342 part 4: Add `OffsetEntry::EndOffsetInTextNode()` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/87409043c8f8 part 5: Add `OffsetEntry::OffsetInTextNodeIsInRangeOrEndOffset()` r=m_kato https://hg.mozilla.org/integration/autoland/rev/8d3b1cd13597 part 6: Rename `OffsetEntry::mStrOffset` to `mOffsetInTextInBlock` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/790a5005cdbe part 7: Add `OffsetEntry::EndOffsetInTextInBlock()` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/5a72fb7d3da4 part 8: Add `OffsetEntry::OffsetInTextInBlockIsInRangeOrEndOffset()` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/3ba594b4b02d part 9: Rename `TextServicesDocument::mSel(Start|End)Offset` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/42ff08e8e796 part 10: Make all `OffsetEntry` instances unique pointers r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/77740303aae0 part 11: Create custom class of `nsTArray<UniquePtr<OffsetEntry>>` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/622d9feefc23 part 12: Move `TextServicesDocument::FindWordBounds()` to `OffsetEntryArray` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/0bffa7979736 part 13: Move `TextServicesDocument::NodeHasOffsetEntry()` into `OffsetEntryArray` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/3ffbb88fe641 part 14: Move `TextServicesDocument::SplitOffsetEntry` to `OffsetEntryArray` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/b771c6d7f21a part 15: Move `TextServicesDocument::mSelStartIndex` and `TextServicesDocument::mSelEndIndex` into `OffsetEntryArray` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/da317e009565 part 16: Move `TextServicesDocument::RemoveInvalidOffsetEntries()` to `OffsetEntry` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/1232d34c7296 part 17: Move `TextServicesDocument::mSelectionStartOffsetInTexInBlock` and `TextServicesDocument::mSelectionEndOffsetInTextInBlock` into `OffsetEntryTable` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/6af3e577108b part 18: Move some blocks which work with `TextServicesDocument::mOffsetTable` into new separated methods r=m_kato
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: