Closed Bug 962005 Opened 6 years ago Closed 6 years ago

Make mozilla::TextComposition refcountable

Categories

(Core :: User events and focus handling, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: inputmethod, Whiteboard: [qa-])

Attachments

(1 file)

TextComposition isn't a countable class now.

But for using/sharing it from editor and nsIMEStateManager, it's better the class to be refcountable.
Attached patch PatchSplinter Review
I worry about the memory fragmentation by this change. If you have better idea, let me know.
Attachment #8362972 - Flags: review?(bugs)
Comment on attachment 8362972 [details] [diff] [review]
Patch

 > TextComposition::TextComposition(const TextComposition& aOther)
> {
>-  mNativeContext = aOther.mNativeContext;
>-  mPresContext = aOther.mPresContext;
>-  mNode = aOther.mNode;
>-  mLastData = aOther.mLastData;
>-  mCompositionStartOffset = aOther.mCompositionStartOffset;
>-  mCompositionTargetOffset = aOther.mCompositionTargetOffset;
>-  mIsSynthesizedForTests = aOther.mIsSynthesizedForTests;
>+  MOZ_CRASH("Don't use copy constructor");
> }
Can't you just have copy ctor unimplemented in the header file.



This makes the code much more manageable, thanks.
Attachment #8362972 - Flags: review?(bugs) → review+
Thank you for the review.

(In reply to Olli Pettay [:smaug] from comment #2)
> Can't you just have copy ctor unimplemented in the header file.

The copy constructor becomes private. I.e., nobody except TextComposition itself cannot use the copy constructor. The MOZ_CRASH() prevents other developers to use it in TextComposition. Isn't it necessary? (copying refcounter makes a bug, I guess)
I meant that do I need to change something before landing the patch?
Flags: needinfo?(bugs)
Does the patch compile if you just have private unimplemented copy-ctor?
If so, land it that way, otherwise land the patch as is
Flags: needinfo?(bugs)
https://hg.mozilla.org/mozilla-central/rev/d56032aebbed
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Whiteboard: [qa-]
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.