Make CreateBR return already_AddRefed

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

Trunk
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Bug 1266882 has demonstrated that returning a raw pointer is error-prone, so let's just revert part 5 of bug 1156062. I'll wait to land this until bug 1260651 has landed, to avoid rebasing trouble for that patch.
(Assignee)

Comment 1

2 years ago
Created attachment 8769879 [details] [diff] [review]
Make CreateBR return already_AddRefed.

Returning already_AddRefed rather than a raw pointer makes it harder to make mistakes with refcounting like the one seen in bug 1266882.
Attachment #8769879 - Flags: review?(masayuki)
Comment on attachment 8769879 [details] [diff] [review]
Make CreateBR return already_AddRefed.

>diff --git a/editor/libeditor/WSRunObject.h b/editor/libeditor/WSRunObject.h
>index a7cbeb50..be6c7c5 100644
>--- a/editor/libeditor/WSRunObject.h
>+++ b/editor/libeditor/WSRunObject.h
>@@ -210,19 +210,20 @@ public:
>   static nsresult PrepareToSplitAcrossBlocks(HTMLEditor* aHTMLEditor,
>                                              nsCOMPtr<nsINode>* aSplitNode,
>                                              int32_t* aSplitOffset);
> 
>   // InsertBreak inserts a br node at {aInOutParent,aInOutOffset}
>   // and makes any needed adjustments to ws around that point.
>   // example of fixup: normalws after {aInOutParent,aInOutOffset}
>   //                   needs to begin with nbsp.
>-  dom::Element* InsertBreak(nsCOMPtr<nsINode>* aInOutParent,
>-                            int32_t* aInOutOffset,
>-                            nsIEditor::EDirection aSelect);
>+  already_AddRefed<mozilla::dom::Element>

nit: |mozilla::| isn't necessary. |dom::Element| should be enough.
Attachment #8769879 - Flags: review?(masayuki) → review+

Comment 3

2 years ago
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c622421db18
Make CreateBR return already_AddRefed. r=masayuki
(Assignee)

Comment 4

2 years ago
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #2)
> nit: |mozilla::| isn't necessary. |dom::Element| should be enough.

Fixed.

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3c622421db18
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.