1141926 - nsCaret should skip ScedulePaint in NotifySelectionChanged when it is invisible

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Ethan Lin[:ethlin]]

In Homescreen, when we touch an item, the nsCarent will do SchedulePaint and carent is invisible. I think we should check the visibility to reduce unnecessary SchedulePaint.

The code stack is as following:
#0  nsIFrame::SchedulePaint (this=0xb0d40a88, aType=aType@entry=nsIFrame::PAINT_DEFAULT) at layout/generic/nsFrame.cpp:5199
#1  0xb57fd33e in nsCaret::SchedulePaint (this=this@entry=0xb3325eb8) at layout/base/nsCaret.cpp:434
#2  0xb57fd548 in NotifySelectionChanged (aDomSel=0xb32cf580, this=0xb3325eb8, aReason=<optimized out>) at layout/base/nsCaret.cpp:571
#3  nsCaret::NotifySelectionChanged (this=0xb3325eb8, aDomSel=0xb32cf580, aReason=<optimized out>) at layout/base/nsCaret.cpp:551
#4  0xb5866d42 in mozilla::dom::Selection::NotifySelectionListeners (this=0xb32cf580) at layout/generic/nsSelection.cpp:5814
#5  0xb5866d86 in nsFrameSelection::NotifySelectionListeners (this=this@entry=0xb322d6a0, aType=aType@entry=1)
    at layout/generic/nsSelection.cpp:2183
#6  0xb586e330 in TakeFocus (aMultipleSelection=false, aContinueSelection=false, aHint=mozilla::CARET_ASSOCIATE_AFTER, aContentEndOffset=0, aContentOffset=<optimized out>,
    aNewFocus=<optimized out>, this=0xb322d6a0) at layout/generic/nsSelection.cpp:1709
#7  nsFrameSelection::TakeFocus (this=0xb322d6a0, aNewFocus=<optimized out>, aContentOffset=<optimized out>, aContentEndOffset=0, aHint=mozilla::CARET_ASSOCIATE_AFTER,
    aContinueSelection=false, aMultipleSelection=false) at /Volumes/firefoxoslayout/generic/nsSelection.cpp:1580
#8  0xb586e55a in HandleClick (aHint=mozilla::CARET_ASSOCIATE_AFTER, aMultipleSelection=false, aContinueSelection=false, aContentEndOffset=0, aContentOffset=0, aNewFocus=0xb3c94500,
    this=0xb322d6a0) at ayout/generic/nsSelection.cpp:1479

Comment 1

[Ethan Lin[:ethlin]]

Attachment: Part1 - Check visibility in NotifySelectionChanged

If nsCaret is invisible, then skip the SchedulePaint in NotifySelectionChanged. The try server result is pass.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=897ba7865d5f

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8575787 [details] [diff] [review]
Part1 - Check visibility in NotifySelectionChanged

Review of attachment 8575787 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsCaret.cpp
@@ +550,5 @@
>  NS_IMETHODIMP
>  nsCaret::NotifySelectionChanged(nsIDOMDocument *, nsISelection *aDomSel,
>                                  int16_t aReason)
>  {
> +  if (aReason & nsISelectionListener::MOUSEUP_REASON || !IsVisible())//this wont do

() around & subexpression

Comment 3

[Ethan Lin[:ethlin]]

Attachment: Part1 - Check visibility in NotifySelectionChanged. r=roc

I applied roc's comment in this patch.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c8c50bb29db7

Comment 5

[Carsten Book [:Tomcat]]

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