[PPAPI] Compositor API implementation

content/renderer/pepper/pepper_compositor_host.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/renderer/pepper/pepper_compositor_host.h"
+
+#include "base/logging.h"
+#include "cc/layers/texture_layer.h"
+#include "cc/resources/texture_mailbox.h"
+#include "content/public/renderer/renderer_ppapi_host.h"
+#include "content/renderer/pepper/gfx_conversion.h"
+#include "content/renderer/pepper/host_globals.h"
+#include "content/renderer/pepper/pepper_plugin_instance_impl.h"
+#include "content/renderer/pepper/ppb_image_data_impl.h"
+#include "ppapi/c/pp_errors.h"
+#include "ppapi/host/dispatch_host_message.h"
+#include "ppapi/host/host_message_context.h"
+#include "ppapi/host/ppapi_host.h"
+#include "ppapi/proxy/ppapi_messages.h"
+#include "ppapi/thunk/enter.h"
+#include "ppapi/thunk/ppb_image_data_api.h"
+#include "third_party/khronos/GLES2/gl2.h"
+#include "ui/gfx/transform.h"
+
+using ppapi::host::HostMessageContext;
+using ppapi::thunk::EnterResourceNoLock;
+using ppapi::thunk::PPB_ImageData_API;
+
+namespace content {
+
+PepperCompositorHost::PepperCompositorHost(
+    RendererPpapiHost* host,
+    PP_Instance instance,
+    PP_Resource resource)
+    : ResourceHost(host->GetPpapiHost(), instance, resource),
+      bound_instance_(NULL),
+      weak_factory_(this) {
+  layer_ = cc::SolidColorLayer::Create();
+  layer_->SetMasksToBounds(true);
+  layer_->SetIsDrawable(true);
+}
+
+PepperCompositorHost::~PepperCompositorHost() {
+  // Unbind from the instance when destroyed if we're still bound.
+  if (bound_instance_)
+    bound_instance_->BindGraphics(bound_instance_->pp_instance(), 0);
+}
+
+bool PepperCompositorHost::BindToInstance(
+    PepperPluginInstanceImpl* new_instance) {
+  if (new_instance && new_instance->pp_instance() != pp_instance())
+    return false;  // Can't bind other instance's contexts.
+  if (bound_instance_ == new_instance)
+    return true;  // Rebinding the same device, nothing to do.
+  if (bound_instance_ && new_instance)
+    return false;  // Can't change a bound device.
+  bound_instance_ = new_instance;
+  return true;
+}
+
+void PepperCompositorHost::ViewInitiatedPaint() {
+  if (!commit_layers_reply_context_.is_valid())
+    return;
+  host()->SendReply(commit_layers_reply_context_,
+                    PpapiPluginMsg_Compositor_CommitLayersReply());
+  commit_layers_reply_context_ = ppapi::host::ReplyMessageContext();
+}
+
+void PepperCompositorHost::ViewFlushedPaint() {}

[raymes, 2014/06/02 03:51:08]

Why is this function implemented at all?

[Peng, 2014/06/02 19:01:55]

Graphics3d and 2d Hosts implements both ViewFlushedPaint() &
ViewInitiatedPaint(), even if one function body is empty. So I followed them.
Should I remove this function?

[raymes, 2014/06/03 00:43:22]

Ah ok, that seems reasonable then. Thanks.

+
+void PepperCompositorHost::ResourceReleased(int32_t id,
+                                            uint32_t sync_point,
+                                            bool is_lost) {
+  host()->SendUnsolicitedReply(
+      pp_resource(),
+      PpapiPluginMsg_Compositor_ReleaseResource(id, sync_point, is_lost));
+}
+
+void PepperCompositorHost::ImageReleased(
+    int32_t id,
+    scoped_ptr<base::SharedMemory> shared_memory,
+    uint32_t sync_point,
+    bool is_lost) {
+  ResourceReleased(id, sync_point, is_lost);
+}
+
+int32_t PepperCompositorHost::OnResourceMessageReceived(
+    const IPC::Message& msg,
+    HostMessageContext* context) {
+  PPAPI_BEGIN_MESSAGE_MAP(PepperCompositorHost, msg)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL(
+      PpapiHostMsg_Compositor_CommitLayers, OnHostMsgCommitLayers)
+  PPAPI_END_MESSAGE_MAP()
+  return ppapi::host::ResourceHost::OnResourceMessageReceived(msg, context);
+}
+
+bool PepperCompositorHost::IsCompositorHost() {
+  return true;
+}
+
+void PepperCompositorHost::UpdateLayer(
+    const scoped_refptr<cc::Layer>& layer,
+    ppapi::CompositorLayer* old_layer,

[raymes, 2014/06/02 03:51:08]

Can this be const too?

[Peng, 2014/06/02 19:01:55]

Done.

+    const ppapi::CompositorLayer* new_layer) {
+  CHECK(!old_layer || new_layer->type == old_layer->type);
+
+  // Always update properties on cc::Layer, because cc::Layer
+  // will ignore any setting with unchanged value.
+  layer->SetBounds(PP_ToGfxSize((new_layer)->size));
+  layer->SetBlendMode(SkXfermode::kSrcOver_Mode);
+  layer->SetOpacity(new_layer->opacity / 255.0f);
+
+  gfx::Transform transform(gfx::Transform::kSkipInitialization);
+  transform.matrix().setColMajorf(new_layer->transform);
+  layer->SetTransform(transform);
+
+  // Consider a (0,0,0,0) rect as no clip rect.
+  if (new_layer->clip_rect.point.x != 0 ||
+      new_layer->clip_rect.point.y != 0 ||
+      new_layer->clip_rect.size.width != 0 ||
+      new_layer->clip_rect.size.height != 0) {

[raymes, 2014/06/02 03:51:08]

Would it be better to consider any rect that has an empty area as indicating no
clip? (new_layer->clip_rect.size.width * new_layer->clip_rect.size.height == 0)?

[Peng, 2014/06/02 19:01:55]

I think using (0,0,0,0) is a better. Otherwise, if a plugin sets a (100, 100,
100, 0),
we will display the whole layer on screen. But plugin probably wants to show
nothing on
screen.

[raymes, 2014/06/03 00:43:22]

Ah I see.

+    if (!layer->mask_layer())
+      layer->SetMaskLayer(cc::Layer::Create());
+    layer->mask_layer()->SetPosition(PP_ToGfxPoint(new_layer->clip_rect.point));
+    layer->mask_layer()->SetBounds(PP_ToGfxSize(new_layer->clip_rect.size));
+  } else {
+    layer->SetMaskLayer(NULL);
+  }
+
+  if (new_layer->type == ppapi::CompositorLayer::TYPE_COLOR) {
+    layer->SetBackgroundColor(SkColorSetARGBMacro(new_layer->color.alpha,
+                                                  new_layer->color.red,
+                                                  new_layer->color.green,
+                                                  new_layer->color.blue));
+  } else if (new_layer->type == ppapi::CompositorLayer::TYPE_TEXTURE) {
+    scoped_refptr<cc::TextureLayer> texture_layer(
+        static_cast<cc::TextureLayer*>(layer.get()));
+    if (!old_layer || new_layer->texture.id != old_layer->texture.id) {
+      gpu::Mailbox gpu_mailbox;
+      gpu_mailbox.SetName(new_layer->texture.mailbox);
+      cc::TextureMailbox mailbox(gpu_mailbox,
+                                 GL_TEXTURE_2D,
+                                 new_layer->texture.sync_point);
+      texture_layer->SetTextureMailbox(mailbox,
+          cc::SingleReleaseCallback::Create(
+              base::Bind(&PepperCompositorHost::ResourceReleased,
+                         weak_factory_.GetWeakPtr(),
+                         new_layer->texture.id)));;
+    }
+    texture_layer->SetPremultipliedAlpha(new_layer->texture.premult_alpha);
+    gfx::RectF rect = PP_ToGfxRectF(new_layer->texture.source_rect);
+    texture_layer->SetUV(rect.origin(), rect.bottom_right());
+  } else if (new_layer->type == ppapi::CompositorLayer::TYPE_IMAGE) {
+    if (!old_layer || new_layer->image.id != old_layer->image.id) {
+      scoped_refptr<cc::TextureLayer> image_layer(
+          static_cast<cc::TextureLayer*>(layer.get()));
+      EnterResourceNoLock<PPB_ImageData_API> enter(
+          new_layer->image.host_resource, true);
+
+      do {

[raymes, 2014/06/02 03:51:08]

Why not get rid of this do/while and just return from the function in the error
cases?

[Peng, 2014/06/02 19:01:55]

Done

+        if(enter.failed()) {
+          LOG(WARNING) << "Image is invalid!";
+          break;
+        }
+
+        PP_ImageDataDesc desc;
+        if (!enter.object()->Describe(&desc)) {
+          LOG(WARNING) << "Can not describe the image!";
+          break;
+        }
+
+        // TODO(penghuang): support all kinds of image.
+        DCHECK_EQ(desc.stride, desc.size.width * 4);
+        DCHECK_EQ(desc.format, PP_IMAGEDATAFORMAT_RGBA_PREMUL);

[raymes, 2014/06/02 03:51:08]

Should we early exit in these cases too?

[Peng, 2014/06/02 19:01:55]

Maybe using CHECK() is better. Because we also checks desc.stride and
desc.format in plugin process. 
Those check should not failed in renderer process.

[raymes, 2014/06/03 00:43:22]

That's a good point. But I think we usually just return an error in these cases
instead, even if it is validated in the plugin (you can also add a NOTREACHED()
so that we crash in debug). Even if the plugin is compromised (or it might just
be buggy) it doesn't necessarily mean that we want to crash the renderer too.

Also we should be careful with this because I don't know if enter.succeeded() is
guaranteed to be successful. We don't want a case where the plugin can
intentionally crash the renderer.

[Peng, 2014/06/03 18:32:23]

Done.

+
+        int handle;
+        uint32_t byte_count;
+        if (enter.object()->GetSharedMemory(&handle, &byte_count) != PP_OK) {
+          LOG(WARNING) << "Can not access the image's shared memory!";
+          break;
+        }
+
+#if defined(OS_WIN)
+        base::SharedMemoryHandle shm_handle =
+            static_cast<base::SharedMemoryHandle>(_dup(handle));
+#else
+        base::SharedMemoryHandle shm_handle(dup(handle), false);
+#endif
+        // Use a scoped_ptr to keep shared_memory alive until
+        // PepperCompositorHost::ImageReleased is called.
+        scoped_ptr<base::SharedMemory> shared_memory(
+            new base::SharedMemory(shm_handle, true));
+        CHECK(shared_memory->Map(desc.stride * desc.size.height));
+        cc::TextureMailbox mailbox(shared_memory.get(),
+                                   PP_ToGfxSize(desc.size));
+        image_layer->SetTextureMailbox(mailbox,
+            cc::SingleReleaseCallback::Create(
+                base::Bind(&PepperCompositorHost::ImageReleased,
+                           weak_factory_.GetWeakPtr(),
+                           new_layer->image.id,
+                           base::Passed(&shared_memory))));
+
+        // ImageData is always premultiplied alpha.
+        image_layer->SetPremultipliedAlpha(true);
+      } while (false);
+    }
+  } else {
+    NOTREACHED();
+  }
+}
+
+int32_t PepperCompositorHost::OnHostMsgCommitLayers(
+    HostMessageContext* context,
+    const std::vector<ppapi::CompositorLayer>& layers,
+    bool changed) {

[raymes, 2014/06/02 03:51:08]

Would "reset" be a better parameter name here?

[Peng, 2014/06/02 19:01:55]

Done.

+  // Do not support CommitLayers on an unbounded compositor.
+  if (!bound_instance_)
+    return PP_ERROR_FAILED;
+
+  DCHECK(!commit_layers_reply_context_.is_valid());

[raymes, 2014/06/02 03:51:08]

I think it might be better to return an error in this case.

[Peng, 2014/06/02 19:01:55]

In plugin process, the CompositorResource does not allow calling CommitLayers(),
if the previous call is in progress. So this check should always true, unless
the plugin or renderer process is compromised. Maybe we should crash renderer
here?
Or crash the plugin process. Do you know how to do it?

Changed it to CHECK() for now.

[raymes, 2014/06/03 00:43:22]

See the note above.

[Peng, 2014/06/03 18:32:23]

Done.

+  commit_layers_reply_context_ = context->MakeReplyMessageContext();
+
+  // ResetLayers() has been called, we need rebuild layer stack.
+  if (changed) {
+    layer_->RemoveAllChildren();
+    cc_layers_.clear();
+    pp_layers_.clear();
+  }
+
+  size_t i = 0;
+  for (std::vector<ppapi::CompositorLayer>::const_iterator it = layers.begin();
+       it != layers.end(); ++it, ++i) {

[raymes, 2014/06/02 03:51:08]

Rather than having two iterating variables, it might be better just to do
for(size_t i = 0; i < layers.size(); ++i) {
  ppapi::CompositorLayer* new_layer = &layers[i];
  new_layer->...
}

[Peng, 2014/06/02 19:01:55]

Done.

+    ppapi::CompositorLayer* old_layer =
+        i >= pp_layers_.size() ? NULL : &pp_layers_[i];
+    scoped_refptr<cc::Layer> layer =

[raymes, 2014/06/02 03:51:08]

cc_layer (may be slightly more descriptive)

[Peng, 2014/06/02 19:01:55]

Done.

+        i >= cc_layers_.size() ? NULL : cc_layers_[i];
+
+    CHECK((old_layer && layer) || (!old_layer && !layer));
+
+    if (!layer) {
+      if (it->type == ppapi::CompositorLayer::TYPE_COLOR) {
+        layer = cc::SolidColorLayer::Create();
+      } else if (it->type == ppapi::CompositorLayer::TYPE_TEXTURE ||
+               it->type == ppapi::CompositorLayer::TYPE_IMAGE) {
+        layer = cc::TextureLayer::CreateForMailbox(NULL);
+      } else {
+        NOTREACHED();

[raymes, 2014/06/02 03:51:08]

We should return an error value here.

[Peng, 2014/06/02 19:01:55]

Same. CompositorLayer() already verifies all layer before sending this IPC.
So it should not happen, unless the plugin or renderer process is compromised.
So maybe crash is better?

[raymes, 2014/06/03 00:43:22]

See the note above.

[Peng, 2014/06/03 18:32:23]

Done.

+      }
+      layer->SetIsDrawable(true);
+      layer->SetAnchorPoint(gfx::PointF());
+    }
+
+    UpdateLayer(layer, old_layer, &(*it));
+
+    if (old_layer) {
+      *old_layer = *it;
+    } else {
+      layer_->AddChild(layer);
+      cc_layers_.push_back(layer);
+      pp_layers_.push_back(*it);
+    }
+  }
+  return PP_OK_COMPLETIONPENDING;
+}
+
+}  // namespace content