Fix display rendering corruption when executing scaled CopyOutputRequests.

GLRendererCopier modifies the GL state, making it inconsistent with that
known to GLRenderer. This change adds a RestoreGLState() call in
GLRenderer to restore consistency.

Bug: 792734
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ic4d08997fcba38a5edcd31ae8509b65ad2e2d6a8
Reviewed-on: https://chromium-review.googlesource.com/812957
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523993}

components/viz/service/display/copy_output_scaling_pixeltest.cc

@@ -108,20 +108,47 @@ class CopyOutputScalingPixelTest
     std::unique_ptr<CopyOutputResult> result;
     {
       base::RunLoop loop;
-      std::unique_ptr<CopyOutputRequest> request(new CopyOutputRequest(
+
+      // Add a dummy copy request to be executed when the RED RenderPass is
+      // drawn (before the root RenderPass). This is a regression test to
+      // confirm GLRenderer state is consistent with the GL context after each
+      // copy request executes, and before the next RenderPass is drawn.
+      // http://crbug.com/792734
+      bool dummy_ran = false;
+      auto request = std::make_unique<CopyOutputRequest>(
+          result_format_,
+          base::BindOnce(
+              [](bool* dummy_ran, std::unique_ptr<CopyOutputResult> result) {
+                EXPECT_TRUE(!result->IsEmpty());
+                EXPECT_FALSE(*dummy_ran);
+                *dummy_ran = true;
+              },
+              &dummy_ran));
+      // Set a 10X zoom, which should be more than sufficient to disturb the
+      // results of the main copy request (below) if the GL state is not
+      // properly restored.
+      request->SetUniformScaleRatio(1, 10);
+      list.front()->copy_requests.push_back(std::move(request));
+
+      // Add a copy request to the root RenderPass, to capture the results of
+      // drawing all passes for this frame.
+      request = std::make_unique<CopyOutputRequest>(
           result_format_,
           base::BindOnce(
-              [](std::unique_ptr<CopyOutputResult>* test_result,
+              [](bool* dummy_ran,
+                 std::unique_ptr<CopyOutputResult>* test_result,
                  const base::Closure& quit_closure,
                  std::unique_ptr<CopyOutputResult> result_from_renderer) {
+                EXPECT_TRUE(*dummy_ran);
                 *test_result = std::move(result_from_renderer);
                 quit_closure.Run();
               },
-              &result, loop.QuitClosure())));
+              &dummy_ran, &result, loop.QuitClosure()));
       request->set_result_selection(
           copy_output::ComputeResultRect(copy_rect, scale_from_, scale_to_));
       request->SetScaleRatio(scale_from_, scale_to_);
       list.back()->copy_requests.push_back(std::move(request));
+
       renderer()->DrawFrame(&list, 1.0f, viewport_size);
       loop.Run();
     }

components/viz/service/display/gl_renderer.cc

@@ -2686,6 +2686,10 @@ void GLRenderer::CopyDrawnRenderPass(
       GetFramebufferCopyTextureFormat(), framebuffer_texture,
       framebuffer_texture_size,
       current_frame()->current_render_pass->color_space);
+
+  // The copier modified texture/framebuffer bindings, shader programs, and
+  // other GL state; and so this must be restored before continuing.
+  RestoreGLState();
 }
 
 void GLRenderer::ToGLMatrix(float* gl_matrix, const gfx::Transform& transform) {

components/viz/service/display/gl_renderer_copier.h

@@ -67,9 +67,10 @@ class VIZ_SERVICE_EXPORT GLRendererCopier {
   // copy of the framebuffer. |color_space| specifies the color space of the
   // pixels in the framebuffer.
   //
-  // This implementation may change texture and framebuffer bindings, and so the
-  // caller must not make any assumptions about the original objects still being
-  // bound to the same units.
+  // This implementation may change a wide variety of GL state, such as texture
+  // and framebuffer bindings, shader programs, and related attributes; and so
+  // the caller must not make any assumptions about the state of the GL context
+  // after this call.
   void CopyFromTextureOrFramebuffer(std::unique_ptr<CopyOutputRequest> request,
                                     const gfx::Rect& output_rect,
                                     GLenum internal_format,