[TSF] nsTextStore should use StaticRefPtr

RESOLVED FIXED in mozilla35

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({inputmethod})

Trunk
mozilla35
x86_64
Windows 8.1
inputmethod
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Comment hidden (empty)
Created attachment 8483368 [details] [diff] [review]
part.1 nsTextStore should use StaticRefPtr for storing COM objects

This makes all static members storing COM objects StaticRefPtr.

In the future, we should move nsTextStore into mozilla::widget namespace for getting rid of the redundant "mozilla::" in nsTextStore.h. However, if we do so now, it breaks my patches which needs to be landed as soon as possible. Therefore, I'll do it later.

And this patch removes Tsf prefix from the static variables because it's very clear that it's related to TSF in nsTextStore.
Attachment #8483368 - Flags: review?(jmathies)
Created attachment 8483369 [details] [diff] [review]
part.2 Rename nsTextStore::sTsfClientId to nsTextStore::sClientId

And also this renames the sTsfClientId to sClientId for consistency with other static member names.
Attachment #8483369 - Flags: review?(jmathies)

Updated

4 years ago
Attachment #8483368 - Flags: review?(jmathies) → review+

Updated

4 years ago
Attachment #8483369 - Flags: review?(jmathies) → review+
Comment on attachment 8483368 [details] [diff] [review]
part.1 nsTextStore should use StaticRefPtr for storing COM objects

>+  static_assert(false,
>+    "GetNativeData() returns pointer to StaticRefPtr<>, fix here for it");

How can this work? Did we already stop compiling TestWinTSF.cpp?
(In reply to Masatoshi Kimura [:emk] from comment #3)
> Comment on attachment 8483368 [details] [diff] [review]
> part.1 nsTextStore should use StaticRefPtr for storing COM objects
> 
> >+  static_assert(false,
> >+    "GetNativeData() returns pointer to StaticRefPtr<>, fix here for it");
> 
> How can this work? Did we already stop compiling TestWinTSF.cpp?

Yes. IIRC, it's disabled due to wrong string API usage. Anyway, the test won't pass with current nsTextStore because the test just checks the behavior of first nsTextStore implementation. In other words, the test assumes that the old implementation is the "correct" implementation as ITextStore.
https://hg.mozilla.org/mozilla-central/rev/ad0c990e9fdc
https://hg.mozilla.org/mozilla-central/rev/47d2dd4ecad1
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.