Initial bisect is to https://chromium.googlesource.com/chromium/src/+log/4fd8a827a8b9c51cbedce4060ee7b841edeee3c5..c5f1009cd42d24d77fb811042815cf5d94c770fb

There are a couple of potential culprits but nothing screams liability.

CC danakj@ because "Move SetContentsOpaqueIsFixed() from WebLayer to GraphicsLayer" is the most likely candidate if the bisect is correct, but I thought the change was not affecting behavior at all.


I'll bisect again and check.

Bisect reproduces.

I'm suspecting https://chromium.googlesource.com/chromium/src/+/e1b6426ea838e2c0dc2d2b1893c5873cce1e53ac because nothing in the Skia roll seems to account for this. enne@, pdr@, as reviewers of the patch could you take a look, and maybe try to revert, to see if the blame is right. danajk@ is out for a while.

This is an M68 regression, mark as blocking?

The problem is transitional, so I am loath to make it blocking in any sense. That is, ugly but not site-breaking.

Blame certainly looks suspicious to me.  Opaque layers in cc (because they're opaque) end up getting the background color filled in on the bottom and right side to make sure they have opaque pixels when scaled.  So, this isn't too surprising to me.  I would probably say that this is the expected behavior for opaque layers.

However, I think the surprising thing is that this layer itself is marked as opaque.  Very few layers get marked as opaque in the renderer.  This line is the most suspicious to me as being a change in behavior (in that image layers do not seem to have been previously fixed):

SetContentsTo(image_layer_ ? image_layer_->Layer() : nullptr, true);

Right, image layers as far as I know are never opaque because we don't know enough about the image before we decode it for raster, or opaqueness is expensive to compute. There's a bug on this for Blink's opaqueness tracking for background painting optimization.

enne@, do you have a fix in mind or would you like to punt it over to the Blink compositor side?

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

commit a9f679016c42686e2a688f9e95d7244aa693e45c
Author: Philip Rogers <pdr@chromium.org>
Date: Sat Aug 04 01:47:19 2018

Do not mark directly composited image layers as opaque

Due to https://crbug.com/870857, we cannot mark directly composited
image layers as opaque. https://crrev.com/556288 started setting
prevent_contents_opaque_changes_ for directly-composited image layers
which exposed this bug.

Bug:  868779 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I9fb0fe4361bd8fd39529bbf698f7549edbc51faf
Reviewed-on: https://chromium-review.googlesource.com/1162750
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580725}
[modify] https://crrev.com/a9f679016c42686e2a688f9e95d7244aa693e45c/third_party/blink/renderer/platform/graphics/graphics_layer.cc
[modify] https://crrev.com/a9f679016c42686e2a688f9e95d7244aa693e45c/third_party/blink/renderer/platform/graphics/image_layer_chromium_test.cc

I've attached a smaller testcase for verification. Lets let this bake for a few days and then consider merging into M69.

Deleted: imagebug.html 679 bytes

imagebug.html 679 bytes View Download

The baking is smelling pretty good. Requesting merge onto the M69 plate.

This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This is regressed in M68. How safe and critical is this to merge to M69?

This is pretty safe but not particularly critical. The only issue is a faint line on some images. Maybe we should not merge this after all.

Rejecting merge to M69 based on comment #19. Thank you.