Closed Bug 1385392 Opened 7 years ago Closed 7 years ago

TextEditRules::WillSetText() can pass down its Selection reference to DoTransaction to avoid making it lookup the selection

Categories

(Core :: DOM: Editor, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The call to GetSelection() from DoTransaction() shows up in profiles, for example see https://perfht.ml/2v729RT

When we are coming from TextEditRules::WillSetText() we can easily just keep passing down the Selection& reference we have on the stack instead of paying the cost of looking it up again three frames down the call chain.
Comment on attachment 8891463 [details] [diff] [review]
Avoid needlessly looking up the selection twice when DoTransaction() is called from TextEditRules::WillSetText()

>diff --git a/editor/libeditor/TextEditRules.cpp b/editor/libeditor/TextEditRules.cpp
>index 76f9b14ee59a..b7c2f814b030 100644
>--- a/editor/libeditor/TextEditRules.cpp
>+++ b/editor/libeditor/TextEditRules.cpp
>@@ -898,17 +898,17 @@ TextEditRules::WillSetText(Selection& aSelection,
> 
>   nsINode* curNode = rootElement->GetFirstChild();
>   if (NS_WARN_IF(!EditorBase::IsTextNode(curNode))) {
>     return NS_OK;
>   }
> 
>   // Even if empty text, we don't remove text node and set empty text
>   // for performance
>-  nsresult rv = textEditor->SetTextImpl(tString, *curNode->GetAsText());
>+  nsresult rv = textEditor->SetTextImpl(aSelection, tString, *curNode->GetAsText());

nit: over 80 chars this line.

Otherwise, looks grate!
Attachment #8891463 - Flags: review?(masayuki) → review+
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ae424f348a3
Avoid needlessly looking up the selection twice when DoTransaction() is called from TextEditRules::WillSetText(); r=masayuki
https://hg.mozilla.org/mozilla-central/rev/0ae424f348a3
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee: nobody → ehsan
You need to log in before you can comment on or make changes to this bug.