I figured this out, but I'm not yet sure how to fix it.

The issue is that we set "effectNode.has_render_surface = true" for all effect nodes. Removing this line reduces the test image failures from 1103 to 16.

@Ajuma, is it okay to blindly set that effect nodes have no render surface (https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp?l=545), or should we be creating a render surface for them (as is done in https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?l=934)? I'm also unsure our approach in PropertyTreeManager::buildEffectNodesRecursively is good because we're relying on a lot of assumptions to match cc, such as creating dummy clip nodes. Do you have any recommendations on how to proceed?

In the long run, render surface determination will happen on the compositor thread, so PaintArtifactCompositor won't need to know which effect nodes get surfaces. Getting there depends on finishing off the ongoing work in cc to remove target information from clip and transform nodes.

For now, I think the best approach is for PaintArtifactCompositor to not create any render surfaces except for the root surface (and that's handled by https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp?rcl=0&l=260). Most content will work fine in this mode (according to UMA, renderers have only a single render surface ~95% of the time), though some composited effects (like composited filters, masks, non-axis-aligned clipping, and opacity that applies to multiple layers) will be broken.

Otherwise, if PaintArtifactCompositor does create non-root surfaces, it also needs to:
1) Figure out which effect nodes need surfaces. The logic cc::PropertyTreeBuilder uses for this is in ShouldCreateRenderSurface (https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?rcl=0&l=812).
2) Create dummy clip nodes and transform nodes for each surface, as well as set up target_ids correctly on descendant clip and transform nodes.

Given that (1) and (2) are throw-away code, I think it only makes sense to add this logic if the composited effects that are broken without it are the only things left to work on.

Thank you Ali, your description made this much clearer to me. I agree that we should punt on this for a while (we're definitely not blocked). I'll upload a patch to remove the render surface line which will get us pretty close to removing the non-layer-list codepath from spv2.

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

commit bf1241949838e4e7f72296480087e1a3f691a89c
Author: pdr <pdr@chromium.org>
Date: Thu Aug 11 03:20:06 2016

[spv2] Do not manage effect node render surfaces from blink

Render surface determination will happen on the compositor thread so we
will not need to make that decision in blink/PaintArtifactCompositor.
This patch switches PaintArtifactCompositor to not set the render
surface bit on effect nodes.

With this patch, test failures with spv2 and layer lists [1] are reduced
from 1103 to 16. No new tests are included in this patch but this brings
us very close to being able to turn on --enable-layer-lists by default
for spv2.

[1] --additional-drt-flag=--enable-slimming-paint-v2 --additional-drt-flag=--enable-layer-lists

BUG= 635691 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/bf1241949838e4e7f72296480087e1a3f691a89c/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp