Where is the best place to add the test? gl_renderer_unittests.cc?

Maybe some GLRendererTest.
1. Create a render_passes_in_draw_order_.
2. Call renderer.DecideRenderPassAllocationsForFrame.
3. DrawFrame.
4. Set some render_passes has_damage_from_contributing_content to false.
5. Delete some of the texture in render_pass_textures_ (need to add test api).
6. Call renderer.DecideRenderPassAllocationsForFrame again.
7. The render_passes in step 4 should have the damage set to true.

> 5. Delete some of the texture in render_pass_textures_ (need to add test api).

This should be reproducable with just the public api, or else it's not possible in production?

Textures are deleted when a frame is drawn without that renderpass id in them. Maybe you need to just do that?

> Where is the best place to add the test? gl_renderer_unittests.cc?

That sounds good, and could copy the test into software/skia_renderer_unittests.cc too? (sorry)

Uploaded a cl: https://chromium-review.googlesource.com/c/chromium/src/+/757027

There is no skia_renderer_unittests yet.

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

commit 43d70d5dd490d5bf146cbb35c79a53edcbdf3a25
Author: danakj <danakj@chromium.org>
Date: Thu Nov 09 01:12:34 2017

When calling UseRenderPass() to restore, ensure it runs.

UseRenderPass() has two use cases, either to set up a render pass
as the framebuffer for drawing at the start of the draw, or to restore
us back to that state after making some intermediate change to which
framebuffer is bound.

UseRenderPass() has some optimizations where it can return false and
not do anything, if it detects that we don't want to draw the render
pass. The first use case, in DrawRenderPass() makes use of this return
value to avoid drawing anything.

Other callers to UseRenderPass() do not check the return value because
we're already drawing the pass and they are trying to restore state. If
it early outs for these it would be unexpected and produce bugs.

Currently DecideRenderPassAllocationsForFrame modifies render passes to
try avoid such bugs, but there are other ways to encounter the bug.

This changes UseRenderPass() to never early out so the latter cases will
not avoid resetting the framebuffer. The former case can avoid
UseRenderPass() entirely by checking CanSkipRenderPass(). If it is true,
then it does not need to be drawn.

Also, we will now ensure that UseRenderPass() is called after using a
ScopedUseGrContext when drawing a RenderPassDrawQuad. Currently on errors
it would early out before restoring the framebuffer, which could cause
bugs for the next quads in the RenderPass. This was introduced when the
UseRenderPass() call was moved out of the ~ScopedUseGrContext()
destructor in https://codereview.chromium.org/2203033005.

This all came up via https://chromium-review.googlesource.com/c/chromium/src/+/757027#message-b6e905925e834419ff448e2093a392053d5d321a

R=enne@chromium.org, wutao@chromium.org

Bug:  782044 ,  738190 , 782042
Change-Id: Ib4662c5fee3f326642add12bbfe3d51e51dff1c9
Reviewed-on: https://chromium-review.googlesource.com/757049
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515043}
[modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/direct_renderer.cc
[modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/direct_renderer.h
[modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/gl_renderer.h
[modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/skia_renderer.cc
[modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/skia_renderer.h
[modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/software_renderer.cc
[modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/software_renderer.h

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

commit 6dcd9ac14c9e5fbb60c659143b40bcc54309486e
Author: Ned Nguyen <nednguyen@google.com>
Date: Thu Nov 09 16:08:22 2017

Revert "When calling UseRenderPass() to restore, ensure it runs."

This reverts commit 43d70d5dd490d5bf146cbb35c79a53edcbdf3a25.

Reason for revert: cause system_health.memory_desktop.browse:media:youtube to crash

(BUG:783198)

Original change's description:
> When calling UseRenderPass() to restore, ensure it runs.
> 
> UseRenderPass() has two use cases, either to set up a render pass
> as the framebuffer for drawing at the start of the draw, or to restore
> us back to that state after making some intermediate change to which
> framebuffer is bound.
> 
> UseRenderPass() has some optimizations where it can return false and
> not do anything, if it detects that we don't want to draw the render
> pass. The first use case, in DrawRenderPass() makes use of this return
> value to avoid drawing anything.
> 
> Other callers to UseRenderPass() do not check the return value because
> we're already drawing the pass and they are trying to restore state. If
> it early outs for these it would be unexpected and produce bugs.
> 
> Currently DecideRenderPassAllocationsForFrame modifies render passes to
> try avoid such bugs, but there are other ways to encounter the bug.
> 
> This changes UseRenderPass() to never early out so the latter cases will
> not avoid resetting the framebuffer. The former case can avoid
> UseRenderPass() entirely by checking CanSkipRenderPass(). If it is true,
> then it does not need to be drawn.
> 
> Also, we will now ensure that UseRenderPass() is called after using a
> ScopedUseGrContext when drawing a RenderPassDrawQuad. Currently on errors
> it would early out before restoring the framebuffer, which could cause
> bugs for the next quads in the RenderPass. This was introduced when the
> UseRenderPass() call was moved out of the ~ScopedUseGrContext()
> destructor in https://codereview.chromium.org/2203033005.
> 
> This all came up via https://chromium-review.googlesource.com/c/chromium/src/+/757027#message-b6e905925e834419ff448e2093a392053d5d321a
> 
> R=​enne@chromium.org, wutao@chromium.org
> 
> Bug:  782044 ,  738190 , 782042
> Change-Id: Ib4662c5fee3f326642add12bbfe3d51e51dff1c9
> Reviewed-on: https://chromium-review.googlesource.com/757049
> Commit-Queue: danakj <danakj@chromium.org>
> Reviewed-by: enne <enne@chromium.org>
> Reviewed-by: Tao Wu <wutao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#515043}

TBR=danakj@chromium.org,enne@chromium.org,wutao@chromium.org

Change-Id: I2acd86c21290c053b1801732599c1ab355161469
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  782044 ,  738190 , 782042
Reviewed-on: https://chromium-review.googlesource.com/760557
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#515178}
[modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/direct_renderer.cc
[modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/direct_renderer.h
[modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/gl_renderer.h
[modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/skia_renderer.cc
[modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/skia_renderer.h
[modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/software_renderer.cc
[modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/software_renderer.h

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

commit 986d87a5fada7910886f1fc07e592c7698da128f
Author: danakj <danakj@chromium.org>
Date: Fri Nov 10 19:30:54 2017

When calling UseRenderPass() to restore, ensure it runs.

UseRenderPass() has two use cases, either to set up a render pass
as the framebuffer for drawing at the start of the draw, or to restore
us back to that state after making some intermediate change to which
framebuffer is bound.

UseRenderPass() has some optimizations where it can return false and
not do anything, if it detects that we don't want to draw the render
pass. The first use case, in DrawRenderPass() makes use of this return
value to avoid drawing anything.

Other callers to UseRenderPass() do not check the return value because
we're already drawing the pass and they are trying to restore state. If
it early outs for these it would be unexpected and produce bugs.

Currently DecideRenderPassAllocationsForFrame modifies render passes to
try avoid such bugs, but there are other ways to encounter the bug.

This changes UseRenderPass() to never early out so the latter cases will
not avoid resetting the framebuffer. The former case can avoid
UseRenderPass() entirely by checking CanSkipRenderPass(). If it is true,
then it does not need to be drawn.

Also, we will now ensure that UseRenderPass() is called after using a
ScopedUseGrContext when drawing a RenderPassDrawQuad. Currently on errors
it would early out before restoring the framebuffer, which could cause
bugs for the next quads in the RenderPass. This was introduced when the
UseRenderPass() call was moved out of the ~ScopedUseGrContext()
destructor in https://codereview.chromium.org/2203033005.

When skipping a RenderPass, current_render_pass is no longer set, which
causes a crash in code that assumes it is, incorrectly. So this updates
some paths in RenderPassDrawQuad drawing to avoid crashing, by passing
the colorspace directly instead of trying to pull it off
current_render_pass, since the caller knows where to find the correct
one, and by using the root_render_pass instead for copying
RenderPassDrawQuads to textures, since they draw into the root.

This all came up via https://chromium-review.googlesource.com/c/chromium/src/+/757027#message-b6e905925e834419ff448e2093a392053d5d321a

R=enne@chromium.org, wutao@chromium.org

Bug:  782044 ,  738190 , 782042,  783087 
Change-Id: I62ea68be6b3008ec8b8370466db0e26321f022f9
Reviewed-on: https://chromium-review.googlesource.com/760558
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515633}
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/direct_renderer.cc
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/direct_renderer.h
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/gl_renderer.h
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/gl_renderer_unittest.cc
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/renderer_pixeltest.cc
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/skia_renderer.cc
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/skia_renderer.h
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/software_renderer.cc
[modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/software_renderer.h

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

commit bd829cadf22696834edf4aefc9033f5c8e272183
Author: danakj <danakj@chromium.org>
Date: Sat Nov 11 00:19:32 2017

Remove caching avoidance when there is a non-empty scissor.

See https://chromium-review.googlesource.com/c/chromium/src/+/760558#message-83efb689457770dfabb6d7371847f06f8091dfd7
and https://bugs.chromium.org/p/chromium/issues/detail?id=783087#c24
for explanations, but tl;dr this check would break caching, and
shouldn't be there. Before it was used to skip render passes
regardless of caching, but incorrectly, and the unittest in
https://chromium-review.googlesource.com/c/chromium/src/+/760558
demonstrates that, and fails with that check.

This removes the check from the caching block.

R=enne@chromium.org, wutao@chromium.org

Bug:  782044 ,  738190 , 782042,  783087 
Change-Id: Idc5baea6da5e6d663da9c1aa23ce81a55e61168d
Reviewed-on: https://chromium-review.googlesource.com/763710
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515757}
[modify] https://crrev.com/bd829cadf22696834edf4aefc9033f5c8e272183/components/viz/service/display/direct_renderer.cc
[modify] https://crrev.com/bd829cadf22696834edf4aefc9033f5c8e272183/components/viz/service/display/gl_renderer_unittest.cc