Avoid destroying surface before the parent surface stops referencing it.

content/browser/compositor/delegated_frame_host.cc

Index: content/browser/compositor/delegated_frame_host.cc
diff --git a/content/browser/compositor/delegated_frame_host.cc b/content/browser/compositor/delegated_frame_host.cc
index 70b93ae08e9f8ddb6a36c3e7d02eae4367211681..9121ae0a7388ac0a35dcd14e16c9bef2ccdb028a 100644
--- a/content/browser/compositor/delegated_frame_host.cc
+++ b/content/browser/compositor/delegated_frame_host.cc
@@ -372,16 +372,22 @@ void DelegatedFrameHost::SwapDelegatedFrame(
       if (!surface_factory_) {
         ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
         cc::SurfaceManager* manager = factory->GetSurfaceManager();
-        id_allocator_ = factory->CreateSurfaceIdAllocator();
+        id_allocator_ =

[danakj, 2014/10/07 20:33:19]

You said "Also move ownership of the SurfaceIdAllocator to the
ui::Compositor, so that the surface id namespace for a
compositor will stay the same across all output surfaces it
ever uses."

Why isn't this id_allocator_ here enough? Do you need a different namespace for
the frames submitted from this host?

And the Compositor allocator only exists to grab a namespace? Maybe we can do
that without requiring us to own an allocator there? Can someone not generate a
namespace and give that to here so we can generate a sequence for the frame?

+            factory->GetContextFactory()->CreateSurfaceIdAllocator();
         surface_factory_ =
             make_scoped_ptr(new cc::SurfaceFactory(manager, this));
       }
       if (surface_id_.is_null() || frame_size != current_surface_size_ ||
           frame_size_in_dip != current_frame_size_in_dip_) {
-        // TODO(jbauman): Wait to destroy this surface until the parent has
-        // finished using it.
-        if (!surface_id_.is_null())
-          surface_factory_->Destroy(surface_id_);
+        if (!surface_id_.is_null()) {
+          if (compositor) {
+            std::set<cc::SurfaceSequence> seq;
+            seq.insert(compositor->CreateSurfaceSequence());

[danakj, 2014/10/07 20:08:25]

What step (in that 1-10 steps) in your design document is this
CreateSurfaceSequence?

[danakj, 2014/10/07 20:33:19]

What if the layer moves to another compositor after this sequence has been
inserted? Is that possible?

It seems to me like this sequence should be created here and given to the
ui::Layer along with the frame, so we can be sure they travel together? Maybe
the ui::Layer could invoke a swap promise to insert it, once it knows a frame is
happening?

[danakj, 2014/10/08 21:51:21]

Comment here please explaining that we want the compositor which /had the old
surface/ to submit a frame before we destroy the old surface (not the compositor
that the layer will be attached to when it's drawn).

+            surface_factory_->DestroyOnSequence(surface_id_, seq);

[danakj, 2014/10/02 01:53:45]

Can you help me understand what's going on here?

What does it mean to satisfy?

Why is the Sequence a set instead of a single id?

What's up with the SwapPromise putting a sequence number (without a namespace)
into the metadata when the browser makes a new surface?

+          } else {
+            surface_factory_->Destroy(surface_id_);
+          }
+        }
         surface_id_ = id_allocator_->GenerateId();
         surface_factory_->Create(surface_id_, frame_size);
         client_->GetLayer()->SetShowSurface(surface_id_, frame_size_in_dip);