Closed
Bug 1395146
Opened 7 years ago
Closed 7 years ago
Optimize nsTextEditorState::GetValue()
Categories
(Core :: DOM: Editor, enhancement)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla57
| Tracking | Status | |
|---|---|---|
| firefox57 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
I still see nsTextEditorState::GetValue() when running attachment 8848015 [details]. Mainly, it's slow due to the cost of TextEditor::OutputToString() and number of calls of that. The reason is, nsTextEditorState doesn't use mCachedValue with <input>. I think that it's worthwhile to use it with any editable elements.
I wonder, IMEContentObserver may not need to observe mutations by itself since nsTextEditorState will observe all mutations of <input> and <textarea>. However, changing it needs more changes than I expected and it still needs to be a mutation observer for HTMLEditor. Therefore, even if we do this hack, we might be able to improve the performance of moving focus to/from <input> and <textarea> only a bit. So, I don't want to do that in this bug. If we'll find that it's a bottleneck of something, I'd work on it.
|
Assignee | |
Comment 1•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=93080e6f531659277a704e2d74dd4c8de6f8de43
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 5•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8902734 [details] Bug 1395146 - part1: Get rid of nsITextControlElement::IsPlainTextControl() and nsTextEditorState::IsPlainTextEditor() https://reviewboard.mozilla.org/r/174378/#review179624 Odd old code.
Attachment #8902734 -
Flags: review?(bugs) → review+
|
|
||
Comment 6•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8902735 [details] Bug 1395146 - part2: Make nsTextEditorState cache the value of TextEditor with mCachedValue even when the editor is single line text control https://reviewboard.mozilla.org/r/174380/#review179630 We should probably merge nsAnonDivObserver to nsTextEditorState to reduce memory usage and extra new/delete, but that can happen in a separate bug. ::: commit-message-274f5:1 (Diff revision 1) > +Bug 1395146 - part2: Make nsTextEditorState cache the value of TextEditor with mCachedValue even when the editor isn't single line text control r?ehsan This comment talks about "even when ... isn't single line text control" ::: commit-message-274f5:3 (Diff revision 1) > +Bug 1395146 - part2: Make nsTextEditorState cache the value of TextEditor with mCachedValue even when the editor isn't single line text control r?ehsan > + > +Currently, nsTextEditorState caches the value of TextEditor only when it's for but this about multi-line. I guess this latter is correct, and the first sentence in the comment should have s/isn't/is/
Attachment #8902735 -
Flags: review+
|
|
||
Updated•7 years ago
|
Attachment #8902735 -
Flags: review?(ehsan)
|
|
||
Comment 7•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8902736 [details] Bug 1395146 - part3: nsTextEditorState should cache empty value with mCachedValue https://reviewboard.mozilla.org/r/174382/#review179644
Attachment #8902736 -
Flags: review?(bugs) → review+
|
|
Assignee | |
Comment 8•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8902735 [details] Bug 1395146 - part2: Make nsTextEditorState cache the value of TextEditor with mCachedValue even when the editor is single line text control https://reviewboard.mozilla.org/r/174380/#review179630 > but this about multi-line. > I guess this latter is correct, and the first sentence in the comment should have > s/isn't/is/ Ah, yes. Exactly.
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 12•7 years ago
|
||
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/bd95ed2a8abc part1: Get rid of nsITextControlElement::IsPlainTextControl() and nsTextEditorState::IsPlainTextEditor() r=smaug https://hg.mozilla.org/integration/autoland/rev/bd7325a8425e part2: Make nsTextEditorState cache the value of TextEditor with mCachedValue even when the editor is single line text control r=smaug https://hg.mozilla.org/integration/autoland/rev/929846a88603 part3: nsTextEditorState should cache empty value with mCachedValue r=smaug
|
||
Comment 13•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/bd95ed2a8abc https://hg.mozilla.org/mozilla-central/rev/bd7325a8425e https://hg.mozilla.org/mozilla-central/rev/929846a88603
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•