From cb7ce881d745f379c0b8a00ec65c329df76543f4 Mon Sep 17 00:00:00 2001
From: Zhenyao Mo <zmo@chromium.org>
Date: Fri, 18 Sep 2026 18:09:48 -0700
Subject: [PATCH] gpu: Don't mark attachments cleared when the lazy clear fails

GLES2DecoderImpl::ClearUnclearedAttachments() performs the
security-critical lazy clear of never-written framebuffer attachments
and then unconditionally calls MarkAttachmentsAsCleared(), without
checking whether the driver clear actually executed.

GL_OUT_OF_MEMORY may be generated by any command (OpenGL ES 3.2 section
2.3.1) and leaves the results of that command undefined. Drivers that
defer physical allocation until first use can therefore fail the clear
under VRAM pressure. When that happens the attachment still holds
whatever a previous consumer left in that GPU memory, but it is
bookkept as cleared, so a subsequent ReadPixels() copies that memory
into the client's shared memory. GPU memory pools are shared across
origins, so the disclosed contents can be cross-origin.

The failure is not otherwise observable by the decoder:

 - The default path calls api()->glClearFn() and never peeks, so the
   queued error is left on the driver and is later drained by an
   unrelated glGetError(). ErrorStateImpl::GetGLError() returns the raw
   driver error without routing it through SetGLError(), so
   OnOutOfMemoryError() never fires and the context is not lost.
 - The gl_clear_broken workaround path runs the blit inside a
   ScopedGLErrorSuppressor, whose destructor silently discards
   GL_OUT_OF_MEMORY and GL_CONTEXT_LOST_KHR.
 - Framebuffer::ClearUnclearedIntRenderbufferAttachments() has the same
   problem for integer renderbuffers: it calls SetCleared(true) after
   glClearBuffer{ui,i}v() without checking for an error.

Fix these sites so that "cleared" bookkeeping is only committed once
the clear is known to have landed:

 - Bracket the glClear() with the decoder's established
   COPY_REAL_GL_ERRORS / PEEK_GL_ERROR pattern, already used by
   ClearLevel3D(), ClearCompressedTextureLevel() and
   DoRenderbufferStorage(). Peeking also routes GL_OUT_OF_MEMORY
   through OnOutOfMemoryError() so lose_context_when_out_of_memory_
   applies.
 - Make ClearFramebufferForWorkaround() peek inside the suppressor's
   scope and return whether the blit succeeded. The suppressor is kept
   because the function has two other callers that must not leak blit
   errors into unrelated commands. Surfacing the first error to those
   callers is bounded, because ClearRealGLErrors() already NOTREACHED()s
   on anything other than GL_OUT_OF_MEMORY and GL_CONTEXT_LOST_KHR.
 - Make ClearUnclearedIntRenderbufferAttachments() return the first
   error it saw and only SetCleared() the attachments whose clear
   succeeded. The decoder re-raises that error through the wrapper, so
   the real error code is preserved rather than being reported as a
   blanket GL_OUT_OF_MEMORY. Its result is [[nodiscard]] so the check
   cannot be silently dropped later.
 - Also report failure for integer attachments beyond max_draw_buffers.
   Those cannot be addressed by glClearBuffer*iv at all, so they were
   skipped without being cleared and then marked cleared by the caller.

When any clear fails the function now returns false without marking
anything cleared, which CheckFramebufferValid() already propagates, so
the caller reports GL_INVALID_FRAMEBUFFER_OPERATION and the read is
abandoned. The attachment stays uncleared and a later access retries
the clear.

Pre-existing driver errors are drained with CopyRealGLErrorsToWrapper()
rather than a bare glGetError() loop, so that errors from earlier
commands are still reported to the client and GL_CONTEXT_LOST_KHR still
reaches OnContextLostError().

GL state restoration (draw buffers, clear state, framebuffer rebind and
GL_RASTERIZER_DISCARD) is performed on both exit paths.

Bug: 559727039
Change-Id: Ie101a3ad851949240a901b7f8a875dfabbdf9972
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8412435
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1701289}
---
diff --git a/gpu/command_buffer/service/framebuffer_manager.cc b/gpu/command_buffer/service/framebuffer_manager.cc
index b11de04..d71a0c23 100644
--- a/gpu/command_buffer/service/framebuffer_manager.cc
+++ b/gpu/command_buffer/service/framebuffer_manager.cc
@@ -462,7 +462,7 @@
   return false;
 }
 
-void Framebuffer::ClearUnclearedIntRenderbufferAttachments(
+GLenum Framebuffer::ClearUnclearedIntRenderbufferAttachments(
     RenderbufferManager* renderbuffer_manager) {
   // glClearBuffer*iv(GL_COLOR, i, ...) targets DRAW_BUFFERi, not
   // COLOR_ATTACHMENTi (ES3 4.2.3): when DRAW_BUFFERi == GL_NONE the clear is a
@@ -474,6 +474,7 @@
     buffers[i] = GL_NONE;
   }
   bool need_clear = false;
+  bool has_unaddressable_attachment = false;
   for (auto const& it : attachments_) {
     if (!it.second->IsRenderbufferAttachment() || it.second->cleared() ||
         !GLES2Util::IsIntegerFormat(it.second->internal_format())) {
@@ -481,16 +482,21 @@
     }
     if (it.first < GL_COLOR_ATTACHMENT0 ||
         it.first >= GL_COLOR_ATTACHMENT0 + manager_->max_draw_buffers_) {
+      // Not reachable via glClearBuffer*iv below, so it cannot be cleared
+      // here. Report failure so the caller does not mark it cleared.
+      has_unaddressable_attachment = true;
       continue;
     }
     buffers[it.first - GL_COLOR_ATTACHMENT0] = it.first;
     need_clear = true;
   }
   if (!need_clear) {
-    return;
+    return has_unaddressable_attachment ? GL_INVALID_OPERATION : GL_NO_ERROR;
   }
   glDrawBuffersARB(manager_->max_draw_buffers_, buffers.data());
 
+  GLenum result =
+      has_unaddressable_attachment ? GL_INVALID_OPERATION : GL_NO_ERROR;
   for (AttachmentMap::const_iterator it = attachments_.begin();
        it != attachments_.end(); ++it) {
     if (!it->second->IsRenderbufferAttachment() || it->second->cleared())
@@ -501,7 +507,8 @@
       DCHECK_LE(static_cast<GLenum>(GL_COLOR_ATTACHMENT0), attaching_point);
       if (attaching_point >=
           GL_COLOR_ATTACHMENT0 + manager_->max_draw_buffers_) {
-        // Can't be addressed via glClearBuffer*iv; leave it marked uncleared.
+        // Can't be addressed via glClearBuffer*iv; already accounted for by
+        // |has_unaddressable_attachment| above, so it stays uncleared.
         continue;
       }
       GLint drawbuffer = it->first - GL_COLOR_ATTACHMENT0;
@@ -513,11 +520,23 @@
         const static GLint kZero[] = { 0, 0, 0, 0 };
         glClearBufferiv(GL_COLOR, drawbuffer, kZero);
       }
+      // GL_OUT_OF_MEMORY can be raised by any command (ES 3.2 section 2.3.1),
+      // and a clear that failed leaves the renderbuffer holding whatever was
+      // previously in that GPU memory - e.g. drivers that defer physical
+      // allocation until first use can fail here under VRAM pressure. Only
+      // record the attachment as cleared when the clear actually succeeded,
+      // otherwise a later read would hand that memory to the client.
+      GLenum error = glGetError();
+      if (error != GL_NO_ERROR) {
+        result = error;
+        continue;
+      }
       it->second->SetCleared(renderbuffer_manager, nullptr, true);
     }
   }
 
   RestoreDrawBuffers();
+  return result;
 }
 
 bool Framebuffer::HasSRGBAttachments() const {
@@ -548,11 +567,12 @@
         it.first < GL_COLOR_ATTACHMENT0 + manager_->max_draw_buffers_ &&
         !it.second->cleared()) {
       // There should be no partially cleared images, uncleared int/3d images.
-      // This is because ClearUnclearedIntOr3DImagesOrPartiallyClearedImages()
-      // is called before this.
-      DCHECK(!GLES2Util::IsIntegerFormat(it.second->internal_format()));
-      DCHECK(!it.second->IsPartiallyCleared());
-      DCHECK(!it.second->Is3D());
+      // This is because
+      // ClearUnclearedIntOr3DTexturesOrPartiallyClearedTextures() and
+      // ClearUnclearedIntRenderbufferAttachments() are called before this.
+      CHECK(!GLES2Util::IsIntegerFormat(it.second->internal_format()));
+      CHECK(!it.second->IsPartiallyCleared());
+      CHECK(!it.second->Is3D());
       buffers[it.first - GL_COLOR_ATTACHMENT0] = it.first;
     }
   }
diff --git a/gpu/command_buffer/service/framebuffer_manager.h b/gpu/command_buffer/service/framebuffer_manager.h
index 75091e9..e909d945 100644
--- a/gpu/command_buffer/service/framebuffer_manager.h
+++ b/gpu/command_buffer/service/framebuffer_manager.h
@@ -103,8 +103,15 @@
 
   bool HasUnclearedIntRenderbufferAttachments() const;
 
-  void ClearUnclearedIntRenderbufferAttachments(
-    RenderbufferManager* renderbuffer_manager);
+  // Clears uncleared integer renderbuffer attachments and marks the ones that
+  // were successfully cleared as cleared. Returns GL_NO_ERROR if every such
+  // attachment is now cleared, otherwise the GL error that prevented it. A
+  // caller must not mark attachments as cleared when this fails, since a
+  // failed clear leaves the renderbuffer holding uninitialized GPU memory.
+  // The caller must drain the real GL error queue first, so that the errors
+  // observed here can be attributed to the clears issued by this function.
+  [[nodiscard]] GLenum ClearUnclearedIntRenderbufferAttachments(
+      RenderbufferManager* renderbuffer_manager);
 
   void MarkAttachmentAsCleared(
     RenderbufferManager* renderbuffer_manager,
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index bd7ee010..cc02dea9 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -2283,7 +2283,10 @@
   const SamplerState& GetSamplerStateForTextureUnit(GLenum target, GLuint unit);
 
   // Helper method to call glClear workaround.
-  void ClearFramebufferForWorkaround(GLbitfield mask);
+  // Clears the framebuffer via a blit on drivers where glClear is broken.
+  // Returns false if the blit failed, in which case the targeted attachments
+  // must not be recorded as cleared.
+  bool ClearFramebufferForWorkaround(GLbitfield mask);
 
   bool SupportsSeparateFramebufferBinds() const {
     return (feature_info_->feature_flags().chromium_framebuffer_multisample ||
@@ -7370,6 +7373,11 @@
     return false;
   }
 
+  // Set when a clear issued below failed. A failed clear leaves the attachment
+  // holding whatever was previously in that GPU memory, so the "cleared"
+  // bookkeeping must not be committed for it.
+  bool clear_failed = false;
+
   bool cleared_int_renderbuffers = false;
   Framebuffer* draw_framebuffer = GetBoundDrawFramebuffer();
   if (framebuffer->HasUnclearedIntRenderbufferAttachments()) {
@@ -7382,98 +7390,131 @@
     state_.SetDeviceCapabilityState(GL_SCISSOR_TEST, false);
     ClearDeviceWindowRectangles();
 
-    // TODO(zmo): Assume DrawBuffers() does not affect ClearBuffer().
-    framebuffer->ClearUnclearedIntRenderbufferAttachments(
-        renderbuffer_manager());
+    // Drain pre-existing driver errors into the wrapper so they are still
+    // reported to the client, and so that any error observed by the clears
+    // below is attributable to those clears.
+    LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("ClearUnclearedAttachments");
 
+    // TODO(zmo): Assume DrawBuffers() does not affect ClearBuffer().
+    GLenum int_clear_error =
+        framebuffer->ClearUnclearedIntRenderbufferAttachments(
+            renderbuffer_manager());
+    if (int_clear_error != GL_NO_ERROR) {
+      // Report the failure through the wrapper so that GL_OUT_OF_MEMORY runs
+      // OnOutOfMemoryError() and lose_context_when_out_of_memory_ applies.
+      LOCAL_SET_GL_ERROR(int_clear_error, "ClearUnclearedAttachments",
+                         "failed to clear integer renderbuffer attachment");
+      clear_failed = true;
+    }
+
+    // Set even when the clear failed: this tracks that the block above
+    // dirtied GL state (draw framebuffer binding, color mask, scissor test,
+    // window rectangles), not that the attachments ended up cleared. It is
+    // what drives the state restoration below, which must run on both the
+    // success and the failure path.
     cleared_int_renderbuffers = true;
   }
 
   GLbitfield clear_bits = 0;
   bool reset_draw_buffers = false;
   bool rebound_draw_for_clear = cleared_int_renderbuffers;
-  if (framebuffer->HasUnclearedColorAttachments()) {
-    // We should always use alpha == 0 here, because 1) some draw buffers may
-    // have alpha and some may not; 2) we won't have the same situation as the
-    // back buffer where alpha channel exists but is not requested.
-    api()->glClearColorFn(0.0f, 0.0f, 0.0f, 0.0f);
-    state_.SetDeviceColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
-    clear_bits |= GL_COLOR_BUFFER_BIT;
+  if (!clear_failed) {
+    if (framebuffer->HasUnclearedColorAttachments()) {
+      // We should always use alpha == 0 here, because 1) some draw buffers may
+      // have alpha and some may not; 2) we won't have the same situation as the
+      // back buffer where alpha channel exists but is not requested.
+      api()->glClearColorFn(0.0f, 0.0f, 0.0f, 0.0f);
+      state_.SetDeviceColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
+      clear_bits |= GL_COLOR_BUFFER_BIT;
 
-    if (SupportsDrawBuffers()) {
-      // Ensure |framebuffer| is bound as DRAW before preparing draw buffers.
-      // Otherwise glDrawBuffersARB mutates the wrong FBO's state, causing
-      // the glClear to skip clearing uncleared attachments.
+      if (SupportsDrawBuffers()) {
+        // Ensure |framebuffer| is bound as DRAW before preparing draw buffers.
+        // Otherwise glDrawBuffersARB mutates the wrong FBO's state, causing
+        // the glClear to skip clearing uncleared attachments.
+        if (!rebound_draw_for_clear && target == GL_READ_FRAMEBUFFER &&
+            draw_framebuffer != framebuffer) {
+          BindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffer->service_id());
+          rebound_draw_for_clear = true;
+        }
+        reset_draw_buffers =
+            framebuffer
+                ->PrepareDrawBuffersForClearingUninitializedAttachments();
+      }
+    }
+
+    const Framebuffer::Attachment* depth_attachment =
+        framebuffer->GetAttachment(GL_DEPTH_ATTACHMENT);
+    const Framebuffer::Attachment* stencil_attachment =
+        framebuffer->GetAttachment(GL_STENCIL_ATTACHMENT);
+    bool clear_depth = depth_attachment && !depth_attachment->cleared();
+    bool clear_stencil = stencil_attachment && !stencil_attachment->cleared();
+
+    // A packed depth-stencil image attached at only one of the depth/stencil
+    // points must be bound and cleared at both points so that both components
+    // are initialized before the image is marked as cleared.
+    GLenum filled_depth_stencil_point = 0;
+    if (clear_depth && !stencil_attachment &&
+        (GLES2Util::GetChannelsForFormat(depth_attachment->internal_format()) &
+         GLES2Util::kStencil) != 0) {
+      filled_depth_stencil_point = GL_STENCIL_ATTACHMENT;
+      Framebuffer::BindAttachmentToPoint(target, GL_STENCIL_ATTACHMENT,
+                                         depth_attachment);
+      clear_stencil = true;
+    } else if (clear_stencil && !depth_attachment &&
+               (GLES2Util::GetChannelsForFormat(
+                    stencil_attachment->internal_format()) &
+                GLES2Util::kDepth) != 0) {
+      filled_depth_stencil_point = GL_DEPTH_ATTACHMENT;
+      Framebuffer::BindAttachmentToPoint(target, GL_DEPTH_ATTACHMENT,
+                                         stencil_attachment);
+      clear_depth = true;
+    }
+
+    if (clear_stencil) {
+      api()->glClearStencilFn(0);
+      state_.SetDeviceStencilMaskSeparate(GL_FRONT, kDefaultStencilMask);
+      state_.SetDeviceStencilMaskSeparate(GL_BACK, kDefaultStencilMask);
+      clear_bits |= GL_STENCIL_BUFFER_BIT;
+    }
+
+    if (clear_depth) {
+      api()->glClearDepthFn(1.0f);
+      state_.SetDeviceDepthMask(GL_TRUE);
+      clear_bits |= GL_DEPTH_BUFFER_BIT;
+    }
+
+    if (clear_bits) {
       if (!rebound_draw_for_clear && target == GL_READ_FRAMEBUFFER &&
           draw_framebuffer != framebuffer) {
+        // TODO(zmo): There is no guarantee that an FBO that is complete on the
+        // READ attachment will be complete as a DRAW attachment.
         BindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffer->service_id());
-        rebound_draw_for_clear = true;
       }
-      reset_draw_buffers =
-          framebuffer->PrepareDrawBuffersForClearingUninitializedAttachments();
+      state_.SetDeviceCapabilityState(GL_SCISSOR_TEST, false);
+      ClearDeviceWindowRectangles();
+      if (workarounds().gl_clear_broken) {
+        clear_failed = !ClearFramebufferForWorkaround(clear_bits);
+      } else {
+        // Drain any pre-existing driver errors so the check below only reflects
+        // errors generated by this clear.
+        LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("ClearUnclearedAttachments");
+        api()->glClearFn(clear_bits);
+        // GL_OUT_OF_MEMORY can be raised by any command (ES 3.2 section 2.3.1),
+        // and leaves the results of that command undefined. A clear that failed
+        // this way leaves the attachment holding whatever was previously in
+        // that GPU memory - drivers that defer physical allocation until first
+        // use can fail here under VRAM pressure. Routing the error through
+        // PeekGLError also makes OnOutOfMemoryError() fire so
+        // lose_context_when_out_of_memory_ applies.
+        clear_failed =
+            LOCAL_PEEK_GL_ERROR("ClearUnclearedAttachments") != GL_NO_ERROR;
+      }
     }
-  }
 
-  const Framebuffer::Attachment* depth_attachment =
-      framebuffer->GetAttachment(GL_DEPTH_ATTACHMENT);
-  const Framebuffer::Attachment* stencil_attachment =
-      framebuffer->GetAttachment(GL_STENCIL_ATTACHMENT);
-  bool clear_depth = depth_attachment && !depth_attachment->cleared();
-  bool clear_stencil = stencil_attachment && !stencil_attachment->cleared();
-
-  // A packed depth-stencil image attached at only one of the depth/stencil
-  // points must be bound and cleared at both points so that both components
-  // are initialized before the image is marked as cleared.
-  GLenum filled_depth_stencil_point = 0;
-  if (clear_depth && !stencil_attachment &&
-      (GLES2Util::GetChannelsForFormat(depth_attachment->internal_format()) &
-       GLES2Util::kStencil) != 0) {
-    filled_depth_stencil_point = GL_STENCIL_ATTACHMENT;
-    Framebuffer::BindAttachmentToPoint(target, GL_STENCIL_ATTACHMENT,
-                                       depth_attachment);
-    clear_stencil = true;
-  } else if (clear_stencil && !depth_attachment &&
-             (GLES2Util::GetChannelsForFormat(
-                  stencil_attachment->internal_format()) &
-              GLES2Util::kDepth) != 0) {
-    filled_depth_stencil_point = GL_DEPTH_ATTACHMENT;
-    Framebuffer::BindAttachmentToPoint(target, GL_DEPTH_ATTACHMENT,
-                                       stencil_attachment);
-    clear_depth = true;
-  }
-
-  if (clear_stencil) {
-    api()->glClearStencilFn(0);
-    state_.SetDeviceStencilMaskSeparate(GL_FRONT, kDefaultStencilMask);
-    state_.SetDeviceStencilMaskSeparate(GL_BACK, kDefaultStencilMask);
... (file diff truncated)

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
index 381bbcd5..016c31e 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
@@ -867,12 +867,13 @@
     GLint restore_scissor_x,
     GLint restore_scissor_y,
     GLsizei restore_scissor_width,
-    GLsizei restore_scissor_height) {
+    GLsizei restore_scissor_height,
+    GLenum clear_error) {
   SetupExpectationsForFramebufferClearingMulti(
       0, 0, target, clear_bits, restore_red, restore_green, restore_blue,
       restore_alpha, restore_stencil, restore_depth, restore_scissor_test,
       restore_scissor_x, restore_scissor_y, restore_scissor_width,
-      restore_scissor_height);
+      restore_scissor_height, clear_error);
 }
 
 void GLES2DecoderTestBase::SetupExpectationsForRestoreClearState(
@@ -924,7 +925,8 @@
     GLint restore_scissor_x,
     GLint restore_scissor_y,
     GLsizei restore_scissor_width,
-    GLsizei restore_scissor_height) {
+    GLsizei restore_scissor_height,
+    GLenum clear_error) {
   // TODO(gman): Figure out why InSequence stopped working.
   // InSequence sequence;
   EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(target))
@@ -961,6 +963,13 @@
         .Times(1)
         .RetiresOnSaturation();
   }
+  // ClearUnclearedAttachments() drains pre-existing driver errors before the
+  // clear and peeks afterwards, so that a clear that failed is not committed
+  // as "cleared".
+  EXPECT_CALL(*gl_, GetError())
+      .WillOnce(Return(GL_NO_ERROR))
+      .WillOnce(Return(clear_error))
+      .RetiresOnSaturation();
   EXPECT_CALL(*gl_, Clear(clear_bits))
       .Times(1)
       .RetiresOnSaturation();
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
index d7270d35..541d34d 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
@@ -467,19 +467,25 @@
                                              GLsizei restore_scissor_width,
                                              GLsizei restore_scissor_height);
 
-  void SetupExpectationsForFramebufferClearing(GLenum target,
-                                               GLuint clear_bits,
-                                               GLclampf restore_red,
-                                               GLclampf restore_green,
-                                               GLclampf restore_blue,
-                                               GLclampf restore_alpha,
-                                               GLuint restore_stencil,
-                                               GLclampf restore_depth,
-                                               bool restore_scissor_test,
-                                               GLint restore_scissor_x,
-                                               GLint restore_scissor_y,
-                                               GLsizei restore_scissor_width,
-                                               GLsizei restore_scissor_height);
+  // |clear_error| is the error reported by the post-clear glGetError() that
+  // GLES2DecoderImpl::ClearUnclearedAttachments uses to decide whether the
+  // clear landed. Pass something other than GL_NO_ERROR to simulate a driver
+  // that failed the clear (e.g. GL_OUT_OF_MEMORY).
+  void SetupExpectationsForFramebufferClearing(
+      GLenum target,
+      GLuint clear_bits,
+      GLclampf restore_red,
+      GLclampf restore_green,
+      GLclampf restore_blue,
+      GLclampf restore_alpha,
+      GLuint restore_stencil,
+      GLclampf restore_depth,
+      bool restore_scissor_test,
+      GLint restore_scissor_x,
+      GLint restore_scissor_y,
+      GLsizei restore_scissor_width,
+      GLsizei restore_scissor_height,
+      GLenum clear_error = GL_NO_ERROR);
 
   void SetupExpectationsForFramebufferClearingMulti(
       GLuint read_framebuffer_service_id,
@@ -496,7 +502,8 @@
       GLint restore_scissor_x,
       GLint restore_scissor_y,
       GLsizei restore_scissor_width,
-      GLsizei restore_scissor_height);
+      GLsizei restore_scissor_height,
+      GLenum clear_error = GL_NO_ERROR);
 
   void SetupExpectationsForDepthMask(bool mask);
   void SetupExpectationsForEnableDisable(GLenum cap, bool enable);
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc
index a8fee822..c2e18f4 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc
@@ -2498,6 +2498,110 @@
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
 
+// Regression test for crbug.com/559727039: if the lazy clear of a never-written
+// attachment fails (GL_OUT_OF_MEMORY is permitted for any command by ES 3.2
+// section 2.3.1, and leaves the attachment holding a previous consumer's GPU
+// memory), the attachment must not be recorded as cleared and the read must not
+// return that memory to the client.
+TEST_P(GLES2DecoderWithShaderTest,
+       UnClearedAttachmentsNotMarkedClearedWhenClearFailsOnReadPixels) {
+  const GLuint kFBOClientTextureId = 4100;
+  const GLuint kFBOServiceTextureId = 4101;
+
+  // Register a texture id.
+  EXPECT_CALL(*gl_, GenTextures(_, _))
+      .WillOnce(SetArgPointee<1>(kFBOServiceTextureId))
+      .RetiresOnSaturation();
+  GenHelper<cmds::GenTexturesImmediate>(kFBOClientTextureId);
+
+  // Setup "render to" texture.
+  DoBindTexture(GL_TEXTURE_2D, kFBOClientTextureId, kFBOServiceTextureId);
+  DoTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, 0,
+               0);
+  DoBindFramebuffer(GL_FRAMEBUFFER, client_framebuffer_id_,
+                    kServiceFramebufferId);
+  DoFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
+                         kFBOClientTextureId, kFBOServiceTextureId, 0,
+                         GL_NO_ERROR);
+  DoEnableDisable(GL_SCISSOR_TEST, false);
+  DoScissor(0, 0, 1, 1);
+
+  // Setup "render from" texture.
+  SetupTexture();
+
+  FramebufferManager* framebuffer_manager = GetFramebufferManager();
+  Framebuffer* framebuffer =
+      framebuffer_manager->GetFramebuffer(client_framebuffer_id_);
+  ASSERT_TRUE(framebuffer != nullptr);
+  EXPECT_FALSE(framebuffer->IsCleared());
+
+  // The driver fails the lazy clear with GL_OUT_OF_MEMORY.
+  SetupExpectationsForFramebufferClearing(GL_FRAMEBUFFER,       // target
+                                          GL_COLOR_BUFFER_BIT,  // clear bits
+                                          0, 0, 0,
+                                          0,      // color
+                                          0,      // stencil
+                                          1.0f,   // depth
+                                          false,  // scissor test
+                                          0, 0, 1, 1,
+                                          GL_OUT_OF_MEMORY);  // clear error
+
+  // The read must be abandoned rather than copying the uncleared attachment
+  // into the client's shared memory.
+  EXPECT_CALL(*gl_, ReadPixels(_, _, _, _, _, _, _)).Times(0);
+
+  auto* result = GetSharedMemoryAs<cmds::ReadPixels::Result*>();
+  uint32_t result_shm_id = shared_memory_id_;
+  uint32_t result_shm_offset = kSharedMemoryOffset;
+  uint32_t pixels_shm_id = shared_memory_id_;
+  uint32_t pixels_shm_offset = kSharedMemoryOffset + sizeof(*result);
+  cmds::ReadPixels cmd;
+  cmd.Init(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixels_shm_id,
+           pixels_shm_offset, result_shm_id, result_shm_offset, false);
+  result->success = 0;
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+
+  // The out of memory must be surfaced rather than swallowed, and the
+  // attachment must still be uncleared so a later access retries the clear.
+  // Note GetGLError() reuses the shared memory that |result| points at.
+  EXPECT_EQ(GL_OUT_OF_MEMORY, GetGLError());
+  EXPECT_FALSE(framebuffer->IsCleared());
+
+  // A subsequent read retries the clear; when it succeeds the attachment is
+  // marked cleared and the read proceeds normally. These expectations mirror
+  // SetupExpectationsForFramebufferClearing() minus the completeness query,
+  // which is not repeated because the first read already cached the result.
+  EXPECT_CALL(*gl_, ClearColor(0.0f, 0.0f, 0.0f, 0.0f))
+      .Times(1)
+      .RetiresOnSaturation();
+  SetupExpectationsForColorMask(true, true, true, true);
+  SetupExpectationsForEnableDisable(GL_SCISSOR_TEST, false);
+  if (feature_info()->feature_flags().ext_window_rectangles) {
+    EXPECT_CALL(*gl_, WindowRectanglesEXT(GL_EXCLUSIVE_EXT, 0, nullptr))
+        .Times(1)
+        .RetiresOnSaturation();
+  }
+  EXPECT_CALL(*gl_, GetError())
+      .WillOnce(Return(GL_NO_ERROR))
+      .WillOnce(Return(GL_NO_ERROR))
+      .RetiresOnSaturation();
+  EXPECT_CALL(*gl_, Clear(GL_COLOR_BUFFER_BIT)).Times(1).RetiresOnSaturation();
+  SetupExpectationsForRestoreClearState(0, 0, 0, 0, 0, 1.0f, false, 0, 0, 1, 1);
+
+  EXPECT_CALL(*gl_, GetError())
+      .WillOnce(Return(GL_NO_ERROR))
+      .WillOnce(Return(GL_NO_ERROR))
+      .RetiresOnSaturation();
+  EXPECT_CALL(*gl_, ReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, _))
+      .Times(1)
+      .RetiresOnSaturation();
+  result = GetSharedMemoryAs<cmds::ReadPixels::Result*>();
+  result->success = 0;
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+  EXPECT_TRUE(framebuffer->IsCleared());
+}
+
 TEST_P(GLES3DecoderTest, CopyTexImage2DValidInternalFormat) {
   const GLuint kFBOClientTextureId = 4100;
   const GLuint kFBOServiceTextureId = 4101;