Fire MozAfterPaint once the compositor finishes

RESOLVED FIXED in mozilla31

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

Tracking

(Blocks 1 bug)

Trunk
mozilla31
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Currently we fire these events as soon as we paint layers.

I think this will be an improvement, and should be a good building block for bug 854421.
Assignee: nobody → matt.woodrow
Attachment #8378710 - Flags: review?(roc)
Comment on attachment 8378710 [details] [diff] [review]
Call MozAfterPaint after the compositor runs

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

::: dom/ipc/TabChild.cpp
@@ +104,5 @@
>  static bool sCpowsEnabled = false;
>  static int32_t sActiveDurationMs = 10;
>  static bool sActiveDurationMsSet = false;
>  
> +typedef std::map<uint64_t, TabChild*> TabChildMap;

nsTHashtable

::: dom/ipc/TabChild.h
@@ +518,5 @@
>      ScreenOrientation mOrientation;
>      bool mUpdateHitRegion;
>      bool mContextMenuHandled;
>      bool mWaitingTouchListeners;
> +    bool mLayersId;

Fix!

::: gfx/layers/ipc/PCompositor.ipdl
@@ +38,5 @@
>  child:
>    // The child should invalidate everything so that the whole window is redrawn.
>    async InvalidateAll();
>  
> +  async DidComposite(uint64_t id);

document this
Attachment #8378710 - Flags: review?(roc) → review+
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/eff307d94044 for a cornucopia of failures in b2g reftest-4 and reftest-6 (https://tbpl.mozilla.org/php/getParsedLog.php?id=35180027&tree=Mozilla-Inbound and https://tbpl.mozilla.org/php/getParsedLog.php?id=35183449&tree=Mozilla-Inbound are characteristic, but not exhaustive) and for making layout/reftests/printing/745025-1.html race to intermittent failure, maybe 30% or so on 10.6 opt (which is gifted with a whole lot of slow) and considerably less on 10.8 opt (https://tbpl.mozilla.org/php/getParsedLog.php?id=35190376&tree=Mozilla-Inbound was the only one out of 15 pushes after you).
Found and fixed the b2g bugs. Debugging emulators is not fun.
Attachment #8378710 - Attachment is obsolete: true
Attachment #8387199 - Flags: review?(roc)
Comment on attachment 8387199 [details] [diff] [review]
Call MozAfterPaint after the compositor runs v2

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

::: gfx/layers/ipc/CompositorParent.cpp
@@ +704,5 @@
> +  unused << SendDidComposite(0);
> +
> +  for (LayerTreeMap::iterator it = sIndirectLayerTrees.begin();
> +       it != sIndirectLayerTrees.end(); it++)
> +  {

goes on previous line
Attachment #8387199 - Flags: review?(roc) → review+
https://hg.mozilla.org/mozilla-central/rev/f94ee00aa4d6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Backed out for causing bug 980944:
https://hg.mozilla.org/mozilla-central/rev/7a2edc5171e6
Status: RESOLVED → REOPENED
Depends on: 980944
Resolution: FIXED → ---
Cross post from bug 980944:
Matt, I ran this again (although on a different computer) and it was passing. I don't know what to try apart from another Try run!
NB for comment #9 I used the Gecko from last week together with today's Gaia.
Kevin, any ideas what I should do here?

Afaict, it's not possible to even run the gaia integration tests with a local build of gecko, so I'm not sure how I'm supposed to debug this.

I also can't find any references to MozAfterPaint in gaia or marionette-js, so it's not obvious why my patch would have affected this.

Is there anything special about the places tests (all the tests in this one file fail) that might be relevant?
Flags: needinfo?(kgrandon)
(In reply to Matt Woodrow (:mattwoodrow) from comment #13)
> I also can't find any references to MozAfterPaint in gaia or marionette-js,
> so it's not obvious why my patch would have affected this.

This one might be related:
http://hg.mozilla.org/mozilla-central/annotate/d68c74e48075/dom/browser-element/BrowserElementChildPreload.js#l574
Could be!

From what I can tell, that file results in the events 'mozbrowserfirstpaint', 'mozbrowsernextpaint' and 'mozbrowserdocumentfirstpaint' being fired in the parent process.

I can't see b2g relying on any of those either though :(
For nextpaint:

Something calls browser.waitForNextPaint, e.g. here: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L1388
waitForNextPaint calls addNextPaintListener: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/browser_mixin.js#L56
BrowserElementParent.addNextPaintListener adds the listener to its _nextPaintListeners list: http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementParent.jsm#678

When MozAfterPaint fires,
BrowserElementChildPreload calls sendAsync("nextpaint"): http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#587
BrowserElementParent calls BrowserElementParent._recvNextPaint: http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementParent.jsm#272
And that calls the listeners in nextPaintListeners.
(In reply to Matt Woodrow (:mattwoodrow) from comment #13)
> Kevin, any ideas what I should do here?
> 
> Afaict, it's not possible to even run the gaia integration tests with a
> local build of gecko, so I'm not sure how I'm supposed to debug this.

We should be able to run the tests with a local build of b2g desktop by placing it in the gaia/b2g/ folder. I can probably help with this if needed.
Flags: needinfo?(kgrandon)
This causes a 28.6% tresize regression on osx 10.6:
http://graphs.mozilla.org/graph.html#tests=[[254,63,21]]&sel=none&displayrange=7&datatype=running

when this was backed out it went back to normal
(In reply to Markus Stange [:mstange] from comment #16)
> For nextpaint:
> 
> Something calls browser.waitForNextPaint, e.g. here:
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.
> js#L1388
> waitForNextPaint calls addNextPaintListener:
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/browser_mixin.
> js#L56
> BrowserElementParent.addNextPaintListener adds the listener to its
> _nextPaintListeners list:
> http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/
> BrowserElementParent.jsm#678
> 
> When MozAfterPaint fires,
> BrowserElementChildPreload calls sendAsync("nextpaint"):
> http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/
> BrowserElementChildPreload.js#587
> BrowserElementParent calls BrowserElementParent._recvNextPaint:
> http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/
> BrowserElementParent.jsm#272
> And that calls the listeners in nextPaintListeners.

Thanks Markus, nice work.

Unfortunately I've now confirmed that the breakage isn't related MozAfterPaint in a child process.
Huh?

So it looks like you disabled layers acceleration for B2G Desktop builds. What implications does that have?
https://hg.mozilla.org/mozilla-central/rev/263f232855ad
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
(In reply to Markus Stange [:mstange] from comment #22)
> Huh?
> 
> So it looks like you disabled layers acceleration for B2G Desktop builds.
> What implications does that have?

Seeing a lot more Gi failures than usual again too since this landed.
Flags: needinfo?(matt.woodrow)
Target Milestone: mozilla30 → mozilla31
 https://hg.mozilla.org/integration/mozilla-inbound/rev/fcb4575e90d9

(In reply to Markus Stange [:mstange] from comment #22)
> Huh?
> 
> So it looks like you disabled layers acceleration for B2G Desktop builds.
> What implications does that have?

That was entirely accidental, well spotted. I have no idea how it happened, it wasn't in my try pushes.

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #24)
> Seeing a lot more Gi failures than usual again too since this landed.

This is pretty weird, because disabling layers acceleration would also disable any changes by my patch. Unless the failures are related to the layers acceleration change itself.
Flags: needinfo?(matt.woodrow)
No longer blocks: 997266
Depends on: 997266
Blocks: 924456
You need to log in before you can comment on or make changes to this bug.