LOG and clear SurfaceFactory if it isn't empty on destruction.

Otherwise it will leave dangling pointers in the SurfaceManager. Also add a DestroyAll method to allow owners to ensure every surface in the factory is destroyed.

BUG=428127

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

Cr-Commit-Position: refs/heads/master@{#302339}

3 files changed, 23 insertions, 4 deletions

diff --git a/cc/surfaces/surface_factory.cc b/cc/surfaces/surface_factory.cc
index dcf1130..88c5455 100644
--- a/cc/surfaces/surface_factory.cc
+++ b/cc/surfaces/surface_factory.cc
@@ -17,6 +17,17 @@ SurfaceFactory::SurfaceFactory(SurfaceManager* manager,
 }
 
 SurfaceFactory::~SurfaceFactory() {
+  if (!surface_map_.empty()) {
+    LOG(ERROR) << "SurfaceFactory has " << surface_map_.size()
+               << " entries in map on destruction.";
+  }
+  DestroyAll();
+}
+
+void SurfaceFactory::DestroyAll() {
+  for (auto& surface : surface_map_)
+    manager_->DeregisterSurface(surface.first);
+  surface_map_.clear();
 }
 
 void SurfaceFactory::Create(SurfaceId surface_id, const gfx::Size& size) {
diff --git a/cc/surfaces/surface_factory.h b/cc/surfaces/surface_factory.h
index 3eb4f43..13d1d63 100644
--- a/cc/surfaces/surface_factory.h
+++ b/cc/surfaces/surface_factory.h
@@ -5,6 +5,8 @@
 #ifndef CC_SURFACES_SURFACE_FACTORY_H_
 #define CC_SURFACES_SURFACE_FACTORY_H_
 
+#include <set>
+
 #include "base/callback_forward.h"
 #include "base/containers/scoped_ptr_hash_map.h"
 #include "base/memory/scoped_ptr.h"
@@ -40,6 +42,7 @@ class CC_SURFACES_EXPORT SurfaceFactory
   void Destroy(SurfaceId surface_id);
   void DestroyOnSequence(SurfaceId surface_id,
                          const std::set<SurfaceSequence>& dependency_set);
+  void DestroyAll();
   // A frame can only be submitted to a surface created by this factory,
   // although the frame may reference surfaces created by other factories.
   // The callback is called the first time this frame is used to draw.
diff --git a/cc/surfaces/surface_factory_unittest.cc b/cc/surfaces/surface_factory_unittest.cc
index 4e0865a..e36636c 100644
--- a/cc/surfaces/surface_factory_unittest.cc
+++ b/cc/surfaces/surface_factory_unittest.cc
@@ -42,7 +42,10 @@ class SurfaceFactoryTest : public testing::Test {
     factory_.Create(surface_id_, gfx::Size(5, 5));
   }
 
-  virtual ~SurfaceFactoryTest() { factory_.Destroy(surface_id_); }
+  virtual ~SurfaceFactoryTest() {
+    if (!surface_id_.is_null())
+      factory_.Destroy(surface_id_);
+  }
 
   void SubmitFrameWithResources(ResourceProvider::ResourceId* resource_ids,
                                 size_t num_resource_ids) {
@@ -355,9 +358,8 @@ TEST_F(SurfaceFactoryTest, ResourceLifetime) {
   }
 }
 
-// Tests shutting down the factory with a surface with outstanding refs still in
-// the map.
-TEST_F(SurfaceFactoryTest, DestroyWithResourceRefs) {
+// Tests doing a DestroyAll before shutting down the factory;
+TEST_F(SurfaceFactoryTest, DestroyAll) {
   SurfaceId id(7);
   factory_.Create(id, gfx::Size(1, 1));
 
@@ -369,6 +371,9 @@ TEST_F(SurfaceFactoryTest, DestroyWithResourceRefs) {
   scoped_ptr<CompositorFrame> frame(new CompositorFrame);
   frame->delegated_frame_data = frame_data.Pass();
   factory_.SubmitFrame(id, frame.Pass(), base::Closure());
+
+  surface_id_ = SurfaceId();
+  factory_.DestroyAll();
 }
 
 TEST_F(SurfaceFactoryTest, DestroySequence) {