Closed Bug 1141926 Opened 6 years ago Closed 6 years ago

nsCaret should skip ScedulePaint in NotifySelectionChanged when it is invisible

Categories

(Core :: Graphics, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: ethlin, Assigned: ethlin)

References

Details

Attachments

(1 file, 1 obsolete file)

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
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
Attachment #8575787 - Flags: feedback?(roc)
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
Attachment #8575787 - Flags: feedback?(roc) → review+
I applied roc's comment in this patch.
Attachment #8575787 - Attachment is obsolete: true
Assignee: nobody → etlin
Keywords: checkin-needed
Blocks: gfxperf
https://hg.mozilla.org/mozilla-central/rev/c8c50bb29db7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.