1449828 - Don't calculate selection rect if unnecessary

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Makoto Kato [:m_kato]]

When profiling bug 1448818, spellchecker spends a lot of time to calculate selection rect.  But it might be unnecessary with willHaveOverflowingSelection = true

Comment 1

[Makoto Kato [:m_kato]]

Attachment: Don't calculate selection rect if unnecessary

Comment 2

[Makoto Kato [:m_kato]]

Comment on attachment 8963450 [details] [diff] [review]
Don't calculate selection rect if unnecessary

When profiling spellchecker issue of bug 1448818, CombineSelectionUnderlineRect spends a lot of time.  If didHaveOverflowingSelection is true, it is unnecessary to calculate selection rect.  So we shouldn't call CombineSelectionUnderlineRect every time.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3b2ddedc7f5
Don't calculate selection rect if unnecessary. r=jfkthame

Comment 4

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/e3b2ddedc7f5

Comment 5

[Makoto Kato [:m_kato]]

Comment on attachment 8963450 [details] [diff] [review]
Don't calculate selection rect if unnecessary

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1403521

[User impact if declined]:
When spellchecker is tuned on textarea (such as wikipedia's edit page), if ther e is a lot of misspell words, moving caret by key operation such as arrow key is slow due to high CPU usages.

[Is this code covered by automated tests?]:
Yes and no.  But this is followed by existed reftests for spellchecker.

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]: 
No

[List of other uplifts needed for the feature/fix]:
N/A

[Is the change risky?]:
Low

[Why is the change risky/not risky?]:
We had unnecessary rectangle calculation for condition.  So we doesn't run it if unnecessary.

[String changes made/needed]:
No

Comment 6

[Makoto Kato [:m_kato]]

User's situation is bug bug 1448818 comment #3

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8963450 [details] [diff] [review]
Don't calculate selection rect if unnecessary

Simple fix for what looks like a pretty easily-hit performance cliff based on the dependent bugs. Let's take it for 60.0b9.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6aba449fa311

Comment 11

[Andrei Vaida [:avaida]]

(In reply to Makoto Kato [:m_kato] from comment #5)
> [Is this code covered by automated tests?]:
> Yes and no.  But this is followed by existed reftests for spellchecker.
> 
> [Has the fix been verified in Nightly?]:
> Yes
> 
> [Needs manual test from QE? If yes, steps to reproduce]: 
> No

Setting qe-verify- based on Makoto Kato's assessment on manual testing needs and the fact that this fix has (some) automated tests.