Aura: Browser-side changes for Composite-To-Mailbox

cc/gl_renderer.cc

Index: cc/gl_renderer.cc
diff --git a/cc/gl_renderer.cc b/cc/gl_renderer.cc
index 9bb2fdeff6f91ddfe986298c2f6297aba5780d0e..6c7ba24fc1961c523187b5f237c86193703ea77f 100644
--- a/cc/gl_renderer.cc
+++ b/cc/gl_renderer.cc
@@ -14,6 +14,7 @@
 #include "base/string_util.h"
 #include "build/build_config.h"
 #include "cc/compositor_frame.h"
+#include "cc/compositor_frame_ack.h"
 #include "cc/compositor_frame_metadata.h"
 #include "cc/damage_tracker.h"
 #include "cc/geometry_binding.h"
@@ -83,8 +84,10 @@ GLRenderer::GLRenderer(RendererClient* client, OutputSurface* outputSurface, Res
     , m_isUsingBindUniform(false)
     , m_visible(true)
     , m_isScissorEnabled(false)
+    , m_renderToMailbox(outputSurface->Capabilities().render_to_mailbox)
 {
     DCHECK(m_context);
+    m_pendingFrames.push_back(MailboxTexture());
 }
 
 bool GLRenderer::initialize()
@@ -153,6 +156,10 @@ GLRenderer::~GLRenderer()
     m_context->setMemoryAllocationChangedCallbackCHROMIUM(0);
     m_context->setContextLostCallback(0);
     cleanupSharedObjects();
+
+    DCHECK(m_pendingFrames.size() == 1);

[piman, 2013/01/18 02:14:11]

What ensures that this doesn't get deleted while a new frame is in flight (the
ack hasn't been received yet)?

+    if (m_pendingFrames.back().textureId)
+        m_context->deleteTexture(m_pendingFrames.back().textureId);
 }
 
 const RendererCapabilities& GLRenderer::capabilities() const
@@ -206,6 +213,43 @@ void GLRenderer::viewportChanged()
     m_isViewportChanged = true;
 }
 
+void GLRenderer::receiveCompositorFrameAck(const CompositorFrameAck& ack)
+{
+    if (m_renderToMailbox) {

[danakj, 2013/01/18 01:01:41]

if (!m_renderToMailbox)
  return;

+        onSwapBuffersComplete();

[Sami, 2013/01/25 23:17:27]

Should we consume the texture and insert the sync point wait before this
callback? I'm not sure if the delegate could do something with the context that
would get executed too soon.

+
+        DCHECK(m_pendingFrames.size());
+        if (!ack.mailbox.isZero()) {
+            std::vector<MailboxTexture>::iterator it = m_pendingFrames.begin();
+            for (; it != m_pendingFrames.end(); it++) {

[danakj, 2013/01/18 01:01:41]

shouldn't the frame always be the .front() here, similar to the assumption in
the else{} below?

[no sievers, 2013/01/18 01:14:54]

The one exception is skipped frames. When RWHV decides that it does not want to
display the current frame (for example because it has the wrong size), then it
will bounce the latest frame back that it received (instead of the older one it
already had).

And since we might have already swapped another frame, it's also not as easy as
just grabbing either front() or back().

We could also send the size back with the ACK, then we would have to do less
tracking here. I guess I was really only doing this to get the size of the
texture :)

+                if (!memcmp(ack.mailbox.name, it->mailbox.name, sizeof(Mailbox::name)))
+                    break;
+            }
+            CHECK(it != m_pendingFrames.end());
+            MailboxTexture& texture = *it;
+
+            DCHECK(texture.textureId);

[danakj, 2013/01/18 01:01:41]

if you like, i think you could create a texture id for use here, instead of
storing it, which would be cheap as it just creates a client id.

+            m_context->bindTexture(GL_TEXTURE_2D, texture.textureId);
+
+            // If the consumer is bouncing back the same texture (i.e. skipping the frame),
+            // we don't have to synchronize here (sync_point == 0).
+            if (ack.sync_point)
+                m_context->waitSyncPoint(ack.sync_point);
+
+            m_context->consumeTextureCHROMIUM(GL_TEXTURE_2D, ack.mailbox.name);
+            m_returnedTextures.push(texture);
+            m_pendingFrames.erase(it);
+        } else {
+            // Either this is the very first ACK or the oldest buffer is not being returned
+            // because the frontbuffer was discarded in the browser.
+            MailboxTexture& texture = m_pendingFrames.front();
+            if (texture.textureId)
+                m_context->deleteTexture(texture.textureId);
+            m_pendingFrames.erase(m_pendingFrames.begin());
+        }
+    }
+}
+
 void GLRenderer::clearFramebuffer(DrawingFrame& frame)
 {
     // On DEBUG builds, opaque render passes are cleared to blue to easily see regions that were not drawn on the screen.
@@ -235,6 +279,11 @@ void GLRenderer::beginDrawingFrame(DrawingFrame& frame)
         // viewport size is never set.
         m_isViewportChanged = false;
         m_context->reshape(viewportWidth(), viewportHeight());
+
+        if (m_renderToMailbox) {
+            discardBackbuffer();
+            ensureBackbuffer();
+        }
     }
 
     makeContextCurrent();
@@ -1176,7 +1225,24 @@ void GLRenderer::finishDrawingFrame(DrawingFrame& frame)
         CompositorFrame compositor_frame;
         compositor_frame.metadata = m_client->makeCompositorFrameMetadata();
         compositor_frame.gl_frame_data.reset(new GLFrameData());
-        // FIXME: Fill in GLFrameData when we implement swapping with it.
+
+        if (m_renderToMailbox) {
+            // In non-threaded mode there is no swap throttling yet.
+            DCHECK(m_client->hasImplThread());
+
+            DCHECK(!viewportSize().IsEmpty());
+            DCHECK(!m_currentBacking.mailbox.isZero());
+            GLC(m_context, m_context->framebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0));
+            context()->bindTexture(GL_TEXTURE_2D, m_currentBacking.textureId);
+            context()->produceTextureCHROMIUM(GL_TEXTURE_2D, m_currentBacking.mailbox.name);
+            compositor_frame.gl_frame_data->mailbox = m_currentBacking.mailbox;
+            compositor_frame.gl_frame_data->size = m_currentBacking.size;
+            DCHECK(viewportSize() == m_currentBacking.size);
+            compositor_frame.gl_frame_data->sync_point = context()->insertSyncPoint();
+            m_pendingFrames.push_back(m_currentBacking);
+            m_currentBacking = MailboxTexture();
+        }
+
         m_outputSurface->SendFrameToParentCompositor(&compositor_frame);
     }
 }
@@ -1306,7 +1372,10 @@ bool GLRenderer::swapBuffers()
     } else {
         // Note that currently this has the same effect as swapBuffers; we should
         // consider exposing a different entry point on WebGraphicsContext3D.
-        m_context->prepareTexture();
+        if (m_renderToMailbox)
+            m_context->flush();

[piman, 2013/01/18 02:14:11]

We definitely also want to support the partial swap case. I think it's just a
matter of passing the dirty rect into the GLFrame.

[Sami, 2013/01/25 23:17:27]

Is it necessary to flush after inserting a sync point? If yes, then it seems
like the renderer could deadlock the browser by sending it a sync point without
ever flushing.

[piman, 2013/01/25 23:44:56]

The flush isn't necessary to ensure the sync point makes it through -
insertSyncPoint returns once the sync point is in the queue to be processed (in
order wrt the other commands).
It is necessary however to ensure the results of the previous commands is
visible by the other contexts.
It is possible for a malicious renderer to deadlock the browser process, but
it's quite hard to do it accidentally: essentially the renderer has to deadlock
its command buffer processing, by having to guess the upcoming syncpoint id, and
rely on a race on the GPU process side, by doing waitSyncPoint(x);
insertSyncPoint();, where x is the sync point that will be returned by the
insertSyncPoint. After that, the renderer could send x to the browser and
deadlock it too.
We could make the ids unguessable (crypto-random), but other attack could do
this, e.g. by making the command buffer deadlock (possible by adding a jump
command that makes it spin in a loop).
That said, all the renderer can do is deadlock the browser, which is a low-value
attack. If we want to solve it, we need to give up the latency/pipelining
benefits of this CL, i.e. using signalSyncPoint in the browser, which would only
call back once the sync point has passed on the GPU process.
Another alternative is to keep track of the renderers we may be waiting on, and
kill them if the GPU process doesn't make progress from the pov of the browser
(after a timeout), which will kill their contexts and release the sync points.

[Sami, 2013/01/26 00:30:27]

Thanks for the explanation. I was just wondering whether we should make creating
a sync point perform an implicit flush, but it sounds like that wouldn't
completely eliminate the possibility of a deadlock. Perhaps it would be best to
have a timeout on the wait like you said. Seems a little out of scope of this
patch though.

[piman, 2013/01/26 00:37:35]

It does an implicit shallow flush FWIW, to ensure ordering within the same
channel. It's not enough to ensure consistency across contexts though, and I'm
wondering if we should possibly either:
- do a full flush in insertSyncPoint
- or make the shallow flush "lazy", i.e. a noop if we just flushed
Right, I talked to security folks, and they're ok even with a reasonably long
timeout (15-30s). Definitely out-of-scope for this CL.

+        else
+            m_context->prepareTexture();
     }
 
     m_swapBufferRect = gfx::Rect();
@@ -1374,13 +1443,28 @@ void GLRenderer::enforceMemoryPolicy()
 
 void GLRenderer::discardBackbuffer()
 {
-    if (m_isBackbufferDiscarded)
-        return;
+    if (m_renderToMailbox && m_currentBacking.textureId) {
+        if (m_currentBacking.textureId) {
+            context()->deleteTexture(m_currentBacking.textureId);
+            m_currentBacking.textureId = 0;
+            m_currentBacking.mailbox = Mailbox();
+        }
 
-    if (!m_capabilities.usingDiscardBackbuffer)
-        return;
+        while (m_returnedTextures.size()) {
+            m_context->deleteTexture(m_returnedTextures.front());
+            m_returnedTextures.pop();
+        }
+
+    } else {
+        if (m_isBackbufferDiscarded)
+            return;
+
+        if (!m_capabilities.usingDiscardBackbuffer)
+            return;
+
+        m_context->discardBackbufferCHROMIUM();
+    }
 
-    m_context->discardBackbufferCHROMIUM();
     m_isBackbufferDiscarded = true;
 
     // Damage tracker needs a full reset every time framebuffer is discarded.
@@ -1389,13 +1473,44 @@ void GLRenderer::discardBackbuffer()
 
 void GLRenderer::ensureBackbuffer()
 {
-    if (!m_isBackbufferDiscarded)
-        return;
+    if (m_renderToMailbox) {
+        if (!m_currentBacking.textureId) {
+            while (m_returnedTextures.size()) {
+                MailboxTexture& texture = m_returnedTextures.front();
+                if (texture.size == viewportSize()) {
+                    m_currentBacking = texture;
+                    m_returnedTextures.pop();
+                    break;
+                }
+
+                m_context->deleteTexture(texture.textureId);
+                m_returnedTextures.pop();
+            }
+
+            if (!m_currentBacking.textureId) {
+                m_currentBacking.textureId = context()->createTexture();
+                m_currentBacking.size = viewportSize();
+                m_context->genMailboxCHROMIUM(m_currentBacking.mailbox.name);
+                m_context->bindTexture(GL_TEXTURE_2D, m_currentBacking.textureId);
+                m_context->texParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);

[Sami, 2013/01/25 23:17:27]

Nit: could use texParameteri consistently.

[no sievers, 2013/02/06 23:36:00]

Done.

+                m_context->texParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+                m_context->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+                m_context->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+                m_context->texImage2D(GL_TEXTURE_2D, 0, GL_RGBA,
+                                      viewportSize().width(), viewportSize().height(), 0,
+                                      GL_RGBA, GL_UNSIGNED_BYTE, NULL);
+            }
+            DCHECK(m_currentBacking.textureId);
+        }
+    } else {
+        if (!m_isBackbufferDiscarded)
+            return;
 
-    if (!m_capabilities.usingDiscardBackbuffer)
-        return;
+        if (!m_capabilities.usingDiscardBackbuffer)
+            return;
 
-    m_context->ensureBackbufferCHROMIUM();
+        m_context->ensureBackbufferCHROMIUM();
+    }
     m_isBackbufferDiscarded = false;
 }
 
@@ -1499,7 +1614,17 @@ bool GLRenderer::useScopedTexture(DrawingFrame& frame, const ScopedResource* tex
 void GLRenderer::bindFramebufferToOutputSurface(DrawingFrame& frame)
 {
     m_currentFramebufferLock.reset();
-    GLC(m_context, m_context->bindFramebuffer(GL_FRAMEBUFFER, 0));
+
+    if (m_renderToMailbox) {
+        ensureBackbuffer();
+        DCHECK(m_offscreenFramebufferId);
+        DCHECK(m_currentBacking.textureId);
+        GLC(m_context, m_context->bindFramebuffer(GL_FRAMEBUFFER, m_offscreenFramebufferId));
+        GLC(m_context, m_context->framebufferTexture2D(
+                GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, m_currentBacking.textureId, 0));
+    } else {
+        GLC(m_context, m_context->bindFramebuffer(GL_FRAMEBUFFER, 0));
+    }
 }
 
 bool GLRenderer::bindFramebufferToTexture(DrawingFrame& frame, const ScopedResource* texture, const gfx::Rect& framebufferRect)