Revert is here: https://codereview.chromium.org/2488193002

I'll take a look at this today.

I was able to reproduce on samus 56.0.2905.0 but not on minnie 55.0.2878.0

Trace from samus attached. The page uses a timer for DOM update/layout in addition to rAF.

enne/vmpstr: Can you take a look at the trace and tell me if anything stands out? I suspect there's some shared state used for raster/draw between blink and compositor that's being updated outside of commit.

Deleted: trace_uber.json.gz 4.7 MB

trace_uber.json.gz 4.7 MB Download

The only thing that looks suspicious is PaintLayerCompositor::updateIfNeededRecursive that happens with the timer but then not again during the begin frame.  I wonder if there's some missing dirty flag, where something in the Blink compositing code needs to be updated again or is updated incorrectly.

If it were me, I'd hack PaintLayerCompositor to always say that it needed updating and see if that changed anything.

Deleted: ubertracing.png 83.7 KB

	ubertracing.png 83.7 KB View Download

Trace with MFBA disabled attached.

Deleted: trace_MFBA_disabled.json.gz 5.9 MB

trace_MFBA_disabled.json.gz 5.9 MB Download

I removed the early out here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp?rcl=0&l=385

  if (updateType == CompositingUpdateNone)
    return;

However, the bug still reproduces. Attached trace shows that paint update happens in every BeginMainFrame.

Deleted: trace_force_paint_update.json.gz 4.2 MB

trace_force_paint_update.json.gz 4.2 MB Download

The black flashes are correlated with extra UDP/PrepareTiles calls in between commits. If I force the scheduler to PrepareTiles in every frame's deadline I can increase the frequency of the black flashes. If I remove PrepareTiles from the scheduler, the black flashes go away.

vmpstr@ suggested debugging missing paint by enabling some debug options in RasterSource. That ruled out missing paint.

If I enable debug borders in dev tools, the same tiles flash red which means they are OOM. I collected a memory-infra trace (attached) which shows that cc memory usage is ~500 MB for that tab and most of it is image memory.

+ ericrk@

Deleted: trace_memory.json.gz 6.9 MB

trace_memory.json.gz 6.9 MB Download

Thanks for the trace! - a few things I notice:

- Image memory is from the SW controller, and appears to all be unlocked discardable memory. This *should* be free, but it's possible that due to fragmentation/etc, it's not actually free on ChromeOS. It appears that there is a *lot* of discarded memory in the controller as well, indicating that we do have memory pressure and are pushing a lot through the discardable cache.

- There seems to be about ~230mb of tile memory (from all processes) - this seems like it should be fine, but is on the high side.

I'll look a bit more in a bit.

Also, FWIW, enne reminded me that the Tile Manager running out of memory doesn't mean the system is out of memory, just that we're running up against our somewhat arbitrary limit. So Images/other tile managers are likely not the cause.

It seems likely that the page in question just generates too much tile content for us to successfully draw.

Pages running out of tile memory comes up every once in a while.  There may be some better way to handle this in the pre-slimming paint v2 short term, though.

IIRC, the tile memory limit is 512mb per renderer. This means if there's a tree that takes over 256mb and we create a second tree with something like a full invalidation, we're going to run out and activate to checkerboards...

On cros, we should be occluding content so that we don't rasterize too much as well. enne@, did you have some solutions in mind?

If I force PrepareTiles on every draw and turn off MFBA, I can still reproduce the bug although less frequently.

Found the cause:

1. Enter high latency mode.
2. Draw and PrepareTiles.
< No tile priorities modified >
3. Commit.

LTHI::CommitComplete calls NotifyReadyToActivate because tile priorities haven't been modified.

Forcing PrepareTiles after every draw and reducing the time e.g. capture a frame viewer trace :-) between steps 2 & 3 helps for repro.

The bug is fixed if I comment out the early out for tile_priorities_dirty_ in LTHI::PrepareTiles (https://cs.chromium.org/chromium/src/cc/trees/layer_tree_host_impl.cc?rcl=0&l=486).

Why this happens with MFBA?
MFBA forces the compositor into a heavily pipelined state to maximize throughput which makes it more likely that tile priorities aren't modified between steps 2 & 3. MFBA also makes PrepareTiles after draw more likely.

I think we need TileManager to expose if it's expecting activation/draw and not issue those signals in LTHI::CommitComplete on PrepareTiles early out if that's the case.

Excellent investigation! Are you saying that there's some new content from the commit and because there's no PrepareTiles call, that once it activates (and PrepareTiles then) it's just racing rasterization against the draw?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53fabb1f55e8db09a8bd53ca7af11c2c96667af3

commit 53fabb1f55e8db09a8bd53ca7af11c2c96667af3
Author: sunnyps <sunnyps@chromium.org>
Date: Wed Nov 23 20:23:20 2016

cc: Force update tile priorities for pending tree tiling set on commit.

Forcing |UpdateTilePriorities| for pending tree tiling sets to prevent
checkerboarding for cases where compositor is heavily pipelined. In
these cases it's possible for a commit to happen after |PrepareTiles| is
called during draw. The commit then skips |PrepareTiles| and activates
the pending tree causing checkerboarding.

|PrepareTiles| is skipped because of a |PictureLayerTilingSet| early out
optimization that relies on frame time and viewport to avoid updating
tiles. To fix this the tile priorities are always updated on commit.

BUG= 664007 
R=vmpstr@chromium.org
TEST=unittest + manual testing on samus
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2529533002
Cr-Commit-Position: refs/heads/master@{#434236}

[modify] https://crrev.com/53fabb1f55e8db09a8bd53ca7af11c2c96667af3/cc/tiles/picture_layer_tiling_set.cc
[modify] https://crrev.com/53fabb1f55e8db09a8bd53ca7af11c2c96667af3/cc/tiles/picture_layer_tiling_set_unittest.cc

Does this warrant a merge to M55?

Your change meets the bar and is auto-approved for M56 (branch: 2924)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c42c5358255976cf362d5bda542107bcbef52af6

commit c42c5358255976cf362d5bda542107bcbef52af6
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Mon Nov 28 23:27:23 2016

cc: Force update tile priorities for pending tree tiling set on commit.

Forcing |UpdateTilePriorities| for pending tree tiling sets to prevent
checkerboarding for cases where compositor is heavily pipelined. In
these cases it's possible for a commit to happen after |PrepareTiles| is
called during draw. The commit then skips |PrepareTiles| and activates
the pending tree causing checkerboarding.

|PrepareTiles| is skipped because of a |PictureLayerTilingSet| early out
optimization that relies on frame time and viewport to avoid updating
tiles. To fix this the tile priorities are always updated on commit.

BUG= 664007 
R=vmpstr@chromium.org
TEST=unittest + manual testing on samus
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2529533002
Cr-Commit-Position: refs/heads/master@{#434236}
(cherry picked from commit 53fabb1f55e8db09a8bd53ca7af11c2c96667af3)

Review URL: https://codereview.chromium.org/2538593002 .

Cr-Commit-Position: refs/branch-heads/2924@{#140}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/c42c5358255976cf362d5bda542107bcbef52af6/cc/tiles/picture_layer_tiling_set.cc
[modify] https://crrev.com/c42c5358255976cf362d5bda542107bcbef52af6/cc/tiles/picture_layer_tiling_set_unittest.cc

[Automated comment] Less than 2 weeks to go before stable on M55, manual review required.

Change landed in 57.0.2931.0 / 9023.0.0.
Change landed in 56.0.2924.10 / 9000.11.0.

I verified this is fixed on elm in 9035.0.0.

TPMs, did you get a chance to look at this merge for M55? I'm guessing it's too late for M55.

At this point I would agree, we are about to cut our stable RC, unless we believe this to be particularly safe and necessary punting to 56 seems reasonable.

Removing the M55 merge request for now.

Moving old issues out of Internal>Graphics to delete this obsolete component ( crbug.com/685425  for details)

Verified on 9592.0.0/60.0.3112.64