Don't remove empty display item pairs that draw content

Don't remove empty display item pairs that draw content

DisplayItemList::add currently removes empty display item pairs, but
such pairs can sometimes have visible content. In particular, certain
kinds of CSS filters produce visible content even when applied to an
otherwise empty element; this translates into an empty pair of
FilterDisplayItems.

This CL makes DisplayItemList::add only remove empty display item pairs
that don't draw content.

BUG=458601
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190497

[ajuma, 2015-02-19 15:59:04]

ajuma@chromium.org changed reviewers:
+ pdr@chromium.org

[Stephen Chennney, 2015-02-19 16:13:26]

schenney@chromium.org changed reviewers:
+ schenney@chromium.org

[Stephen Chennney, 2015-02-19 16:13:27]

Happy with the direction.

https://codereview.chromium.org/924533006/diff/20001/LayoutTests/css3/filters...
File LayoutTests/css3/filters/empty-element-with-filter-expected.html (right):

https://codereview.chromium.org/924533006/diff/20001/LayoutTests/css3/filters...
LayoutTests/css3/filters/empty-element-with-filter-expected.html:9: /* Make the
div have visible content without the filter. */
This comment confused me, but that's because the term "expected" is not really
describing the test. Really it is "these 2 things should be the same"

I would prefer a test that alone, without the expected result, showed the
right/wrong behavior. That is, two divs in the same document, one with content
and one without, that should look the same. The reference would then have the
same 2 divs, both with content (or you could be tricky and switch the
content/no-content rect in the expected).

https://codereview.chromium.org/924533006/diff/20001/Source/platform/graphics...
File Source/platform/graphics/paint/DrawingDisplayItem.h (right):

https://codereview.chromium.org/924533006/diff/20001/Source/platform/graphics...
Source/platform/graphics/paint/DrawingDisplayItem.h:26: virtual bool
drawsContent() const override { return true; }
This should check the picture contents. approximateOpCount == 0 implies a
strictly empty picture. A picture with non-zero op-count may also draw nothing,
but that's harder to assess so not useful in this case.

[ajuma, 2015-02-19 17:36:51]

https://codereview.chromium.org/924533006/diff/20001/LayoutTests/css3/filters...
File LayoutTests/css3/filters/empty-element-with-filter-expected.html (right):

https://codereview.chromium.org/924533006/diff/20001/LayoutTests/css3/filters...
LayoutTests/css3/filters/empty-element-with-filter-expected.html:9: /* Make the
div have visible content without the filter. */
On 2015/02/19 16:13:27, Stephen Chenney wrote:
> This comment confused me, but that's because the term "expected" is not really
> describing the test. Really it is "these 2 things should be the same"
> 
> I would prefer a test that alone, without the expected result, showed the
> right/wrong behavior. That is, two divs in the same document, one with content
> and one without, that should look the same. The reference would then have the
> same 2 divs, both with content (or you could be tricky and switch the
> content/no-content rect in the expected).

Done.

https://codereview.chromium.org/924533006/diff/20001/Source/platform/graphics...
File Source/platform/graphics/paint/DrawingDisplayItem.h (right):

https://codereview.chromium.org/924533006/diff/20001/Source/platform/graphics...
Source/platform/graphics/paint/DrawingDisplayItem.h:26: virtual bool
drawsContent() const override { return true; }
On 2015/02/19 16:13:27, Stephen Chenney wrote:
> This should check the picture contents. approximateOpCount == 0 implies a
> strictly empty picture. A picture with non-zero op-count may also draw
nothing,
> but that's harder to assess so not useful in this case.

Done.

[Stephen Chennney, 2015-02-19 17:39:07]

Great. LGTM.

[ajuma, 2015-02-19 17:42:18]

The CQ bit was checked by ajuma@chromium.org

[commit-bot: I haz the power, 2015-02-19 17:43:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/924533006/40001

[commit-bot: I haz the power, 2015-02-19 21:09:45]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190497