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

RESOLVED FIXED in Firefox 56

Status

()

Core
Editor
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: Away for a while, Unassigned)

Tracking

(Blocks: 1 bug)

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 months ago
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.
(Reporter)

Comment 1

3 months ago
Created attachment 8891463 [details] [diff] [review]
Avoid needlessly looking up the selection twice when DoTransaction() is called from TextEditRules::WillSetText()
Attachment #8891463 - Flags: review?(masayuki)
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+

Comment 3

3 months ago
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

Comment 4

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0ae424f348a3
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.