Trim unneeded depedendencies from sky/compositor

Now we just depend on Skia and Base, which makes the compositor easier
to understand.

Also, update our skia/ext/refptr.h to match the current version of the
code in Chromium. This version adds support for several useful C++11
patterns.

skia/ext/refptr.h

@@ -5,6 +5,9 @@
 #ifndef SKIA_EXT_REFPTR_H_
 #define SKIA_EXT_REFPTR_H_
 
+#include <algorithm>
+#include <cstddef>
+
 #include "third_party/skia/include/core/SkRefCnt.h"
 
 namespace skia {
@@ -37,6 +40,12 @@ namespace skia {
 //
 //   skia::RefPtr<SkShader> shader = skia::SharePtr(paint.getShader());
 //
+// To pass a reference while clearing the pointer (without changing the ref
+// count):
+//
+//   skia::RefPtr<SkShader> shader = ...;
+//   UseThisShader(std::move(shader));
+//
 // Never call ref() or unref() on the underlying ref-counted pointer. If you
 // AdoptRef() the raw pointer immediately into a skia::RefPtr and always work
 // with skia::RefPtr instances instead, the ref-counting will be taken care of
@@ -44,23 +53,43 @@ namespace skia {
 template<typename T>
 class RefPtr {
  public:
-  RefPtr() : ptr_(NULL) {}
+  RefPtr() : ptr_(nullptr) {}
+
+  RefPtr(std::nullptr_t) : ptr_(nullptr) {}
 
+  // Copy constructor.
   RefPtr(const RefPtr& other)
       : ptr_(other.get()) {
     SkSafeRef(ptr_);
   }
 
+  // Copy conversion constructor.
   template<typename U>
   RefPtr(const RefPtr<U>& other)
       : ptr_(other.get()) {
     SkSafeRef(ptr_);
   }
 
+  // Move constructor. This is required in addition to the conversion
+  // constructor below in order for clang to warn about pessimizing moves.
+  RefPtr(RefPtr&& other) : ptr_(other.get()) { other.ptr_ = nullptr; }
+
+  // Move conversion constructor.
+  template <typename U>
+  RefPtr(RefPtr<U>&& other)
+      : ptr_(other.get()) {
+    other.ptr_ = nullptr;
+  }
+
   ~RefPtr() {
     clear();
   }
 
+  RefPtr& operator=(std::nullptr_t) {
+    clear();
+    return *this;
+  }
+
   RefPtr& operator=(const RefPtr& other) {
     SkRefCnt_SafeAssign(ptr_, other.get());
     return *this;
@@ -72,9 +101,16 @@ class RefPtr {
     return *this;
   }
 
+  template <typename U>
+  RefPtr& operator=(RefPtr<U>&& other) {
+    RefPtr<T> temp(std::move(other));
+    std::swap(ptr_, temp.ptr_);
+    return *this;
+  }
+
   void clear() {
     T* to_unref = ptr_;
-    ptr_ = NULL;
+    ptr_ = nullptr;
     SkSafeUnref(to_unref);
   }
 
@@ -84,7 +120,7 @@ class RefPtr {
 
   typedef T* RefPtr::*unspecified_bool_type;
   operator unspecified_bool_type() const {
-    return ptr_ ? &RefPtr::ptr_ : NULL;
+    return ptr_ ? &RefPtr::ptr_ : nullptr;
   }
 
  private:
@@ -102,6 +138,9 @@ class RefPtr {
 
   template<typename U>
   friend RefPtr<U> SharePtr(U* ptr);
+
+  template <typename U>
+  friend class RefPtr;
 };
 
 // For objects that have an unowned reference (such as newly created objects).

skia/ext/refptr_unittest.cc

@@ -44,7 +44,7 @@ TEST(RefPtrTest, ReferenceCounting) {
       EXPECT_FALSE(ref->unique());
 
       EXPECT_FALSE(refptr2);
-      EXPECT_EQ(NULL, refptr2.get());
+      EXPECT_EQ(nullptr, refptr2.get());
 
       EXPECT_TRUE(refptr3);
       EXPECT_FALSE(refptr3->unique());
@@ -121,5 +121,98 @@ TEST(RefPtrTest, Upcast) {
   EXPECT_FALSE(parent->unique());
 }
 
+// Counts the number of ref/unref operations (which require atomic operations)
+// that are done.
+class RefCountCounter : public SkRefCnt {
+ public:
+  void ref() const {
+    ref_count_changes_++;
+    SkRefCnt::ref();
+  }
+  void unref() const {
+    ref_count_changes_++;
+    SkRefCnt::unref();
+  }
+  int ref_count_changes() const { return ref_count_changes_; }
+  void ResetRefCountChanges() { ref_count_changes_ = 0; }
+
+ private:
+  mutable int ref_count_changes_ = 0;
+};
+
+TEST(RefPtrTest, ConstructionFromTemporary) {
+  // No ref count changes to move temporary into a local.
+  RefPtr<RefCountCounter> object = skia::AdoptRef(new RefCountCounter);
+  EXPECT_EQ(0, object->ref_count_changes());
+
+  // Only one change to share the pointer.
+  object->ResetRefCountChanges();
+  RefPtr<RefCountCounter> shared = skia::SharePtr(object.get());
+  EXPECT_EQ(1, object->ref_count_changes());
+
+  // Two ref count changes for the extra ref when passed as an argument, but no
+  // more.
+  object->ResetRefCountChanges();
+  auto do_nothing = [](RefPtr<RefCountCounter>) {};
+  do_nothing(object);
+  EXPECT_EQ(2, object->ref_count_changes());
+
+  // No ref count changes when passing a newly adopted ref as an argument.
+  auto lambda = [](RefPtr<RefCountCounter> arg) {
+    EXPECT_EQ(0, arg->ref_count_changes());
+  };
+  lambda(skia::AdoptRef(new RefCountCounter));
+}
+
+TEST(RefPtrTest, AssignmentFromTemporary) {
+  // No ref count changes to move temporary into a local.
+  RefPtr<RefCountCounter> object;
+  object = skia::AdoptRef(new RefCountCounter);
+  EXPECT_EQ(0, object->ref_count_changes());
+
+  // Only one change to share the pointer.
+  object->ResetRefCountChanges();
+  RefPtr<RefCountCounter> shared;
+  shared = skia::SharePtr(object.get());
+  EXPECT_EQ(1, object->ref_count_changes());
+}
+
+TEST(RefPtrTest, PassIntoArguments) {
+  // No ref count changes when passing an argument with Pass().
+  RefPtr<RefCountCounter> object = skia::AdoptRef(new RefCountCounter);
+  RefPtr<RefCountCounter> object2 = std::move(object);
+  auto lambda = [](RefPtr<RefCountCounter> arg) {
+    EXPECT_EQ(0, arg->ref_count_changes());
+  };
+  lambda(std::move(object2));
+}
+
+class DestructionNotifier : public SkRefCnt {
+ public:
+  DestructionNotifier(bool* flag) : flag_(flag) {}
+  ~DestructionNotifier() override { *flag_ = true; }
+
+ private:
+  bool* flag_;
+};
+
+TEST(RefPtrTest, Nullptr) {
+  RefPtr<SkRefCnt> null(nullptr);
+  EXPECT_FALSE(null);
+
+  bool is_destroyed = false;
+  RefPtr<DestructionNotifier> destroy_me =
+      skia::AdoptRef(new DestructionNotifier(&is_destroyed));
+  destroy_me = nullptr;
+  EXPECT_TRUE(is_destroyed);
+  EXPECT_FALSE(destroy_me);
+
+  // Check that returning nullptr from a function correctly causes an implicit
+  // conversion.
+  auto lambda = []() -> RefPtr<SkRefCnt> { return nullptr; };
+  RefPtr<SkRefCnt> returned = lambda();
+  EXPECT_FALSE(returned);
+}
+
 }  // namespace
 }  // namespace skia

sky/compositor/BUILD.gn

@@ -28,8 +28,6 @@ source_set("compositor") {
     "paint_context.h",
     "picture_layer.cc",
     "picture_layer.h",
-    "picture_serializer.cc",
-    "picture_serializer.h",
     "raster_cache.cc",
     "raster_cache.h",
     "performance_overlay_layer.cc",
@@ -43,8 +41,5 @@ source_set("compositor") {
   deps = [
     "//base",
     "//skia",
-    "//sky/engine/wtf",
-    "//ui/gfx",
-    "//ui/gfx/geometry",
   ]
 }

sky/compositor/color_filter_layer.cc

@@ -19,8 +19,8 @@ void ColorFilterLayer::Preroll(PrerollContext* context, const SkMatrix& matrix)
 }
 
 void ColorFilterLayer::Paint(PaintContext::ScopedFrame& frame) {
-  RefPtr<SkColorFilter> color_filter =
-      adoptRef(SkColorFilter::CreateModeFilter(color_, transfer_mode_));
+  skia::RefPtr<SkColorFilter> color_filter =
+      skia::AdoptRef(SkColorFilter::CreateModeFilter(color_, transfer_mode_));
   SkPaint paint;
   paint.setColorFilter(color_filter.get());
 

sky/compositor/layer.h

@@ -8,9 +8,10 @@
 #include <memory>
 #include <vector>
 
+#include "base/logging.h"
 #include "base/macros.h"
-#include "sky/engine/wtf/PassRefPtr.h"
-#include "sky/engine/wtf/RefPtr.h"
+#include "skia/ext/refptr.h"
+#include "sky/compositor/paint_context.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "third_party/skia/include/core/SkColorFilter.h"
@@ -20,7 +21,6 @@
 #include "third_party/skia/include/core/SkRect.h"
 #include "third_party/skia/include/core/SkRRect.h"
 #include "third_party/skia/include/core/SkXfermode.h"
-#include "sky/compositor/paint_context.h"
 
 namespace sky {
 namespace compositor {
@@ -46,7 +46,7 @@ class Layer {
   const bool has_paint_bounds() const { return has_paint_bounds_; }
 
   const SkRect& paint_bounds() const {
-    ASSERT(has_paint_bounds_);
+    DCHECK(has_paint_bounds_);
     return paint_bounds_;
   }
 

sky/compositor/paint_context.cc

@@ -3,10 +3,9 @@
 // found in the LICENSE file.
 
 #include "sky/compositor/paint_context.h"
+
 #include "base/logging.h"
 #include "third_party/skia/include/core/SkCanvas.h"
-#include "sky/compositor/picture_serializer.h"
-#include "sky/engine/wtf/RefPtr.h"
 
 namespace sky {
 namespace compositor {
@@ -28,50 +27,24 @@ void PaintContext::endFrame(ScopedFrame& frame, bool enableInstrumentation) {
   }
 }
 
-PaintContext::ScopedFrame PaintContext::AcquireFrame(GrContext* gr_context,
-                                                     SkCanvas& canvas) {
-  return ScopedFrame(*this, gr_context, canvas);
-}
-
 PaintContext::ScopedFrame PaintContext::AcquireFrame(
-    const std::string& trace_file_name,
-    gfx::Size frame_size) {
-  return ScopedFrame(*this, trace_file_name, frame_size);
+    GrContext* gr_context, SkCanvas& canvas, bool instrumentation_enabled) {
+  return ScopedFrame(*this, gr_context, canvas, instrumentation_enabled);
 }
 
 PaintContext::ScopedFrame::ScopedFrame(PaintContext& context,
                                        GrContext* gr_context,
-                                       SkCanvas& canvas)
+                                       SkCanvas& canvas,
+                                       bool instrumentation_enabled)
     : context_(context), gr_context_(gr_context), canvas_(&canvas),
-      instrumentation_enabled_(true) {
+      instrumentation_enabled_(instrumentation_enabled) {
   context_.beginFrame(*this, instrumentation_enabled_);
 }
 
 PaintContext::ScopedFrame::ScopedFrame(ScopedFrame&& frame) = default;
 
-PaintContext::ScopedFrame::ScopedFrame(PaintContext& context,
-                                       const std::string& trace_file_name,
-                                       gfx::Size frame_size)
-    : context_(context),
-      trace_file_name_(trace_file_name),
-      trace_recorder_(new SkPictureRecorder()),
-      instrumentation_enabled_(false) {
-  trace_recorder_->beginRecording(
-      SkRect::MakeWH(frame_size.width(), frame_size.height()));
-  canvas_ = trace_recorder_->getRecordingCanvas();
-  DCHECK(canvas_);
-  DCHECK(trace_file_name.length() > 0);
-  context_.beginFrame(*this, instrumentation_enabled_);
-}
-
 PaintContext::ScopedFrame::~ScopedFrame() {
   context_.endFrame(*this, instrumentation_enabled_);
-
-  if (trace_file_name_.length() > 0) {
-    RefPtr<SkPicture> picture =
-        adoptRef(trace_recorder_->endRecordingAsPicture());
-    SerializePicture(trace_file_name_.c_str(), picture.get());
-  }
 }
 
 PaintContext::~PaintContext() {

sky/compositor/paint_context.h

@@ -14,7 +14,6 @@
 #include "sky/compositor/raster_cache.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkPictureRecorder.h"
-#include "ui/gfx/geometry/size.h"
 
 namespace sky {
 namespace compositor {
@@ -24,27 +23,22 @@ class PaintContext {
   class ScopedFrame {
    public:
     SkCanvas& canvas() { return *canvas_; }
-
     PaintContext& context() const { return context_; }
     GrContext* gr_context() const { return gr_context_; }
 
     ScopedFrame(ScopedFrame&& frame);
-
     ~ScopedFrame();
 
    private:
     PaintContext& context_;
     GrContext* gr_context_;
     SkCanvas* canvas_;
-    std::string trace_file_name_;
-    std::unique_ptr<SkPictureRecorder> trace_recorder_;
     const bool instrumentation_enabled_;
 
-    ScopedFrame(PaintContext& context, GrContext* gr_context, SkCanvas& canvas);
-
     ScopedFrame(PaintContext& context,
-                const std::string& trace_file_name,
-                gfx::Size frame_size);
+                GrContext* gr_context,
+                SkCanvas& canvas,
+                bool instrumentation_enabled);
 
     friend class PaintContext;
 
@@ -54,17 +48,13 @@ class PaintContext {
   PaintContext();
   ~PaintContext();
 
-  ScopedFrame AcquireFrame(GrContext* gr_context, SkCanvas& canvas);
-
-  ScopedFrame AcquireFrame(const std::string& trace_file_name,
-                           gfx::Size frame_size);
+  ScopedFrame AcquireFrame(GrContext* gr_context,
+                           SkCanvas& canvas,
+                           bool instrumentation_enabled = true);
 
   RasterCache& raster_cache() { return raster_cache_; }
-
   const instrumentation::Counter& frame_count() const { return frame_count_; }
-
   const instrumentation::Stopwatch& frame_time() const { return frame_time_; }
-
   instrumentation::Stopwatch& engine_time() { return engine_time_; };
 
  private:

sky/compositor/picture_layer.h

@@ -17,7 +17,7 @@ class PictureLayer : public Layer {
 
   void set_offset(const SkPoint& offset) { offset_ = offset; }
 
-  void set_picture(PassRefPtr<SkPicture> picture) { picture_ = picture; }
+  void set_picture(SkPicture* picture) { picture_ = skia::SharePtr(picture); }
 
   SkPicture* picture() const { return picture_.get(); }
 
@@ -26,10 +26,10 @@ class PictureLayer : public Layer {
 
  private:
   SkPoint offset_;
-  RefPtr<SkPicture> picture_;
+  skia::RefPtr<SkPicture> picture_;
 
   // If we rasterized the picture separately, image_ holds the pixels.
-  RefPtr<SkImage> image_;
+  skia::RefPtr<SkImage> image_;
 
   DISALLOW_COPY_AND_ASSIGN(PictureLayer);
 };

sky/compositor/raster_cache.cc

@@ -41,9 +41,9 @@ RasterCache::Entry::Entry() {
 RasterCache::Entry::~Entry() {
 }
 
-RefPtr<SkImage> RasterCache::GetPrerolledImage(GrContext* context,
-                                               SkPicture* picture,
-                                               const SkMatrix& ctm) {
+skia::RefPtr<SkImage> RasterCache::GetPrerolledImage(GrContext* context,
+                                                     SkPicture* picture,
+                                                     const SkMatrix& ctm) {
 #if ENABLE_RASTER_CACHE
   SkScalar scaleX = ctm.getScaleX();
   SkScalar scaleY = ctm.getScaleY();
@@ -80,14 +80,14 @@ RefPtr<SkImage> RasterCache::GetPrerolledImage(GrContext* context,
                    "width", physical_size.width(),
                    "height", physical_size.height());
       SkImageInfo info = SkImageInfo::MakeN32Premul(physical_size);
-      RefPtr<SkSurface> surface = adoptRef(SkSurface::NewRenderTarget(
-          context, SkSurface::kYes_Budgeted, info));
+      skia::RefPtr<SkSurface> surface = skia::AdoptRef(
+          SkSurface::NewRenderTarget(context, SkSurface::kYes_Budgeted, info));
       if (surface) {
         SkCanvas* canvas = surface->getCanvas();
         canvas->clear(SK_ColorTRANSPARENT);
         SkMatrix matrix = SkMatrix::MakeScale(scaleX, scaleY);
         canvas->drawPicture(picture, &matrix, nullptr);
-        entry.image = adoptRef(surface->newImageSnapshot());
+        entry.image = skia::AdoptRef(surface->newImageSnapshot());
       }
     }
   }

sky/compositor/raster_cache.h

@@ -9,11 +9,10 @@
 #include <unordered_map>
 
 #include "base/macros.h"
+#include "skia/ext/refptr.h"
 #include "third_party/skia/include/core/SkSize.h"
 #include "third_party/skia/include/core/SkImage.h"
 #include "sky/compositor/instrumentation.h"
-#include "sky/engine/wtf/PassRefPtr.h"
-#include "sky/engine/wtf/RefPtr.h"
 
 namespace sky {
 namespace compositor {
@@ -23,8 +22,8 @@ class RasterCache {
   RasterCache();
   ~RasterCache();
 
-  RefPtr<SkImage> GetPrerolledImage(GrContext* context, SkPicture* picture,
-                                    const SkMatrix& ctm);
+  skia::RefPtr<SkImage> GetPrerolledImage(
+      GrContext* context, SkPicture* picture, const SkMatrix& ctm);
   void SweepAfterFrame();
 
  private:
@@ -35,7 +34,7 @@ class RasterCache {
     bool used_this_frame = false;
     int access_count = 0;
     SkISize physical_size;
-    RefPtr<SkImage> image;
+    skia::RefPtr<SkImage> image;
   };
 
   using Cache = std::unordered_map<uint32_t, Entry>;

sky/compositor/shader_mask_layer.h

@@ -17,7 +17,7 @@ class ShaderMaskLayer : public ContainerLayer {
   ShaderMaskLayer();
   ~ShaderMaskLayer() override;
 
-  void set_shader(SkShader* shader) { shader_ = shader; }
+  void set_shader(SkShader* shader) { shader_ = skia::SharePtr(shader); }
 
   void set_mask_rect(const SkRect& mask_rect) {
     mask_rect_ = mask_rect;
@@ -32,7 +32,7 @@ class ShaderMaskLayer : public ContainerLayer {
   void Paint(PaintContext::ScopedFrame& frame) override;
 
  private:
-  RefPtr<SkShader> shader_;
+  skia::RefPtr<SkShader> shader_;
   SkRect mask_rect_;
   SkXfermode::Mode transfer_mode_;
 

sky/shell/BUILD.gn

@@ -6,6 +6,8 @@ source_set("common") {
   sources = [
     "gpu/ganesh_canvas.cc",
     "gpu/ganesh_canvas.h",
+    "gpu/picture_serializer.cc",
+    "gpu/picture_serializer.h",
     "rasterizer.cc",
     "rasterizer.h",
     "platform_view.cc",

sky/shell/gpu/direct/rasterizer_direct.cc

@@ -10,6 +10,9 @@
 #include "sky/compositor/layer.h"
 #include "sky/compositor/paint_context.h"
 #include "sky/compositor/picture_layer.h"
+#include "sky/engine/wtf/PassRefPtr.h"
+#include "sky/engine/wtf/RefPtr.h"
+#include "sky/shell/gpu/picture_serializer.h"
 #include "sky/shell/shell.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkPicture.h"
@@ -111,9 +114,18 @@ void RasterizerDirect::Draw(uint64_t layer_tree_ptr,
 
   if (frameExceededThreshold || tracingController.picture_tracing_enabled()) {
     base::FilePath path = tracingController.PictureTracingPathForCurrentTime();
-    sky::compositor::PaintContext::ScopedFrame to_file_frame =
-        paint_context_.AcquireFrame(path.AsUTF8Unsafe(), size);
-    layer_tree->root_layer()->Paint(to_file_frame);
+
+    SkPictureRecorder recoder;
+    recoder.beginRecording(SkRect::MakeWH(size.width(), size.height()));
+
+    {
+      auto frame = paint_context_.AcquireFrame(
+          nullptr, *recoder.getRecordingCanvas(), false);
+      layer_tree->Raster(frame);
+    }
+
+    RefPtr<SkPicture> picture = adoptRef(recoder.endRecordingAsPicture());
+    SerializePicture(path, picture.get());
   }
 
   callback.Run();

sky/compositor/picture_serializer.cc → sky/shell/gpu/picture_serializer.cc

@@ -2,8 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "sky/shell/gpu/picture_serializer.h"
+
 #include <vector>
-#include "sky/compositor/picture_serializer.h"
 #include "third_party/skia/include/core/SkData.h"
 #include "third_party/skia/include/core/SkPixelSerializer.h"
 #include "third_party/skia/include/core/SkStream.h"
@@ -27,8 +28,8 @@ class PngPixelSerializer : public SkPixelSerializer {
   }
 };
 
-void SerializePicture(const char* file_name, SkPicture* picture) {
-  SkFILEWStream stream(file_name);
+void SerializePicture(const base::FilePath& file_name, SkPicture* picture) {
+  SkFILEWStream stream(file_name.AsUTF8Unsafe().c_str());
   PngPixelSerializer serializer;
   picture->serialize(&stream, &serializer);
 }

sky/compositor/picture_serializer.h → sky/shell/gpu/picture_serializer.h

@@ -2,15 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef SKY_COMPOSITOR_PICTURE_SERIALIZER_H_
-#define SKY_COMPOSITOR_PICTURE_SERIALIZER_H_
+#ifndef SKY_SHELL_GPU_PICTURE_SERIALIZER_H_
+#define SKY_SHELL_GPU_PICTURE_SERIALIZER_H_
 
+#include "base/files/file_path.h"
 #include "third_party/skia/include/core/SkPicture.h"
 
 namespace sky {
 
-void SerializePicture(const char* file_name, SkPicture*);
+void SerializePicture(const base::FilePath& file_name, SkPicture*);
 
 }  // namespace sky
 
-#endif  // SKY_COMPOSITOR_PICTURE_SERIALIZER_H_
+#endif  // SKY_SHELL_GPU_PICTURE_SERIALIZER_H_