Revert 278728 "[PPAPI] Add browser tests for compositor API"

> [PPAPI] Add browser tests for compositor API
> 
>  And fix a bug found with the tests.
>  BindGraphics() does not work for a device which is in the same type with the current bound device.
> 
> BUG=374383
> R=piman@chromium.org, raymes@chromium.org
> 
> Review URL: https://codereview.chromium.org/324983005

TBR=penghuang@chromium.org

Review URL: https://codereview.chromium.org/342323006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278765 0039d316-1c4b-4281-b951-d872f2087c98

chrome/test/ppapi/ppapi_browsertest.cc

@@ -1208,16 +1208,6 @@ IN_PROC_BROWSER_TEST_F(OutOfProcessPPAPITest, MAYBE_FlashMessageLoop) {
   RUN_FLASH_MESSAGE_LOOP_SUBTESTS;
 }
 
-#if defined(OS_WIN) && defined(USE_AURA)
-// These tests fail with the test compositor which is what's used by default for
-// browser tests on Windows Aura. Renable when the software compositor is
-// available.
-#define MAYBE_Compositor DISABLED_Compositor
-#else  // defined(OS_WIN) && defined(USE_AURA)
-#define MAYBE_Compositor Compositor
-#endif  // defined(OS_WIN) && defined(USE_AURA)
-TEST_PPAPI_NACL(MAYBE_Compositor)
-
 TEST_PPAPI_NACL(MediaStreamAudioTrack)
 
 TEST_PPAPI_NACL(MediaStreamVideoTrack)

content/renderer/pepper/pepper_compositor_host.cc

@@ -158,35 +158,10 @@ bool PepperCompositorHost::BindToInstance(
   if (bound_instance_ && new_instance)
     return false;  // Can't change a bound device.
   bound_instance_ = new_instance;
-  if (!bound_instance_)
-    SendCommitLayersReplyIfNecessary();
-
   return true;
 }
 
 void PepperCompositorHost::ViewInitiatedPaint() {
-  SendCommitLayersReplyIfNecessary();
-}
-
-void PepperCompositorHost::ViewFlushedPaint() {}
-
-void PepperCompositorHost::ImageReleased(
-    int32_t id,
-    const scoped_ptr<base::SharedMemory>& shared_memory,
-    uint32_t sync_point,
-    bool is_lost) {
-  ResourceReleased(id, sync_point, is_lost);
-}
-
-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::SendCommitLayersReplyIfNecessary() {
   if (!commit_layers_reply_context_.is_valid())
     return;
   host()->SendReply(commit_layers_reply_context_,
@@ -194,6 +169,8 @@ void PepperCompositorHost::SendCommitLayersReplyIfNecessary() {
   commit_layers_reply_context_ = ppapi::host::ReplyMessageContext();
 }
 
+void PepperCompositorHost::ViewFlushedPaint() {}
+
 void PepperCompositorHost::UpdateLayer(
     const scoped_refptr<cc::Layer>& layer,
     const ppapi::CompositorLayerData* old_layer,
@@ -300,6 +277,22 @@ void PepperCompositorHost::UpdateLayer(
   NOTREACHED();
 }
 
+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,
+    const 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) {
@@ -318,9 +311,15 @@ int32_t PepperCompositorHost::OnHostMsgCommitLayers(
     HostMessageContext* context,
     const std::vector<ppapi::CompositorLayerData>& layers,
     bool reset) {
+  // Do not support CommitLayers() on an unbounded compositor.
+  if (!bound_instance_)
+    return PP_ERROR_FAILED;
+
   if (commit_layers_reply_context_.is_valid())
     return PP_ERROR_INPROGRESS;
 
+  commit_layers_reply_context_ = context->MakeReplyMessageContext();
+
   scoped_ptr<scoped_ptr<base::SharedMemory>[]> image_shms;
   if (layers.size() > 0) {
     image_shms.reset(new scoped_ptr<base::SharedMemory>[layers.size()]);
@@ -374,11 +373,6 @@ int32_t PepperCompositorHost::OnHostMsgCommitLayers(
   if (layer_->layer_tree_host())
     layer_->layer_tree_host()->SetNeedsCommit();
 
-  // If the host is not bound to the instance, return PP_OK immediately.
-  if (!bound_instance_)
-    return PP_OK;
-
-  commit_layers_reply_context_ = context->MakeReplyMessageContext();
   return PP_OK_COMPLETIONPENDING;
 }
 

content/renderer/pepper/pepper_compositor_host.h

@@ -44,18 +44,17 @@ class PepperCompositorHost : public ppapi::host::ResourceHost {
  private:
   virtual ~PepperCompositorHost();
 
-  void ImageReleased(int32_t id,
-                     const scoped_ptr<base::SharedMemory>& shared_memory,
-                     uint32_t sync_point,
-                     bool is_lost);
-  void ResourceReleased(int32_t id,
-                        uint32_t sync_point,
-                        bool is_lost);
-  void SendCommitLayersReplyIfNecessary();
   void UpdateLayer(const scoped_refptr<cc::Layer>& layer,
                    const ppapi::CompositorLayerData* old_layer,
                    const ppapi::CompositorLayerData* new_layer,
                    scoped_ptr<base::SharedMemory> image_shm);
+  void ResourceReleased(int32_t id,
+                        uint32_t sync_point,
+                        bool is_lost);
+  void ImageReleased(int32_t id,
+                     const scoped_ptr<base::SharedMemory>& shared_memory,
+                     uint32_t sync_point,
+                     bool is_lost);
 
   // ResourceMessageHandler overrides:
   virtual int32_t OnResourceMessageReceived(

content/renderer/pepper/pepper_plugin_instance_impl.cc

@@ -1877,7 +1877,7 @@ void PepperPluginInstanceImpl::UpdateFlashFullscreenState(
     return;
   }
 
-  UpdateLayer(false);
+  UpdateLayer();
 
   bool old_plugin_focus = PluginHasFocus();
   flash_fullscreen_ = flash_fullscreen;
@@ -2016,7 +2016,7 @@ bool PepperPluginInstanceImpl::PrintPDFOutput(PP_Resource print_output,
 #endif
 }
 
-void PepperPluginInstanceImpl::UpdateLayer(bool device_changed) {
+void PepperPluginInstanceImpl::UpdateLayer() {
   if (!container_)
     return;
 
@@ -2031,8 +2031,7 @@ void PepperPluginInstanceImpl::UpdateLayer(bool device_changed) {
   bool want_texture_layer = want_3d_layer || want_2d_layer;
   bool want_compositor_layer = !!bound_compositor_;
 
-  if (!device_changed &&
-      (want_texture_layer == !!texture_layer_.get()) &&
+  if ((want_texture_layer == !!texture_layer_.get()) &&
       (want_3d_layer == layer_is_hardware_) &&
       (want_compositor_layer == !!compositor_layer_) &&
       layer_bound_to_fullscreen_ == !!fullscreen_container_) {
@@ -2273,7 +2272,7 @@ PP_Bool PepperPluginInstanceImpl::BindGraphics(PP_Instance instance,
 
   // Special-case clearing the current device.
   if (!device) {
-    UpdateLayer(true);
+    UpdateLayer();
     InvalidateRect(gfx::Rect());
     return PP_TRUE;
   }
@@ -2309,13 +2308,13 @@ PP_Bool PepperPluginInstanceImpl::BindGraphics(PP_Instance instance,
   if (compositor) {
     if (compositor->BindToInstance(this)) {
       bound_compositor_ = compositor;
-      UpdateLayer(true);
+      UpdateLayer();
       return PP_TRUE;
     }
   } else if (graphics_2d) {
     if (graphics_2d->BindToInstance(this)) {
       bound_graphics_2d_platform_ = graphics_2d;
-      UpdateLayer(true);
+      UpdateLayer();
       return PP_TRUE;
     }
   } else if (graphics_3d) {
@@ -2324,7 +2323,7 @@ PP_Bool PepperPluginInstanceImpl::BindGraphics(PP_Instance instance,
     if (graphics_3d->pp_instance() == pp_instance() &&
         graphics_3d->BindToInstance(true)) {
       bound_graphics_3d_ = graphics_3d;
-      UpdateLayer(true);
+      UpdateLayer();
       return PP_TRUE;
     }
   }
@@ -3095,7 +3094,7 @@ bool PepperPluginInstanceImpl::FlashSetFullscreen(bool fullscreen,
     DCHECK(!fullscreen_container_);
     fullscreen_container_ =
         render_frame_->CreatePepperFullscreenContainer(this);
-    UpdateLayer(false);
+    UpdateLayer();
   } else {
     DCHECK(fullscreen_container_);
     fullscreen_container_->Destroy();

content/renderer/pepper/pepper_plugin_instance_impl.h

@@ -635,10 +635,7 @@ class CONTENT_EXPORT PepperPluginInstanceImpl
   // - we are not in Flash full-screen mode (or transitioning to it)
   // Otherwise it destroys the layer.
   // It does either operation lazily.
-  // device_changed: true if the bound device has been changed, and
-  // UpdateLayer() will be forced to recreate the layer and attaches to the
-  // container.
-  void UpdateLayer(bool device_changed);
+  void UpdateLayer();
 
   // Internal helper function for PrintPage().
   bool PrintPageHelper(PP_PrintPageNumberRange_Dev* page_ranges,

ppapi/examples/compositor/compositor.cc

@@ -72,7 +72,6 @@ class DemoInstance : public pp::Instance, public pp::Graphics3DClient {
   void InitGL(int32_t result);
   GLuint PrepareFramebuffer();
   pp::ImageData PrepareImage();
-  void PrepareLayers(int32_t frame);
   void Paint(int32_t result, int32_t frame);
   void OnTextureReleased(int32_t result, GLuint texture);
   void OnImageReleased(int32_t result, const pp::ImageData& image);
@@ -107,7 +106,8 @@ DemoInstance::DemoInstance(PP_Instance instance)
       context_(NULL),
       fbo_(0),
       rbo_(0),
-      rebuild_layers_(true),
+      compositor_(this),
+      rebuild_layers_(false),
       total_resource_(0),
       cube_(new SpinningCube()) {
   RequestInputEvents(PP_INPUTEVENT_CLASS_MOUSE);
@@ -147,7 +147,11 @@ bool DemoInstance::HandleInputEvent(const pp::InputEvent& event) {
 void DemoInstance::Graphics3DContextLost() {
   fbo_ = 0;
   rbo_ = 0;
-  rebuild_layers_ = true;
+  compositor_.ResetLayers();
+  color_layer_ = pp::CompositorLayer();
+  stable_texture_layer_ = pp::CompositorLayer();
+  texture_layer_ = pp::CompositorLayer();
+  image_layer_ = pp::CompositorLayer();
   total_resource_ -= static_cast<int32_t>(textures_.size());
   textures_.clear();
   delete context_;
@@ -256,12 +260,14 @@ pp::ImageData DemoInstance::PrepareImage() {
 
 void DemoInstance::Paint(int32_t result, int32_t frame) {
   assert(result == PP_OK);
+
   if (result != PP_OK || !context_)
     return;
 
+  int32_t rv;
+
   if (rebuild_layers_) {
-    compositor_ = pp::Compositor(this);
-    assert(BindGraphics(compositor_));
+    compositor_.ResetLayers();
     color_layer_ = pp::CompositorLayer();
     stable_texture_layer_ = pp::CompositorLayer();
     texture_layer_ = pp::CompositorLayer();
@@ -270,21 +276,6 @@ void DemoInstance::Paint(int32_t result, int32_t frame) {
     rebuild_layers_ = false;
   }
 
-  PrepareLayers(frame);
-
-  int32_t rv = compositor_.CommitLayers(
-      callback_factory_.NewCallback(&DemoInstance::Paint, ++frame));
-  assert(rv == PP_OK_COMPLETIONPENDING);
-
-  pp::VarDictionary dict;
-  dict.Set("total_resource", total_resource_);
-  size_t free_resource = textures_.size() + images_.size();
-  dict.Set("free_resource", static_cast<int32_t>(free_resource));
-  PostMessage(dict);
-}
-
-void DemoInstance::PrepareLayers(int32_t frame) {
-  int32_t rv;
   float factor_sin = sin(M_PI / 180 * frame);
   float factor_cos = cos(M_PI / 180 * frame);
   {
@@ -404,23 +395,26 @@ void DemoInstance::PrepareLayers(int32_t frame) {
     rv = texture_layer_.SetPremultipliedAlpha(PP_FALSE);
     assert(rv == PP_OK);
   }
+
+  rv = compositor_.CommitLayers(
+      callback_factory_.NewCallback(&DemoInstance::Paint, ++frame));
+  assert(rv == PP_OK_COMPLETIONPENDING);
+
+  pp::VarDictionary dict;
+  dict.Set(pp::Var("total_resource"), pp::Var(total_resource_));
+  dict.Set(pp::Var("free_resource"),
+      pp::Var((int32_t)(textures_.size() + images_.size())));
+  PostMessage(dict);
 }
 
 void DemoInstance::OnTextureReleased(int32_t result, GLuint texture) {
-  if (result == PP_OK) {
+  if (result == PP_OK)
     textures_.push_back(texture);
-  } else {
-    glDeleteTextures(1, &texture);
-    total_resource_--;
-  }
 }
 
 void DemoInstance::OnImageReleased(int32_t result, const pp::ImageData& image) {
-  if (result == PP_OK) {
+  if (result == PP_OK)
     images_.push_back(image);
-  } else {
-    total_resource_--;
-  }
 }
 
 // This object is the global object representing this plugin library as long

ppapi/ppapi_sources.gypi

@@ -406,8 +406,6 @@
       'tests/test_audio_config.h',
       'tests/test_case.cc',
       'tests/test_case.h',
-      'tests/test_compositor.cc',
-      'tests/test_compositor.h',
       'tests/test_console.cc',
       'tests/test_console.h',
       'tests/test_core.cc',

ppapi/proxy/compositor_layer_resource.cc

@@ -32,17 +32,11 @@ void OnTextureReleased(
     const ScopedPPResource& context,
     uint32_t texture,
     const scoped_refptr<TrackedCallback>& release_callback,
-    int32_t result,
     uint32_t sync_point,
     bool is_lost) {
   if (!TrackedCallback::IsPending(release_callback))
     return;
 
-  if (result != PP_OK) {
-    release_callback->Run(result);
-    return;
-  }
-
   do {
     if (!sync_point)
       break;
@@ -65,12 +59,11 @@ void OnImageReleased(
     const ScopedPPResource& layer,
     const ScopedPPResource& image,
     const scoped_refptr<TrackedCallback>& release_callback,
-    int32_t result,
     uint32_t sync_point,
     bool is_lost) {
   if (!TrackedCallback::IsPending(release_callback))
     return;
-  release_callback->Run(result);
+  release_callback->Run(PP_OK);
 }
 
 }  // namespace
@@ -112,6 +105,7 @@ int32_t CompositorLayerResource::SetColor(float red,
   if (!size)
     return PP_ERROR_BADARGUMENT;
 
+
   data_.color->red = clamp(red);
   data_.color->green = clamp(green);
   data_.color->blue = clamp(blue);
@@ -213,11 +207,6 @@ int32_t CompositorLayerResource::SetImage(
   data_.image->source_rect.point = PP_MakeFloatPoint(0.0f, 0.0f);
   data_.image->source_rect.size = source_size_;
 
-  // If the PP_Resource of this layer is released by the plugin, the
-  // release_callback will be aborted immediately, but the texture or image
-  // in this layer may still being used by chromium compositor. So we have to
-  // use ScopedPPResource to keep this resource alive until the texture or image
-  // is released by the chromium compositor.
   release_callback_ = base::Bind(
       &OnImageReleased,
       ScopedPPResource(pp_resource()), // Keep layer alive.
@@ -344,7 +333,7 @@ bool CompositorLayerResource::SetType(LayerType type) {
 int32_t CompositorLayerResource::CheckForSetTextureAndImage(
     LayerType type,
     const scoped_refptr<TrackedCallback>& release_callback) {
-  if (!compositor_)
+ if (!compositor_)
     return PP_ERROR_BADRESOURCE;
 
   if (compositor_->IsInProgress())

ppapi/proxy/compositor_layer_resource.h

@@ -22,7 +22,7 @@ class PPAPI_PROXY_EXPORT CompositorLayerResource
       public thunk::PPB_CompositorLayer_API {
  public:
   // Release callback for texture or image layer.
-  typedef base::Callback<void(int32_t, uint32_t, bool)> ReleaseCallback;
+  typedef base::Callback<void(uint32_t, bool)> ReleaseCallback;
 
   CompositorLayerResource(Connection connection,
                           PP_Instance instance,

ppapi/proxy/compositor_resource.cc

@@ -6,7 +6,6 @@
 
 #include "base/logging.h"
 #include "ppapi/proxy/ppapi_messages.h"
-#include "ppapi/thunk/enter.h"
 
 namespace ppapi {
 namespace proxy {
@@ -19,25 +18,8 @@ CompositorResource::CompositorResource(Connection connection,
   SendCreate(RENDERER, PpapiHostMsg_Compositor_Create());
 }
 
-bool CompositorResource::IsInProgress() const {
-  ProxyLock::AssertAcquiredDebugOnly();
-  return TrackedCallback::IsPending(commit_callback_);
-}
-
-int32_t CompositorResource::GenerateResourceId() const {
-  ProxyLock::AssertAcquiredDebugOnly();
-  return ++last_resource_id_;
-}
-
 CompositorResource::~CompositorResource() {
-  ResetLayersInternal(true);
-
-  // Abort all release callbacks.
-  for (ReleaseCallbackMap::iterator it = release_callback_map_.begin();
-       it != release_callback_map_.end(); ++it) {
-    if (!it->second.is_null())
-      it->second.Run(PP_ERROR_ABORTED, 0, false);
-  }
+  ResetLayersInternal();
 }
 
 thunk::PPB_Compositor_API* CompositorResource::AsPPB_Compositor_API() {
@@ -92,8 +74,7 @@ int32_t CompositorResource::CommitLayers(
 int32_t CompositorResource::ResetLayers() {
   if (IsInProgress())
     return PP_ERROR_INPROGRESS;
-
-  ResetLayersInternal(false);
+  ResetLayersInternal();
   return PP_OK;
 }
 
@@ -131,16 +112,16 @@ void CompositorResource::OnPluginMsgReleaseResource(
   ReleaseCallbackMap::iterator it = release_callback_map_.find(id);
   DCHECK(it != release_callback_map_.end()) <<
       "Can not found release_callback_ by id(" << id << ")!";
-  it->second.Run(PP_OK, sync_point, is_lost);
+  it->second.Run(sync_point, is_lost);
   release_callback_map_.erase(it);
 }
 
-void CompositorResource::ResetLayersInternal(bool is_aborted) {
+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(is_aborted ? PP_ERROR_ABORTED : PP_OK, 0, false);
+      release_callback.Run(0, false);
       (*it)->ResetReleaseCallback();
     }
     (*it)->Invalidate();

ppapi/proxy/compositor_resource.h

@@ -10,7 +10,6 @@
 #include "ppapi/proxy/compositor_layer_resource.h"
 #include "ppapi/proxy/plugin_resource.h"
 #include "ppapi/proxy/ppapi_proxy_export.h"
-#include "ppapi/shared_impl/proxy_lock.h"
 #include "ppapi/thunk/ppb_compositor_api.h"
 
 namespace ppapi {
@@ -23,9 +22,13 @@ class PPAPI_PROXY_EXPORT CompositorResource
   CompositorResource(Connection connection,
                      PP_Instance instance);
 
-  bool IsInProgress() const;
+  bool IsInProgress() const {
+    return TrackedCallback::IsPending(commit_callback_);
+  }
 
-  int32_t GenerateResourceId() const;
+  int32_t GenerateResourceId() const {
+    return ++last_resource_id_;
+  }
 
  private:
   virtual ~CompositorResource();
@@ -51,7 +54,7 @@ class PPAPI_PROXY_EXPORT CompositorResource
       uint32_t sync_point,
       bool is_lost);
 
-  void ResetLayersInternal(bool is_aborted);
+  void ResetLayersInternal();
 
   // Callback for CommitLayers().
   scoped_refptr<TrackedCallback> commit_callback_;

ppapi/tests/test_compositor.h

-// Copyright (c) 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.
-
-#ifndef PAPPI_TESTS_TEST_COMPOSITOR_H_
-#define PAPPI_TESTS_TEST_COMPOSITOR_H_
-
-#include <set>
-#include <string>
-
-#include "ppapi/cpp/compositor.h"
-#include "ppapi/cpp/compositor_layer.h"
-#include "ppapi/cpp/graphics_3d.h"
-#include "ppapi/lib/gl/include/GLES2/gl2.h"
-#include "ppapi/tests/test_case.h"
-
-class TestCompositor : public TestCase {
- public:
-  TestCompositor(TestingInstance* instance) : TestCase(instance) {}
-
-  // TestCase implementation.
-  virtual bool Init();
-  virtual void RunTests(const std::string& filter);
-
- private:
-  // Various tests.
-  std::string TestRelease();
-  std::string TestReleaseWithoutCommit();
-  std::string TestCommitTwoTimesWithoutChange();
-  std::string TestGeneral();
-
-  std::string TestReleaseUnbound();
-  std::string TestReleaseWithoutCommitUnbound();
-  std::string TestCommitTwoTimesWithoutChangeUnbound();
-  std::string TestGeneralUnbound();
-
-  std::string TestBindUnbind();
-
-  std::string TestReleaseInternal(bool bind);
-  std::string TestReleaseWithoutCommitInternal(bool bind);
-  std::string TestCommitTwoTimesWithoutChangeInternal(bool bind);
-  std::string TestGeneralInternal(bool bind);
-
-  // Helper functions
-  std::string CreateTexture(uint32_t* texture);
-  std::string ReleaseTexture(uint32_t texture);
-  std::string CreateImage(pp::ImageData* image);
-  std::string SetColorLayer(pp::CompositorLayer layer, int32_t result);
-
-};
-
-#endif  // PAPPI_TESTS_TEST_COMPOSItor_H_