1055342 - [TSF] Rename CurrentContent of nsTextStore to LockedContent

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

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

Currently, while the document is locked, nsTextStore caches the content and selection to CurrentContent and CurrentSelection.  But these information are available only while the document is locked.

However, for implement ITfMouseTrackerACP, nsTextStore needs to retrieve content information at installing ITfMouseSink (i.e., when ITfMouseTrackerACP::AdviseSink() is called).

I think that CurrentContent and CurrentSelection should be renamed to Locked*. And we need to make create GetCurrentContent() and GetCurrentSelection() for retrieving them anytime.

Comment 1

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

Er, sorry. CurrentSelection() is also available without the document lock. So, we can keep its name.

Comment 2

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

Attachment: part.1 Rename nsTextStore::mContent and nsTextStore::CurrentContent() to mLockedContent and LockedContent()

Storing all text of focused editor may be bad for footprint. Therefore, we still need to release it at unlocking the document because TIP accesses only during document lock.

So, "locked content" is better name than "(current) content".

This will help when we need to access "current" content without document lock. E.g., at installing a sink.

Comment 3

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

Attachment: part.2 Implement nsTextStore::GetCurrentText() for retrieving current text content even without document lock

This creates a helper method for getting current content.

Comment 4

[Masatoshi Kimura [:emk]]

Comment on attachment 8477320 [details] [diff] [review]
part.1 Rename nsTextStore::mContent and nsTextStore::CurrentContent() to mLockedContent and LockedContent()

>--- a/widget/windows/nsTextStore.h
>+++ b/widget/windows/nsTextStore.h
>@@ -245,18 +245,23 @@ protected:
>   bool     IsReadLock(DWORD aLock) const
>   {
>     return (TS_LF_READ == (aLock & TS_LF_READ));
>   }
>   bool     IsReadWriteLock(DWORD aLock) const
>   {
>     return (TS_LF_READWRITE == (aLock & TS_LF_READWRITE));
>   }
>+  bool     IsReadOrWriteLock(DWORD aLock) const
>+  {
>+    return !!(aLock & TS_LF_READ);
>+  }

What's the difference between IsReadLock and IsReadOrWriteLock?

Comment 5

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

(In reply to Masatoshi Kimura [:emk] from comment #4)
> Comment on attachment 8477320 [details] [diff] [review]
> part.1 Rename nsTextStore::mContent and nsTextStore::CurrentContent() to
> mLockedContent and LockedContent()
> 
> >--- a/widget/windows/nsTextStore.h
> >+++ b/widget/windows/nsTextStore.h
> >@@ -245,18 +245,23 @@ protected:
> >   bool     IsReadLock(DWORD aLock) const
> >   {
> >     return (TS_LF_READ == (aLock & TS_LF_READ));
> >   }
> >   bool     IsReadWriteLock(DWORD aLock) const
> >   {
> >     return (TS_LF_READWRITE == (aLock & TS_LF_READWRITE));
> >   }
> >+  bool     IsReadOrWriteLock(DWORD aLock) const
> >+  {
> >+    return !!(aLock & TS_LF_READ);
> >+  }
> 
> What's the difference between IsReadLock and IsReadOrWriteLock?

Oops, I misunderstood IsReadLock() as IsReadOnlyLock(). I'll remove IsReadOrWriteLock() and use IsReadLock().

Comment 6

[Masatoshi Kimura [:emk]]

Comment on attachment 8477320 [details] [diff] [review]
part.1 Rename nsTextStore::mContent and nsTextStore::CurrentContent() to mLockedContent and LockedContent()

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #5)
> Oops, I misunderstood IsReadLock() as IsReadOnlyLock(). I'll remove
> IsReadOrWriteLock() and use IsReadLock().

OK, r=me with the extra method removed.

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/a180dbeebe4f
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a7771c44e38

Thank you very much, Kimura-san!

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a180dbeebe4f
https://hg.mozilla.org/mozilla-central/rev/8a7771c44e38