[PPAPI] Compositor API implementation

ppapi/proxy/compositor_resource.cc

Index: ppapi/proxy/compositor_resource.cc
diff --git a/ppapi/proxy/compositor_resource.cc b/ppapi/proxy/compositor_resource.cc
index 497e58a5e6020b4ceef9c562453f7aae3944f762..3ada734849839aca56078ac7e3859b6d574df1a6 100644
--- a/ppapi/proxy/compositor_resource.cc
+++ b/ppapi/proxy/compositor_resource.cc
@@ -4,32 +4,131 @@
 
 #include "ppapi/proxy/compositor_resource.h"
 
+#include "base/logging.h"
+#include "ppapi/proxy/ppapi_messages.h"
+
 namespace ppapi {
 namespace proxy {
 
 CompositorResource::CompositorResource(Connection connection,
                                        PP_Instance instance)
-    : PluginResource(connection, instance) {
+    : PluginResource(connection, instance),
+      layer_reset_(true),
+      last_resource_id_(0) {
+  SendCreate(RENDERER, PpapiHostMsg_Compositor_Create());
 }
 
 CompositorResource::~CompositorResource() {
+  ResetLayersInternal();
 }
 
 thunk::PPB_Compositor_API* CompositorResource::AsPPB_Compositor_API() {
   return this;
 }
 
+void CompositorResource::OnReplyReceived(
+    const ResourceMessageReplyParams& params,
+    const IPC::Message& msg) {
+   PPAPI_BEGIN_MESSAGE_MAP(CompositorResource, msg)
+     PPAPI_DISPATCH_PLUGIN_RESOURCE_CALL(
+         PpapiPluginMsg_Compositor_ReleaseResource,
+         OnPluginMsgReleaseResource)
+     PPAPI_DISPATCH_PLUGIN_RESOURCE_CALL_UNHANDLED(
+          PluginResource::OnReplyReceived(params, msg))
+   PPAPI_END_MESSAGE_MAP()
+}
+
 PP_Resource CompositorResource::AddLayer() {
-  return 0;
+  scoped_refptr<CompositorLayerResource> resource(new CompositorLayerResource(
+      connection(), pp_instance(), this));
+  layers_.push_back(resource);
+  return resource->GetReference();
 }
 
 int32_t CompositorResource::CommitLayers(
     const scoped_refptr<ppapi::TrackedCallback>& callback) {
-  return PP_ERROR_NOTSUPPORTED;
+  if (IsInProgress())
+    return PP_ERROR_INPROGRESS;
+
+  std::vector<CompositorLayerData> layers;
+  layers.reserve(layers_.size());
+
+  for (LayerList::const_iterator it = layers_.begin();
+       it != layers_.end(); ++it) {
+    if ((*it)->data().is_null())
+      return PP_ERROR_FAILED;
+    layers.push_back((*it)->data());
+  }
+
+  commit_callback_ = callback;
+  Call<PpapiPluginMsg_Compositor_CommitLayersReply>(
+      RENDERER,
+      PpapiHostMsg_Compositor_CommitLayers(layers, layer_reset_),
+      base::Bind(&CompositorResource::OnPluginMsgCommitLayersReply,
+                 base::Unretained(this)),
+      callback);
+
+  return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t CompositorResource::ResetLayers() {
-  return PP_ERROR_NOTSUPPORTED;
+  if (IsInProgress())
+    return PP_ERROR_INPROGRESS;
+  ResetLayersInternal();
+  return PP_OK;
+}
+
+void CompositorResource::OnPluginMsgCommitLayersReply(
+    const ResourceMessageReplyParams& params) {
+  if (!TrackedCallback::IsPending(commit_callback_))
+    return;
+
+  // On success, we put layers' release_callbacks into a map,
+  // otherwise we will do nothing. So plugin may change layers and
+  // call CommitLayers() again.
+  if (params.result() == PP_OK) {
+    layer_reset_ = false;
+    for (LayerList::iterator it = layers_.begin();
+         it != layers_.end(); ++it) {
+      ReleaseCallback release_callback = (*it)->release_callback();
+      if (!release_callback.is_null()) {
+        release_callback_map_.insert(ReleaseCallbackMap::value_type(
+            (*it)->data().common.resource_id, release_callback));
+        (*it)->ResetReleaseCallback();
+      }
+    }
+  }
+
+  scoped_refptr<TrackedCallback> callback;
+  callback.swap(commit_callback_);
+  callback->Run(params.result());
+}
+
+void CompositorResource::OnPluginMsgReleaseResource(
+    const ResourceMessageReplyParams& params,
+    int32_t id,
+    uint32_t sync_point,
+    bool is_lost) {
+  ReleaseCallbackMap::iterator it = release_callback_map_.find(id);
+  DCHECK(it != release_callback_map_.end()) <<

[nasko, 2014/06/13 17:23:01]

Why is this just a DCHECK? If there is no callback, the process will crash. It
is better to crash in controlled way (aka CHECK) than through accessing possibly
invalid memory.

[piman, 2014/06/13 17:30:05]

Preemptive comment: no CHECK please. If this needs to be verified, it should be
tested not asserted.

This is in the plugin process, coming from the renderer. Is the renderer
"elevating" into the plugin a concern?

[nasko, 2014/06/13 17:38:49]

Sure, I don't have strong preference on which way.
 
In general, I prefer to be defensive on process boundaries. If you feel strongly
that a check isn't needed here, I'm ok with that.

[Peng, 2014/06/13 18:04:05]

I think this DCHECK() should be true always (unless some bug).
If it does happen for some reason. probably crashing the plugin is a good
choice. So I think current code is OK.

+      "Can not found release_callback_ by id(" << id << ")!";
+  it->second.Run(sync_point, is_lost);
+  release_callback_map_.erase(it);
+}
+
+void CompositorResource::ResetLayersInternal() {
+  for (LayerList::iterator it = layers_.begin();
+       it != layers_.end(); ++it) {
+    ReleaseCallback release_callback = (*it)->release_callback();
+    if (!release_callback.is_null()) {
+      release_callback.Run(0, false);
+      (*it)->ResetReleaseCallback();
+    }
+    (*it)->Invalidate();
+  }
+
+  layers_.clear();
+  layer_reset_ = true;
 }
 
 }  // namespace proxy