1718924 - Make `OffsetEntry` in `TextServicesDocument.cpp` clearer

Status: RESOLVED FIXED

(Core :: Spelling checker, task, P3)

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 1: Get rid of `TextServicesDocument::IsTextNode()` and use `AsText()` if it's safe r=m_kato!

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.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 2: Change `OffsetEntry::mNode` to `OwningNonNull<Text>` r=m_kato!

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

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 3: Rename `OffsetEntry::mNodeOffset` to `mOffsetInTextNode` r=m_kato!

Depends on D119149

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 4: Add `OffsetEntry::EndOffsetInTextNode()` r=m_kato!

Depends on D119150

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 5: Add `OffsetEntry::OffsetInTextNodeIsInRangeOrEndOffset()` r=m_kato!

Depends on D119151

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 6: Rename `OffsetEntry::mStrOffset` to `mOffsetInTextInBlock` r=m_kato!

Depends on D119152

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 7: Add `OffsetEntry::EndOffsetInTextInBlock()` r=m_kato!

Depends on D119153

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 8: Add `OffsetEntry::OffsetInTextInBlockIsInRangeOrEndOffset()` r=m_kato!

Depends on D119154

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 9: Rename `TextServicesDocument::mSel(Start|End)Offset` r=m_kato!

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

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 10: Make all `OffsetEntry` instances unique pointers r=m_kato!

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

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 11: Create custom class of `nsTArray<UniquePtr<OffsetEntry>>` r=m_kato!

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

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 12: Move `TextServicesDocument::FindWordBounds()` to `OffsetEntryArray` r=m_kato!

Depends on D119158

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 13: Move `TextServicesDocument::NodeHasOffsetEntry()` into `OffsetEntryArray` r=m_kato!

Depends on D119159

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 14: Move `TextServicesDocument::SplitOffsetEntry` to `OffsetEntryArray` r=m_kato!

Depends on D119160

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 15: Move `TextServicesDocument::mSelStartIndex` and `TextServicesDocument::mSelEndIndex` into `OffsetEntryArray` r=m_kato!

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

Depends on D119161

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 16: Move `TextServicesDocument::RemoveInvalidOffsetEntries()` to `OffsetEntry` r=m_kato!

Depends on D119162

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 17: Move `TextServicesDocument::mSelectionStartOffsetInTexInBlock` and `TextServicesDocument::mSelectionEndOffsetInTextInBlock` into `OffsetEntryTable` r=m_kato!

Depends on D119163

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1718924 - part 18: Move some blocks which work with `TextServicesDocument::mOffsetTable` into new separated methods r=m_kato!

Depends on D119164

Comment 19

[Pulsebot]

Pushed by [e-mail]:
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

Comment 20

[Pulsebot]

Pushed by [e-mail]:
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

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e43427ec342
part 4: Add `OffsetEntry::EndOffsetInTextNode()` r=m_kato

Comment 22

[Pulsebot]

Pushed by [e-mail]:
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

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/790a5005cdbe
part 7: Add `OffsetEntry::EndOffsetInTextInBlock()` r=m_kato

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a72fb7d3da4
part 8: Add `OffsetEntry::OffsetInTextInBlockIsInRangeOrEndOffset()` r=m_kato

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ba594b4b02d
part 9: Rename `TextServicesDocument::mSel(Start|End)Offset` r=m_kato

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42ff08e8e796
part 10: Make all `OffsetEntry` instances unique pointers r=m_kato

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77740303aae0
part 11: Create custom class of `nsTArray<UniquePtr<OffsetEntry>>` r=m_kato

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/622d9feefc23
part 12: Move `TextServicesDocument::FindWordBounds()` to `OffsetEntryArray` r=m_kato

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0bffa7979736
part 13: Move `TextServicesDocument::NodeHasOffsetEntry()` into `OffsetEntryArray` r=m_kato

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ffbb88fe641
part 14: Move `TextServicesDocument::SplitOffsetEntry` to `OffsetEntryArray` r=m_kato

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b771c6d7f21a
part 15: Move `TextServicesDocument::mSelStartIndex` and `TextServicesDocument::mSelEndIndex` into `OffsetEntryArray` r=m_kato

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/da317e009565
part 16: Move `TextServicesDocument::RemoveInvalidOffsetEntries()` to `OffsetEntry` r=m_kato

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1232d34c7296
part 17: Move `TextServicesDocument::mSelectionStartOffsetInTexInBlock` and `TextServicesDocument::mSelectionEndOffsetInTextInBlock` into `OffsetEntryTable` r=m_kato

Comment 34

[Pulsebot]

Pushed by [e-mail]:
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

Comment 35

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ade9a3381ccb
https://hg.mozilla.org/mozilla-central/rev/58608619e491
https://hg.mozilla.org/mozilla-central/rev/3857f3a91edb
https://hg.mozilla.org/mozilla-central/rev/3e43427ec342
https://hg.mozilla.org/mozilla-central/rev/87409043c8f8
https://hg.mozilla.org/mozilla-central/rev/8d3b1cd13597
https://hg.mozilla.org/mozilla-central/rev/790a5005cdbe
https://hg.mozilla.org/mozilla-central/rev/5a72fb7d3da4
https://hg.mozilla.org/mozilla-central/rev/3ba594b4b02d
https://hg.mozilla.org/mozilla-central/rev/42ff08e8e796
https://hg.mozilla.org/mozilla-central/rev/77740303aae0
https://hg.mozilla.org/mozilla-central/rev/622d9feefc23
https://hg.mozilla.org/mozilla-central/rev/0bffa7979736
https://hg.mozilla.org/mozilla-central/rev/3ffbb88fe641
https://hg.mozilla.org/mozilla-central/rev/b771c6d7f21a
https://hg.mozilla.org/mozilla-central/rev/da317e009565
https://hg.mozilla.org/mozilla-central/rev/1232d34c7296
https://hg.mozilla.org/mozilla-central/rev/6af3e577108b