Closed Bug 807472 Opened 7 years ago Closed 7 years ago

Don't call NotifyDidPaint more than once per paint

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: jrmuizel, Assigned: roc)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

In this profile http://people.mozilla.com/~bgirard/cleopatra/?report=9dd651fb9b9623449feae2e07cf9445b1a39464a we see NotifyDidPaint being called twice per nsWindow::DispatchStarvedPaints.

For sanity, we should probably just call NotifyDidPaint from nsRefreshDriver::Notify()
Blocks: 807408
This is odd. It looks like we're calling XPConnect()->NotifyDidPaint() from both PresShell::Paint and PresShell::DidPaintWindow, but the call from PresShell::Paint is conditioned on !(aFlags & nsIPresShell::PAINT_WILL_SEND_DID_PAINT)...
Attached patch fixSplinter Review
Assignee: nobody → roc
Attachment #677174 - Flags: review?(matt.woodrow)
Attachment #677174 - Flags: review?(matt.woodrow) → review+
Do we want this on Aurora? It's kind of a regression.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> Do we want this on Aurora? It's kind of a regression.

Sounds nice to me. What caused the regression?
https://hg.mozilla.org/mozilla-central/rev/38d7dcc623a5
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
DLBI and the refresh driver painting changes that happened before that.
Comment on attachment 677174 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): refresh driver painting caused this regression. We turned that off in FF17. So I think we should land this in Aurora so we don't have this issue in FF18 only.
User impact if declined: JS GC will run too often and possibly slow down animated Web pages and games
Testing completed (on m-c, etc.): a few days on m-c
Risk to taking this patch (and alternatives if risky): Regression risk is pretty low; it should only affect JS GC timing.
String or UUID changes made by this patch: none
Attachment #677174 - Flags: approval-mozilla-aurora?
Comment on attachment 677174 [details] [diff] [review]
fix

Approving on Aurora as it is a low risk patch for a regression caused in 18
Attachment #677174 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.