Make Surface creation lazy for OffscreenCanvasFrameReceiverImpl

This CL shifts the SurfaceFactory pointer and SurfaceFactoryClient implementation
 from OffscreenCanvasSurfaceImpl to OffscreenCanvasFrameReceiverImpl to
facilitate resource handling after compositor frame is submitted. As a result,
surface on browser is lazily created (only happened on the first commit()).

BUG=563852

Review-Url: https://codereview.chromium.org/2333133003
Cr-Commit-Position: refs/heads/master@{#418402}

content/browser/renderer_host/offscreen_canvas_frame_receiver_impl.cc

@@ -13,7 +13,18 @@ namespace content {
 
 OffscreenCanvasFrameReceiverImpl::OffscreenCanvasFrameReceiverImpl() {}
 
-OffscreenCanvasFrameReceiverImpl::~OffscreenCanvasFrameReceiverImpl() {}
+OffscreenCanvasFrameReceiverImpl::~OffscreenCanvasFrameReceiverImpl() {
+  if (surface_factory_) {
+    if (!GetSurfaceManager()) {
+      // Inform SurfaceFactory that SurfaceManager's no longer alive to
+      // avoid its destruction error.
+      surface_factory_->DidDestroySurfaceManager();
+    } else {
+      GetSurfaceManager()->InvalidateSurfaceClientId(surface_id_.client_id());
+    }
+    surface_factory_->Destroy(surface_id_);
+  }
+}
 
 // static
 void OffscreenCanvasFrameReceiverImpl::Create(
@@ -25,11 +36,30 @@ void OffscreenCanvasFrameReceiverImpl::Create(
 void OffscreenCanvasFrameReceiverImpl::SubmitCompositorFrame(
     const cc::SurfaceId& surface_id,
     cc::CompositorFrame frame) {
-    cc::Surface* surface = GetSurfaceManager()->GetSurfaceForId(surface_id);
-    if (surface) {
-        surface->QueueFrame(std::move(frame), base::Closure());
+  if (!surface_factory_) {
+    cc::SurfaceManager* manager = GetSurfaceManager();
+    surface_factory_ = base::MakeUnique<cc::SurfaceFactory>(manager, this);
+    surface_factory_->Create(surface_id);
+
+    GetSurfaceManager()->RegisterSurfaceClientId(surface_id.client_id());
+  }
+  if (surface_id_.is_null()) {
+    surface_id_ = surface_id;
     }
-    // If surface doet not exist, drop the frame.
+    surface_factory_->SubmitCompositorFrame(surface_id, std::move(frame),
+                                            base::Closure());
 }
 
+// TODO(619136): Implement cc::SurfaceFactoryClient functions for resources
+// return.
+void OffscreenCanvasFrameReceiverImpl::ReturnResources(
+    const cc::ReturnedResourceArray& resources) {}
+
+void OffscreenCanvasFrameReceiverImpl::WillDrawSurface(
+    const cc::SurfaceId& id,
+    const gfx::Rect& damage_rect) {}
+
+void OffscreenCanvasFrameReceiverImpl::SetBeginFrameSource(
+    cc::BeginFrameSource* begin_frame_source) {}
+
 }  // namespace content

content/browser/renderer_host/offscreen_canvas_frame_receiver_impl.h

@@ -5,12 +5,15 @@
 #ifndef CONTENT_BROWSER_RENDERER_HOST_OFFSCREEN_CANVAS_FRAME_RECEIVER_IMPL_H_
 #define CONTENT_BROWSER_RENDERER_HOST_OFFSCREEN_CANVAS_FRAME_RECEIVER_IMPL_H_
 
+#include "cc/surfaces/surface_factory.h"
+#include "cc/surfaces/surface_factory_client.h"
 #include "third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom.h"
 
 namespace content {
 
 class OffscreenCanvasFrameReceiverImpl
-    : public blink::mojom::OffscreenCanvasFrameReceiver {
+    : public blink::mojom::OffscreenCanvasFrameReceiver,
+      public cc::SurfaceFactoryClient {
  public:
   OffscreenCanvasFrameReceiverImpl();
   ~OffscreenCanvasFrameReceiverImpl() override;
@@ -20,7 +23,16 @@ class OffscreenCanvasFrameReceiverImpl
   void SubmitCompositorFrame(const cc::SurfaceId& surface_id,
                              cc::CompositorFrame frame) override;
 
+  // cc::SurfaceFactoryClient implementation.
+  void ReturnResources(const cc::ReturnedResourceArray& resources) override;
+  void WillDrawSurface(const cc::SurfaceId& id,
+                       const gfx::Rect& damage_rect) override;
+  void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) override;
+
  private:
+  cc::SurfaceId surface_id_;
+  std::unique_ptr<cc::SurfaceFactory> surface_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(OffscreenCanvasFrameReceiverImpl);
 };
 

content/browser/renderer_host/offscreen_canvas_surface_impl.cc

@@ -14,21 +14,9 @@
 namespace content {
 
 OffscreenCanvasSurfaceImpl::OffscreenCanvasSurfaceImpl()
-    : id_allocator_(new cc::SurfaceIdAllocator(AllocateSurfaceClientId())) {
-  GetSurfaceManager()->RegisterSurfaceClientId(id_allocator_->client_id());
-}
+    : id_allocator_(new cc::SurfaceIdAllocator(AllocateSurfaceClientId())) {}
 
-OffscreenCanvasSurfaceImpl::~OffscreenCanvasSurfaceImpl() {
-  if (!GetSurfaceManager()) {
-    // Inform both members that SurfaceManager's no longer alive to
-    // avoid their destruction errors.
-    if (surface_factory_)
-        surface_factory_->DidDestroySurfaceManager();
-  } else {
-    GetSurfaceManager()->InvalidateSurfaceClientId(id_allocator_->client_id());
-  }
-  surface_factory_->Destroy(surface_id_);
-}
+OffscreenCanvasSurfaceImpl::~OffscreenCanvasSurfaceImpl() {}
 
 // static
 void OffscreenCanvasSurfaceImpl::Create(
@@ -46,15 +34,6 @@ void OffscreenCanvasSurfaceImpl::GetSurfaceId(
   callback.Run(surface_id_);
 }
 
-void OffscreenCanvasSurfaceImpl::RequestSurfaceCreation(
-    const cc::SurfaceId& surface_id) {
-  cc::SurfaceManager* manager = GetSurfaceManager();
-  if (!surface_factory_) {
-    surface_factory_ = base::MakeUnique<cc::SurfaceFactory>(manager, this);
-  }
-  surface_factory_->Create(surface_id);
-}
-
 void OffscreenCanvasSurfaceImpl::Require(const cc::SurfaceId& surface_id,
                                          const cc::SurfaceSequence& sequence) {
   cc::SurfaceManager* manager = GetSurfaceManager();
@@ -73,16 +52,4 @@ void OffscreenCanvasSurfaceImpl::Satisfy(const cc::SurfaceSequence& sequence) {
   manager->DidSatisfySequences(sequence.client_id, &sequences);
 }
 
-// TODO(619136): Implement cc::SurfaceFactoryClient functions for resources
-// return.
-void OffscreenCanvasSurfaceImpl::ReturnResources(
-    const cc::ReturnedResourceArray& resources) {}
-
-void OffscreenCanvasSurfaceImpl::WillDrawSurface(const cc::SurfaceId& id,
-                                                 const gfx::Rect& damage_rect) {
-}
-
-void OffscreenCanvasSurfaceImpl::SetBeginFrameSource(
-    cc::BeginFrameSource* begin_frame_source) {}
-
 }  // namespace content

content/browser/renderer_host/offscreen_canvas_surface_impl.h

@@ -5,8 +5,6 @@
 #ifndef CONTENT_BROWSER_RENDERER_HOST_OFFSCREEN_CANVAS_SURFACE_IMPL_H_
 #define CONTENT_BROWSER_RENDERER_HOST_OFFSCREEN_CANVAS_SURFACE_IMPL_H_
 
-#include "cc/surfaces/surface_factory.h"
-#include "cc/surfaces/surface_factory_client.h"
 #include "cc/surfaces/surface_id.h"
 #include "cc/surfaces/surface_id_allocator.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
@@ -15,8 +13,7 @@
 
 namespace content {
 
-class OffscreenCanvasSurfaceImpl : public blink::mojom::OffscreenCanvasSurface,
-                                   public cc::SurfaceFactoryClient {
+class OffscreenCanvasSurfaceImpl : public blink::mojom::OffscreenCanvasSurface {
  public:
   OffscreenCanvasSurfaceImpl();
   ~OffscreenCanvasSurfaceImpl() override;
@@ -25,22 +22,14 @@ class OffscreenCanvasSurfaceImpl : public blink::mojom::OffscreenCanvasSurface,
 
   // blink::mojom::OffscreenCanvasSurface implementation.
   void GetSurfaceId(const GetSurfaceIdCallback& callback) override;
-  void RequestSurfaceCreation(const cc::SurfaceId& surface_id) override;
   void Require(const cc::SurfaceId& surface_id,
                const cc::SurfaceSequence& sequence) override;
   void Satisfy(const cc::SurfaceSequence& sequence) override;
 
-  // cc::SurfaceFactoryClient implementation.
-  void ReturnResources(const cc::ReturnedResourceArray& resources) override;
-  void WillDrawSurface(const cc::SurfaceId& id,
-                       const gfx::Rect& damage_rect) override;
-  void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) override;
-
  private:
   // Surface-related state
   std::unique_ptr<cc::SurfaceIdAllocator> id_allocator_;
   cc::SurfaceId surface_id_;
-  std::unique_ptr<cc::SurfaceFactory> surface_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(OffscreenCanvasSurfaceImpl);
 };

third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp

@@ -32,7 +32,6 @@ bool CanvasSurfaceLayerBridge::createSurfaceLayer(int canvasWidth, int canvasHei
     if (!m_client->syncGetSurfaceId(&m_surfaceId))
         return false;
 
-    m_client->asyncRequestSurfaceCreation(m_surfaceId);
     cc::SurfaceLayer::SatisfyCallback satisfyCallback = convertToBaseCallback(WTF::bind(&CanvasSurfaceLayerBridge::satisfyCallback, WTF::unretained(this)));
     cc::SurfaceLayer::RequireCallback requireCallback = convertToBaseCallback(WTF::bind(&CanvasSurfaceLayerBridge::requireCallback, WTF::unretained(this)));
     m_surfaceLayer = cc::SurfaceLayer::Create(std::move(satisfyCallback), std::move(requireCallback));

third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridgeClient.h

@@ -19,7 +19,6 @@ public:
 
     // Calls that help initial creation of SurfaceLayer.
     virtual bool syncGetSurfaceId(cc::SurfaceId*) = 0;
-    virtual void asyncRequestSurfaceCreation(const cc::SurfaceId&) = 0;
 
     // Calls that ensure correct destruction order of surface.
     virtual void asyncRequire(const cc::SurfaceId&, const cc::SurfaceSequence&) = 0;

third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridgeClientImpl.cpp

@@ -24,11 +24,6 @@ bool CanvasSurfaceLayerBridgeClientImpl::syncGetSurfaceId(cc::SurfaceId* surface
     return m_service->GetSurfaceId(surfaceIdPtr);
 }
 
-void CanvasSurfaceLayerBridgeClientImpl::asyncRequestSurfaceCreation(const cc::SurfaceId& surfaceId)
-{
-    m_service->RequestSurfaceCreation(surfaceId);
-}
-
 void CanvasSurfaceLayerBridgeClientImpl::asyncRequire(const cc::SurfaceId& surfaceId, const cc::SurfaceSequence& sequence)
 {
     m_service->Require(surfaceId, sequence);

third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridgeClientImpl.h

@@ -17,7 +17,6 @@ public:
     ~CanvasSurfaceLayerBridgeClientImpl() override;
 
     bool syncGetSurfaceId(cc::SurfaceId*) override;
-    void asyncRequestSurfaceCreation(const cc::SurfaceId&) override;
     void asyncRequire(const cc::SurfaceId&, const cc::SurfaceSequence&) override;
     void asyncSatisfy(const cc::SurfaceSequence&) override;
 

third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridgeTest.cpp

@@ -10,7 +10,6 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "wtf/PtrUtil.h"
-#include "wtf/Vector.h"
 #include <memory>
 
 namespace blink {
@@ -18,15 +17,9 @@ namespace blink {
 class FakeOffscreenCanvasSurfaceImpl {
 public:
     FakeOffscreenCanvasSurfaceImpl() {}
-    ~FakeOffscreenCanvasSurfaceImpl();
+    ~FakeOffscreenCanvasSurfaceImpl() {}
 
     bool GetSurfaceId(cc::SurfaceId*);
-    void RequestSurfaceCreation(const cc::SurfaceId&);
-
-    bool isSurfaceInSurfaceMap(const cc::SurfaceId&);
-
-private:
-    Vector<cc::SurfaceId> m_fakeSurfaceMap;
 };
 
 //-----------------------------------------------------------------------------
@@ -37,7 +30,6 @@ public:
     ~MockCanvasSurfaceLayerBridgeClient() override;
 
     bool syncGetSurfaceId(cc::SurfaceId*) override;
-    void asyncRequestSurfaceCreation(const cc::SurfaceId&) override;
     void asyncRequire(const cc::SurfaceId&, const cc::SurfaceSequence&) override {}
     void asyncSatisfy(const cc::SurfaceSequence&) override {}
 
@@ -77,32 +69,12 @@ bool MockCanvasSurfaceLayerBridgeClient::syncGetSurfaceId(cc::SurfaceId* surface
     return m_service->GetSurfaceId(surfaceIdPtr);
 }
 
-void MockCanvasSurfaceLayerBridgeClient::asyncRequestSurfaceCreation(const cc::SurfaceId& surfaceId)
-{
-    m_service->RequestSurfaceCreation(surfaceId);
-}
-
-FakeOffscreenCanvasSurfaceImpl::~FakeOffscreenCanvasSurfaceImpl()
-{
-    m_fakeSurfaceMap.clear();
-}
-
 bool FakeOffscreenCanvasSurfaceImpl::GetSurfaceId(cc::SurfaceId* surfaceId)
 {
     *surfaceId = cc::SurfaceId(10, 15, 0);
     return true;
 }
 
-void FakeOffscreenCanvasSurfaceImpl::RequestSurfaceCreation(const cc::SurfaceId& surfaceId)
-{
-    m_fakeSurfaceMap.append(surfaceId);
-}
-
-bool FakeOffscreenCanvasSurfaceImpl::isSurfaceInSurfaceMap(const cc::SurfaceId& surfaceId)
-{
-    return m_fakeSurfaceMap.contains(surfaceId);
-}
-
 void CanvasSurfaceLayerBridgeTest::SetUp()
 {
     m_surfaceService = wrapUnique(new FakeOffscreenCanvasSurfaceImpl());
@@ -113,7 +85,6 @@ void CanvasSurfaceLayerBridgeTest::SetUp()
 TEST_F(CanvasSurfaceLayerBridgeTest, SurfaceLayerCreation)
 {
     bool success = this->surfaceLayerBridge()->createSurfaceLayer(50, 50);
-    EXPECT_TRUE(this->surfaceService()->isSurfaceInSurfaceMap(this->surfaceLayerBridge()->getSurfaceId()));
     EXPECT_TRUE(success);
 }
 

third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom

@@ -14,7 +14,6 @@ interface OffscreenCanvasSurface {
   // unique surface_id.id_namespace alone.
   [Sync]
   GetSurfaceId() => (cc.mojom.SurfaceId surface_id);
-  RequestSurfaceCreation(cc.mojom.SurfaceId surface_id);
 
   Require(cc.mojom.SurfaceId surface_id, cc.mojom.SurfaceSequence sequence);
   Satisfy(cc.mojom.SurfaceSequence sequence);